From 5e3eb28cb892d73723e3ec924d0e01d9f3d1dd58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Sun, 18 Jul 2021 18:28:43 +0300 Subject: [PATCH 1/9] Add test case for too lax application/json prefix matching --- tests/test_helpers.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index 01280ab74ed..e945d21f4c3 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -789,6 +789,14 @@ def test_is_expected_content_type_json_non_lowercase(): ) +def test_is_expected_content_type_json_match_not_just_prefix(): + expected_ct = "application/json" + response_ct = "application/json-seq" + assert not is_expected_content_type( + response_content_type=response_ct, expected_content_type=expected_ct + ) + + def test_is_expected_content_type_non_json_match_exact(): expected_ct = "text/javascript" response_ct = "text/javascript" From 203e3cf408b0f3be88c7f40fb5c1f09920787381 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Sun, 31 Oct 2021 07:52:21 +0200 Subject: [PATCH 2/9] Add test case for matching application/json with a charset --- tests/test_helpers.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index e945d21f4c3..b97d3db4b25 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -789,6 +789,14 @@ def test_is_expected_content_type_json_non_lowercase(): ) +def test_is_expected_content_type_json_with_charset(): + expected_ct = "application/json" + response_ct = "application/json; charset=UTF-8" + assert is_expected_content_type( + response_content_type=response_ct, expected_content_type=expected_ct + ) + + def test_is_expected_content_type_json_match_not_just_prefix(): expected_ct = "application/json" response_ct = "application/json-seq" From 4a4dc9f250497fcdc84ff7d0aaa99e3c13128bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Sun, 31 Oct 2021 07:54:52 +0200 Subject: [PATCH 3/9] Fix too lax expected JSON content-type parsing E.g. application/jsonfoobar is not expected for it, but ones with parameters -- for example charset -- are. The IANA registered application/json-seq is a good example. --- CHANGES/6180.bugfix | 1 + aiohttp/helpers.py | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 CHANGES/6180.bugfix diff --git a/CHANGES/6180.bugfix b/CHANGES/6180.bugfix new file mode 100644 index 00000000000..55d6938b91f --- /dev/null +++ b/CHANGES/6180.bugfix @@ -0,0 +1 @@ +Fix JSON media type matching to not accept everything starting with application/json. diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 92c0700ac06..0dd4fa0f6eb 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -124,7 +124,9 @@ def iscoroutinefunction(func: Any) -> bool: return asyncio.iscoroutinefunction(func) -json_re = re.compile(r"(?:application/|[\w.-]+/[\w.+-]+?\+)json", re.IGNORECASE) +json_re = re.compile( + r"(?:application/|[\w.-]+/[\w.+-]+?\+)json(?:\s*;.*)?$", re.IGNORECASE +) class BasicAuth(namedtuple("BasicAuth", ["login", "password", "encoding"])): From f4353371f3fad7e1aa0130a5e0fb85b74ca89476 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Mon, 1 Nov 2021 21:55:10 +0200 Subject: [PATCH 4/9] Use parametrized JSON expected content type tests Co-authored-by: Sviatoslav Sydorenko --- tests/test_helpers.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index b97d3db4b25..c11fcce854a 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -789,20 +789,18 @@ def test_is_expected_content_type_json_non_lowercase(): ) -def test_is_expected_content_type_json_with_charset(): - expected_ct = "application/json" - response_ct = "application/json; charset=UTF-8" +@pytest.mark.paratetrize( + ("expected_ct", "response_ct", "should_match"), + ( + ("application/json", "application/json; charset=UTF-8", True), + ("application/json", "application/json-seq", False), + ), + ids=("with-charset", "non-plus-suffix-should-not-match"), +) +def test_is_expected_content_type_json(expected_ct, response_ct, should_match): assert is_expected_content_type( response_content_type=response_ct, expected_content_type=expected_ct - ) - - -def test_is_expected_content_type_json_match_not_just_prefix(): - expected_ct = "application/json" - response_ct = "application/json-seq" - assert not is_expected_content_type( - response_content_type=response_ct, expected_content_type=expected_ct - ) + ) is should_match def test_is_expected_content_type_non_json_match_exact(): From 1956da82f1541ff48cfc6d635c6e4408e9de8a50 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 1 Nov 2021 19:55:53 +0000 Subject: [PATCH 5/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_helpers.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c11fcce854a..c3f95ff7e7d 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -798,9 +798,12 @@ def test_is_expected_content_type_json_non_lowercase(): ids=("with-charset", "non-plus-suffix-should-not-match"), ) def test_is_expected_content_type_json(expected_ct, response_ct, should_match): - assert is_expected_content_type( - response_content_type=response_ct, expected_content_type=expected_ct - ) is should_match + assert ( + is_expected_content_type( + response_content_type=response_ct, expected_content_type=expected_ct + ) + is should_match + ) def test_is_expected_content_type_non_json_match_exact(): From ab3de932ef803a184d4924c436bbc660ffd127ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Mon, 1 Nov 2021 21:57:49 +0200 Subject: [PATCH 6/9] Rephrase CHANGES entry for #6180 --- CHANGES/6180.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/6180.bugfix b/CHANGES/6180.bugfix index 55d6938b91f..0358d8ce2dc 100644 --- a/CHANGES/6180.bugfix +++ b/CHANGES/6180.bugfix @@ -1 +1 @@ -Fix JSON media type matching to not accept everything starting with application/json. +Fixed matching the JSON media type to not accept arbitrary characters after ``application/json`` or the ``+json`` media type suffix. From 1e26966717928c59a2c0e4e08d4f3ac4ad51f853 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Tue, 2 Nov 2021 23:02:29 +0200 Subject: [PATCH 7/9] Check content type expectedness using parameterless, parsed source --- aiohttp/client_reqrep.py | 6 +++--- aiohttp/helpers.py | 7 ++++--- aiohttp/web_request.py | 6 +++--- tests/test_helpers.py | 19 +++++-------------- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index f70efff899f..035ba40b5fe 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -1031,13 +1031,13 @@ async def json( await self.read() if content_type: - ctype = self.headers.get(hdrs.CONTENT_TYPE, "").lower() - if not is_expected_content_type(ctype, content_type): + if not is_expected_content_type(self.content_type, content_type): raise ContentTypeError( self.request_info, self.history, message=( - "Attempt to decode JSON with " "unexpected mimetype: %s" % ctype + "Attempt to decode JSON with " + "unexpected mimetype: %s" % self.content_type ), headers=self.headers, ) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 0dd4fa0f6eb..53f76877af2 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -124,9 +124,7 @@ def iscoroutinefunction(func: Any) -> bool: return asyncio.iscoroutinefunction(func) -json_re = re.compile( - r"(?:application/|[\w.-]+/[\w.+-]+?\+)json(?:\s*;.*)?$", re.IGNORECASE -) +json_re = re.compile(r"(?:application/|[\w.-]+/[\w.+-]+?\+)json$", re.IGNORECASE) class BasicAuth(namedtuple("BasicAuth", ["login", "password", "encoding"])): @@ -425,6 +423,9 @@ def content_disposition_header( def is_expected_content_type( response_content_type: str, expected_content_type: str ) -> bool: + """Checks if received content type is processable as an expected one. + Both arguments should be given without parameters. + """ if expected_content_type == "application/json": return json_re.match(response_content_type) is not None return expected_content_type in response_content_type diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index 0424d7757c8..b47737a89b6 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -661,11 +661,11 @@ async def json( """Return BODY as JSON.""" body = await self.text() if content_type: - ctype = self.headers.get(hdrs.CONTENT_TYPE, "").lower() - if not is_expected_content_type(ctype, content_type): + if not is_expected_content_type(self.content_type, content_type): raise HTTPBadRequest( text=( - "Attempt to decode JSON with " "unexpected mimetype: %s" % ctype + "Attempt to decode JSON with " + "unexpected mimetype: %s" % self.content_type ) ) diff --git a/tests/test_helpers.py b/tests/test_helpers.py index c3f95ff7e7d..6c56e5d3aae 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -789,20 +789,11 @@ def test_is_expected_content_type_json_non_lowercase(): ) -@pytest.mark.paratetrize( - ("expected_ct", "response_ct", "should_match"), - ( - ("application/json", "application/json; charset=UTF-8", True), - ("application/json", "application/json-seq", False), - ), - ids=("with-charset", "non-plus-suffix-should-not-match"), -) -def test_is_expected_content_type_json(expected_ct, response_ct, should_match): - assert ( - is_expected_content_type( - response_content_type=response_ct, expected_content_type=expected_ct - ) - is should_match +def test_is_expected_content_type_json_trailing_chars(): + expected_ct = "application/json" + response_ct = "application/json-seq" + assert not is_expected_content_type( + response_content_type=response_ct, expected_content_type=expected_ct ) From 7bc72c7aa5f2e7e67e2e4fae57b522f2e5497314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Wed, 3 Nov 2021 07:27:41 +0200 Subject: [PATCH 8/9] Update aiohttp/helpers.py Co-authored-by: Sviatoslav Sydorenko --- aiohttp/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 53f76877af2..0a1bf85ea3d 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -424,6 +424,7 @@ def is_expected_content_type( response_content_type: str, expected_content_type: str ) -> bool: """Checks if received content type is processable as an expected one. + Both arguments should be given without parameters. """ if expected_content_type == "application/json": From 434c6c5839492676128d6a432efde61e74022bb2 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Wed, 3 Nov 2021 20:50:17 +0200 Subject: [PATCH 9/9] Update aiohttp/helpers.py Co-authored-by: Sviatoslav Sydorenko --- aiohttp/helpers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/aiohttp/helpers.py b/aiohttp/helpers.py index 0a1bf85ea3d..77eaf8c3550 100644 --- a/aiohttp/helpers.py +++ b/aiohttp/helpers.py @@ -425,6 +425,7 @@ def is_expected_content_type( ) -> bool: """Checks if received content type is processable as an expected one. + Both arguments should be given without parameters. """ if expected_content_type == "application/json":