Skip to content

Commit 4de4edf

Browse files
RealOrangeOnesarahboyce
authored andcommitted
[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 c075508. Thank you to Anders Kaseorg for the report. Backport of 12c1557 from main.
1 parent f5cc6a8 commit 4de4edf

4 files changed

Lines changed: 94 additions & 9 deletions

File tree

django/http/request.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,27 +90,38 @@ def headers(self):
9090

9191
@cached_property
9292
def accepted_types(self):
93-
"""Return a list of MediaType instances, in order of preference."""
93+
"""Return a list of MediaType instances, in order of preference (quality)."""
9494
header_value = self.headers.get("Accept", "*/*")
9595
return sorted(
9696
(
9797
media_type
9898
for token in header_value.split(",")
9999
if token.strip() and (media_type := MediaType(token)).quality != 0
100100
),
101+
key=operator.attrgetter("quality", "specificity"),
102+
reverse=True,
103+
)
104+
105+
@cached_property
106+
def accepted_types_by_precedence(self):
107+
"""
108+
Return a list of MediaType instances, in order of precedence (specificity).
109+
"""
110+
return sorted(
111+
self.accepted_types,
101112
key=operator.attrgetter("specificity", "quality"),
102113
reverse=True,
103114
)
104115

105116
def accepted_type(self, media_type):
106117
"""
107-
Return the preferred MediaType instance which matches the given media type.
118+
Return the MediaType instance which best matches the given media type.
108119
"""
109120
media_type = MediaType(media_type)
110121
return next(
111122
(
112123
accepted_type
113-
for accepted_type in self.accepted_types
124+
for accepted_type in self.accepted_types_by_precedence
114125
if media_type.match(accepted_type)
115126
),
116127
None,
@@ -130,7 +141,7 @@ def get_preferred_type(self, media_types):
130141
if not desired_types:
131142
return None
132143

133-
# Of the desired media types, select the one which is most desirable.
144+
# Of the desired media types, select the one which is preferred.
134145
return min(desired_types, key=lambda t: self.accepted_types.index(t[0]))[1]
135146

136147
def accepts(self, media_type):

docs/ref/request-response.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,7 @@ Methods
478478
None
479479

480480
(For further details on how content negotiation is performed, see
481-
:rfc:`7231#section-5.3.2`.)
481+
:rfc:`9110#section-12.5.1`.)
482482

483483
Most browsers send ``Accept: */*`` by default, meaning they don't have a
484484
preference, in which case the first item in ``media_types`` would be

docs/releases/5.2.4.txt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,6 @@ Django 5.2.4 fixes several bugs in 5.2.3.
99
Bugfixes
1010
========
1111

12-
* ...
12+
* Fixed a regression in Django 5.2.2 where :meth:`HttpRequest.get_preferred_type()
13+
<django.http.HttpRequest.get_preferred_type>` incorrectly preferred more
14+
specific media types with a lower quality (:ticket:`36447`).

tests/requests_tests/test_accept_header.py

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,19 @@ def test_accept_headers(self):
170170
)
171171
self.assertEqual(
172172
[str(accepted_type) for accepted_type in request.accepted_types],
173+
[
174+
"text/html",
175+
"application/xhtml+xml",
176+
"text/*",
177+
"application/xml; q=0.9",
178+
"*/*; q=0.8",
179+
],
180+
)
181+
self.assertEqual(
182+
[
183+
str(accepted_type)
184+
for accepted_type in request.accepted_types_by_precedence
185+
],
173186
[
174187
"text/html",
175188
"application/xhtml+xml",
@@ -196,7 +209,10 @@ def test_precedence(self):
196209
"text/*, text/plain, text/plain;format=flowed, */*"
197210
)
198211
self.assertEqual(
199-
[str(accepted_type) for accepted_type in request.accepted_types],
212+
[
213+
str(accepted_type)
214+
for accepted_type in request.accepted_types_by_precedence
215+
],
200216
[
201217
"text/plain; format=flowed",
202218
"text/plain",
@@ -261,6 +277,16 @@ def test_accept_header_priority_overlapping_mime(self):
261277
"text/*; q=0.8",
262278
],
263279
)
280+
self.assertEqual(
281+
[
282+
str(accepted_type)
283+
for accepted_type in request.accepted_types_by_precedence
284+
],
285+
[
286+
"text/html; q=0.8",
287+
"text/*; q=0.8",
288+
],
289+
)
264290

265291
def test_no_matching_accepted_type(self):
266292
request = HttpRequest()
@@ -289,7 +315,7 @@ def test_accept_with_param(self):
289315
]:
290316
self.assertEqual(request.get_preferred_type(media_types), expected)
291317

292-
def test_quality(self):
318+
def test_quality_for_media_type_rfc7231(self):
293319
"""
294320
Taken from https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2.
295321
"""
@@ -314,7 +340,36 @@ def test_quality(self):
314340

315341
for media_types, expected in [
316342
(["text/html", "text/html; level=1"], "text/html; level=1"),
317-
(["text/html; level=2", "text/html; level=3"], "text/html; level=2"),
343+
(["text/html; level=2", "text/html; level=3"], "text/html; level=3"),
344+
]:
345+
self.assertEqual(request.get_preferred_type(media_types), expected)
346+
347+
def test_quality_for_media_type_rfc9110(self):
348+
"""
349+
Taken from https://www.rfc-editor.org/rfc/rfc9110.html#section-12.5.1-18.
350+
"""
351+
request = HttpRequest()
352+
request.META["HTTP_ACCEPT"] = (
353+
"text/*;q=0.3, text/plain;q=0.7, text/plain;format=flowed, "
354+
"text/plain;format=fixed;q=0.4, */*;q=0.5"
355+
)
356+
357+
for media_type, quality in [
358+
("text/plain;format=flowed", 1),
359+
("text/plain", 0.7),
360+
("text/html", 0.3),
361+
("image/jpeg", 0.5),
362+
("text/plain;format=fixed", 0.4),
363+
("text/html;level=3", 0.3), # https://www.rfc-editor.org/errata/eid7138
364+
]:
365+
with self.subTest(media_type):
366+
accepted_media_type = request.accepted_type(media_type)
367+
self.assertIsNotNone(accepted_media_type)
368+
self.assertEqual(accepted_media_type.quality, quality)
369+
370+
for media_types, expected in [
371+
(["text/plain", "text/plain; format=flowed"], "text/plain; format=flowed"),
372+
(["text/html", "image/jpeg"], "image/jpeg"),
318373
]:
319374
self.assertEqual(request.get_preferred_type(media_types), expected)
320375

@@ -334,3 +389,20 @@ def test_quality_breaks_specificity(self):
334389
self.assertEqual(
335390
request.get_preferred_type(["text/html", "text/plain"]), "text/html"
336391
)
392+
393+
def test_quality_over_specificity(self):
394+
"""
395+
For media types with the same quality, prefer the more specific type.
396+
"""
397+
request = HttpRequest()
398+
request.META["HTTP_ACCEPT"] = "text/*,image/jpeg"
399+
400+
self.assertEqual(request.accepted_type("text/plain").quality, 1)
401+
self.assertEqual(request.accepted_type("text/plain").specificity, 1)
402+
403+
self.assertEqual(request.accepted_type("image/jpeg").quality, 1)
404+
self.assertEqual(request.accepted_type("image/jpeg").specificity, 2)
405+
406+
self.assertEqual(
407+
request.get_preferred_type(["text/plain", "image/jpeg"]), "image/jpeg"
408+
)

0 commit comments

Comments
 (0)