Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions Lib/test/test_wsgiref.py
Original file line number Diff line number Diff line change
Expand Up @@ -504,14 +504,20 @@ def testExtras(self):
)

def testRaisesControlCharacters(self):
headers = Headers()
for c0 in control_characters_c0():
self.assertRaises(ValueError, headers.__setitem__, f"key{c0}", "val")
self.assertRaises(ValueError, headers.__setitem__, "key", f"val{c0}")
self.assertRaises(ValueError, headers.add_header, f"key{c0}", "val", param="param")
self.assertRaises(ValueError, headers.add_header, "key", f"val{c0}", param="param")
self.assertRaises(ValueError, headers.add_header, "key", "val", param=f"param{c0}")

with self.subTest(c0):
headers = Headers()
self.assertRaises(ValueError, headers.__setitem__, f"key{c0}", "val")
self.assertRaises(ValueError, headers.add_header, f"key{c0}", "val", param="param")
# HTAB (\x09) is allowed in values, not names.
if c0 == "\t":
headers["key"] = f"val{c0}"
headers.add_header("key", f"val{c0}")
headers.setdefault(f"key", f"val{c0}")
else:
self.assertRaises(ValueError, headers.__setitem__, "key", f"val{c0}")
self.assertRaises(ValueError, headers.add_header, "key", f"val{c0}", param="param")
self.assertRaises(ValueError, headers.add_header, "key", "val", param=f"param{c0}")

class ErrorHandler(BaseCGIHandler):
"""Simple handler subclass for testing BaseHandler"""
Expand Down
35 changes: 20 additions & 15 deletions Lib/wsgiref/headers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
# existence of which force quoting of the parameter value.
import re
tspecials = re.compile(r'[ \(\)<>@,;:\\"/\[\]\?=]')
_control_chars_re = re.compile(r'[\x00-\x1F\x7F]')
# Disallowed characters for headers and values.
# HTAB (\x09) is allowed in header values, but
# not in header names. (RFC 9110 Section 5.5)
_name_disallowed_re = re.compile(r'[\x00-\x1F\x7F]')
_value_disallowed_re = re.compile(r'[\x00-\x08\x0A-\x1F\x7F]')

def _formatparam(param, value=None, quote=1):
"""Convenience function to format and return a key=value pair.
Expand All @@ -36,13 +40,14 @@ def __init__(self, headers=None):
self._headers = headers
if __debug__:
for k, v in headers:
self._convert_string_type(k)
self._convert_string_type(v)
self._convert_string_type(k, name=True)
self._convert_string_type(v, name=False)

def _convert_string_type(self, value):
def _convert_string_type(self, value, *, name):
"""Convert/check value type."""
if type(value) is str:
if _control_chars_re.search(value):
regex = (_name_disallowed_re if name else _value_disallowed_re)
if regex.search(value):
raise ValueError("Control characters not allowed in headers")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError("Control characters not allowed in headers")
raise ValueError("Control characters not allowed in headers, values and statuses")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get what are values and statuses. I prefer to just say "headers".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, sorry for the confusion, I was missing one word: I wanted to say "in header names, values and statuses" because there seems to be a differenciation between values and statuses and names (see in my PR) and that's why I added this to the error message and now I thought that this is now more consistent as it's kind of the same error message. And there's a difference in the handling between header names and values (see in the issue linked here), but I think that for the short error message it's not required to go into detail with the exact allowed and disallowed characters. But thanks for the information that I missed one word, hope that this is clear now! Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError("Control characters not allowed in headers")
raise ValueError("Control characters not allowed in header names, values and statuses")

return value
raise AssertionError("Header names/values must be"
Expand All @@ -56,14 +61,14 @@ def __setitem__(self, name, val):
"""Set the value of a header."""
del self[name]
self._headers.append(
(self._convert_string_type(name), self._convert_string_type(val)))
(self._convert_string_type(name, name=True), self._convert_string_type(val, name=False)))

def __delitem__(self,name):
"""Delete all occurrences of a header, if present.

Does *not* raise an exception if the header is missing.
"""
name = self._convert_string_type(name.lower())
name = self._convert_string_type(name.lower(), name=True)
self._headers[:] = [kv for kv in self._headers if kv[0].lower() != name]

def __getitem__(self,name):
Expand All @@ -90,13 +95,13 @@ def get_all(self, name):
fields deleted and re-inserted are always appended to the header list.
If no fields exist with the given name, returns an empty list.
"""
name = self._convert_string_type(name.lower())
name = self._convert_string_type(name.lower(), name=True)
return [kv[1] for kv in self._headers if kv[0].lower()==name]


def get(self,name,default=None):
"""Get the first header value for 'name', or return 'default'"""
name = self._convert_string_type(name.lower())
name = self._convert_string_type(name.lower(), name=True)
for k,v in self._headers:
if k.lower()==name:
return v
Expand Down Expand Up @@ -151,8 +156,8 @@ def setdefault(self,name,value):
and value 'value'."""
result = self.get(name)
if result is None:
self._headers.append((self._convert_string_type(name),
self._convert_string_type(value)))
self._headers.append((self._convert_string_type(name, name=True),
self._convert_string_type(value, name=False)))
return value
else:
return result
Expand All @@ -175,13 +180,13 @@ def add_header(self, _name, _value, **_params):
"""
parts = []
if _value is not None:
_value = self._convert_string_type(_value)
_value = self._convert_string_type(_value, name=False)
parts.append(_value)
for k, v in _params.items():
k = self._convert_string_type(k)
k = self._convert_string_type(k, name=True)
if v is None:
parts.append(k.replace('_', '-'))
else:
v = self._convert_string_type(v)
v = self._convert_string_type(v, name=False)
parts.append(_formatparam(k.replace('_', '-'), v))
self._headers.append((self._convert_string_type(_name), "; ".join(parts)))
self._headers.append((self._convert_string_type(_name, name=True), "; ".join(parts)))
Loading