Don't skip normal locale negotiation if Accept-Language is set (#17968)

* Add failing test

* Accept-Language shouldn't bypass local negotiation

This should more closely mimic https://github.com/pypi/infra/blob/d5374e6e92f5a09729d649a0a587935d19f2d76f/terraform/warehouse/vcl/main.vcl#L160-L174

* Update test to use AcceptLanguageValidHeader

* Add failing test for Accept-Language fallback

* Don't renegotiate possibly unknown default locale

Currently if the default locale negotiation fails, we fall back to
trying to match based on Accept-Language. If this fails, it falls back
on the default value, but because we re-negotiate the locale to
determine the default value (without determining if the default is a
known locale) we could potentially still set a garbage locale.

Since the default for the default_match in best_match is already `None`,
we can just use the default instead of re-negotiating.

Author: di

tests/unit/i18n/test_init.py

@@ -15,6 +15,7 @@
 
 from pyramid import viewderivers
 from pyramid.i18n import Localizer
+from webob.acceptparse import AcceptLanguageValidHeader
 
 from warehouse import i18n
 from warehouse.i18n.extensions import FallbackInternationalizationExtension
@@ -120,11 +121,9 @@ def test_when_locale_is_missing(monkeypatch):
             pretend.stub(
                 params={},
                 cookies={},
-                accept_language=pretend.stub(
-                    best_match=lambda *a, **kw: "fake-locale-best-match"
-                ),
+                accept_language=AcceptLanguageValidHeader(header_value="eo"),
             ),
-            "fake-locale-best-match",
+            "eo",
         ),
         (
             pretend.stub(
@@ -144,6 +143,20 @@ def test_when_locale_is_missing(monkeypatch):
             ),
             None,
         ),
+        (
+            pretend.stub(
+                _LOCALE_="he",
+                accept_language=AcceptLanguageValidHeader(header_value="eo"),
+            ),
+            "he",
+        ),
+        (
+            pretend.stub(
+                _LOCALE_="garbage",
+                accept_language=AcceptLanguageValidHeader(header_value="xx"),
+            ),
+            None,
+        ),
     ],
 )
 def test_negotiate_locale(monkeypatch, req, expected):

warehouse/i18n/__init__.py

@@ -80,15 +80,12 @@ def _locale(request):
 
 
 def _negotiate_locale(request):
-    if not request.accept_language:
-        locale_name = default_locale_negotiator(request)
-        if locale_name in KNOWN_LOCALES:
-            return locale_name
-    else:
-        return request.accept_language.best_match(
-            tuple(KNOWN_LOCALES.keys()),
-            default_match=default_locale_negotiator(request),
-        )
+    locale_name = default_locale_negotiator(request)
+    if locale_name in KNOWN_LOCALES:
+        return locale_name
+
+    if request.accept_language:
+        return request.accept_language.best_match(tuple(KNOWN_LOCALES.keys()))
 
     return None