Skip to content

Commit

Permalink
Disallow newlines in reason (#9167)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dreamsorcerer authored Sep 19, 2024
1 parent 139f3a6 commit 88f3834
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGES/9167.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Rejected `\n` in `reason` values to avoid sending broken HTTP messages -- by :user:`Dreamsorcerer`.
2 changes: 2 additions & 0 deletions aiohttp/web_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ def __init__(
super().__init__()
if reason is None:
reason = self.default_reason
elif "\n" in reason:
raise ValueError("Reason cannot contain \\n")

if text is None:
if not self.empty_body:
Expand Down
2 changes: 2 additions & 0 deletions aiohttp/web_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ def set_status(
reason = HTTPStatus(self._status).phrase
except ValueError:
reason = ""
if "\n" in reason:
raise ValueError("Reason cannot contain \\n")
self._reason = reason

@property
Expand Down
4 changes: 4 additions & 0 deletions tests/test_web_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ def test_ctor_all(self) -> None:
assert resp.reason == "Done"
assert resp.status == 200

def test_multiline_reason(self) -> None:
with pytest.raises(ValueError, match=r"Reason cannot contain \\n"):
web.HTTPOk(reason="Bad\r\nInjected-header: foo")

def test_pickle(self) -> None:
resp = web.HTTPOk(
headers={"X-Custom": "value"},
Expand Down
13 changes: 9 additions & 4 deletions tests/test_web_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -811,14 +811,14 @@ async def test_start_force_close() -> None:

async def test___repr__() -> None:
req = make_request("GET", "/path/to")
resp = StreamResponse(reason=301)
resp = StreamResponse(reason="foo")
await resp.prepare(req)
assert "<StreamResponse 301 GET /path/to >" == repr(resp)
assert "<StreamResponse foo GET /path/to >" == repr(resp)


def test___repr___not_prepared() -> None:
resp = StreamResponse(reason=301)
assert "<StreamResponse 301 not prepared>" == repr(resp)
resp = StreamResponse(reason="foo")
assert "<StreamResponse foo not prepared>" == repr(resp)


async def test_keep_alive_http10_default() -> None:
Expand Down Expand Up @@ -1092,6 +1092,11 @@ async def test_render_with_body(buf: Any, writer: Any) -> None:
)


async def test_multiline_reason(buf: Any, writer: Any) -> None:
with pytest.raises(ValueError, match=r"Reason cannot contain \\n"):
Response(reason="Bad\r\nInjected-header: foo")


async def test_send_set_cookie_header(buf: Any, writer: Any) -> None:
resp = Response()
resp.cookies["name"] = "value"
Expand Down

0 comments on commit 88f3834

Please sign in to comment.