From 4de4edf2c05cc80c514c989db480f2fe23ad5ee2 Mon Sep 17 00:00:00 2001 From: Jake Howard Date: Tue, 10 Jun 2025 23:00:25 +0100 Subject: [PATCH] [5.2.x] Fixed #36447 -- Selected preferred media type based on quality. When matching which entry in the `Accept` header should be used for a given media type, the specificity matters. However once those are resolved, only the quality matters when selecting preference. Regression in c075508b4de8edf9db553b409f8a8ed2f26ecead. Thank you to Anders Kaseorg for the report. Backport of 12c1557060fc94fe5e1fbddc4578a4e29d38f77c from main. --- django/http/request.py | 19 ++++-- docs/ref/request-response.txt | 2 +- docs/releases/5.2.4.txt | 4 +- tests/requests_tests/test_accept_header.py | 78 +++++++++++++++++++++- 4 files changed, 94 insertions(+), 9 deletions(-) diff --git a/django/http/request.py b/django/http/request.py index c26a4954d1..daaf3748cc 100644 --- a/django/http/request.py +++ b/django/http/request.py @@ -90,7 +90,7 @@ class HttpRequest: @cached_property def accepted_types(self): - """Return a list of MediaType instances, in order of preference.""" + """Return a list of MediaType instances, in order of preference (quality).""" header_value = self.headers.get("Accept", "*/*") return sorted( ( @@ -98,19 +98,30 @@ class HttpRequest: for token in header_value.split(",") if token.strip() and (media_type := MediaType(token)).quality != 0 ), + key=operator.attrgetter("quality", "specificity"), + reverse=True, + ) + + @cached_property + def accepted_types_by_precedence(self): + """ + Return a list of MediaType instances, in order of precedence (specificity). + """ + return sorted( + self.accepted_types, key=operator.attrgetter("specificity", "quality"), reverse=True, ) def accepted_type(self, media_type): """ - Return the preferred MediaType instance which matches the given media type. + Return the MediaType instance which best matches the given media type. """ media_type = MediaType(media_type) return next( ( accepted_type - for accepted_type in self.accepted_types + for accepted_type in self.accepted_types_by_precedence if media_type.match(accepted_type) ), None, @@ -130,7 +141,7 @@ class HttpRequest: if not desired_types: return None - # Of the desired media types, select the one which is most desirable. + # Of the desired media types, select the one which is preferred. return min(desired_types, key=lambda t: self.accepted_types.index(t[0]))[1] def accepts(self, media_type): diff --git a/docs/ref/request-response.txt b/docs/ref/request-response.txt index c9fb05af1d..79419ea56e 100644 --- a/docs/ref/request-response.txt +++ b/docs/ref/request-response.txt @@ -478,7 +478,7 @@ Methods None (For further details on how content negotiation is performed, see - :rfc:`7231#section-5.3.2`.) + :rfc:`9110#section-12.5.1`.) Most browsers send ``Accept: */*`` by default, meaning they don't have a preference, in which case the first item in ``media_types`` would be diff --git a/docs/releases/5.2.4.txt b/docs/releases/5.2.4.txt index 028ef68290..ebfa556f23 100644 --- a/docs/releases/5.2.4.txt +++ b/docs/releases/5.2.4.txt @@ -9,4 +9,6 @@ Django 5.2.4 fixes several bugs in 5.2.3. Bugfixes ======== -* ... +* Fixed a regression in Django 5.2.2 where :meth:`HttpRequest.get_preferred_type() + ` incorrectly preferred more + specific media types with a lower quality (:ticket:`36447`). diff --git a/tests/requests_tests/test_accept_header.py b/tests/requests_tests/test_accept_header.py index 608af58b62..f6febc937a 100644 --- a/tests/requests_tests/test_accept_header.py +++ b/tests/requests_tests/test_accept_header.py @@ -170,6 +170,19 @@ class AcceptHeaderTests(TestCase): ) self.assertEqual( [str(accepted_type) for accepted_type in request.accepted_types], + [ + "text/html", + "application/xhtml+xml", + "text/*", + "application/xml; q=0.9", + "*/*; q=0.8", + ], + ) + self.assertEqual( + [ + str(accepted_type) + for accepted_type in request.accepted_types_by_precedence + ], [ "text/html", "application/xhtml+xml", @@ -196,7 +209,10 @@ class AcceptHeaderTests(TestCase): "text/*, text/plain, text/plain;format=flowed, */*" ) self.assertEqual( - [str(accepted_type) for accepted_type in request.accepted_types], + [ + str(accepted_type) + for accepted_type in request.accepted_types_by_precedence + ], [ "text/plain; format=flowed", "text/plain", @@ -261,6 +277,16 @@ class AcceptHeaderTests(TestCase): "text/*; q=0.8", ], ) + self.assertEqual( + [ + str(accepted_type) + for accepted_type in request.accepted_types_by_precedence + ], + [ + "text/html; q=0.8", + "text/*; q=0.8", + ], + ) def test_no_matching_accepted_type(self): request = HttpRequest() @@ -289,7 +315,7 @@ class AcceptHeaderTests(TestCase): ]: self.assertEqual(request.get_preferred_type(media_types), expected) - def test_quality(self): + def test_quality_for_media_type_rfc7231(self): """ Taken from https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2. """ @@ -314,7 +340,36 @@ class AcceptHeaderTests(TestCase): for media_types, expected in [ (["text/html", "text/html; level=1"], "text/html; level=1"), - (["text/html; level=2", "text/html; level=3"], "text/html; level=2"), + (["text/html; level=2", "text/html; level=3"], "text/html; level=3"), + ]: + self.assertEqual(request.get_preferred_type(media_types), expected) + + def test_quality_for_media_type_rfc9110(self): + """ + Taken from https://www.rfc-editor.org/rfc/rfc9110.html#section-12.5.1-18. + """ + request = HttpRequest() + request.META["HTTP_ACCEPT"] = ( + "text/*;q=0.3, text/plain;q=0.7, text/plain;format=flowed, " + "text/plain;format=fixed;q=0.4, */*;q=0.5" + ) + + for media_type, quality in [ + ("text/plain;format=flowed", 1), + ("text/plain", 0.7), + ("text/html", 0.3), + ("image/jpeg", 0.5), + ("text/plain;format=fixed", 0.4), + ("text/html;level=3", 0.3), # https://www.rfc-editor.org/errata/eid7138 + ]: + with self.subTest(media_type): + accepted_media_type = request.accepted_type(media_type) + self.assertIsNotNone(accepted_media_type) + self.assertEqual(accepted_media_type.quality, quality) + + for media_types, expected in [ + (["text/plain", "text/plain; format=flowed"], "text/plain; format=flowed"), + (["text/html", "image/jpeg"], "image/jpeg"), ]: self.assertEqual(request.get_preferred_type(media_types), expected) @@ -334,3 +389,20 @@ class AcceptHeaderTests(TestCase): self.assertEqual( request.get_preferred_type(["text/html", "text/plain"]), "text/html" ) + + def test_quality_over_specificity(self): + """ + For media types with the same quality, prefer the more specific type. + """ + request = HttpRequest() + request.META["HTTP_ACCEPT"] = "text/*,image/jpeg" + + self.assertEqual(request.accepted_type("text/plain").quality, 1) + self.assertEqual(request.accepted_type("text/plain").specificity, 1) + + self.assertEqual(request.accepted_type("image/jpeg").quality, 1) + self.assertEqual(request.accepted_type("image/jpeg").specificity, 2) + + self.assertEqual( + request.get_preferred_type(["text/plain", "image/jpeg"]), "image/jpeg" + )