Settle 404 pages (#17811)

* Don't pre-compute the canonical URL

Turns out content within Jinja blocks will get executed even if they are
later replaced with another block. Additionally, if template rendering
fails for a exception view, it falls back to a plain status response.

Instead, use a request method to try and compute the canonical URL only
if one was not provided, and gracefully handle the case where the
canonical URL can't be constructed by not including a block at all.

* Use request.canonical_url to simplify

* Update translations

* Add tests

Author: di

tests/unit/test_filters.py

@@ -314,3 +314,25 @@ def test_remove_invalid_xml_unicode(inp, expected):
     Test that invalid XML unicode characters are removed.
     """
     assert filters.remove_invalid_xml_unicode(inp) == expected
+
+
+def test_canonical_url():
+    request = pretend.stub(
+        matched_route=pretend.stub(name="foo"),
+        route_url=pretend.call_recorder(lambda a: "bar"),
+    )
+    assert filters._canonical_url(request) == "bar"
+    assert request.route_url.calls == [pretend.call("foo")]
+
+
+def test_canonical_url_no_matched_route():
+    request = pretend.stub(matched_route=None)
+    assert filters._canonical_url(request) is None
+
+
+def test_canonical_url_missing_kwargs():
+    request = pretend.stub(
+        matched_route=pretend.stub(name="foo"),
+        route_url=pretend.raiser(KeyError),
+    )
+    assert filters._canonical_url(request) is None

warehouse/filters.py

@@ -203,5 +203,14 @@ def remove_invalid_xml_unicode(value: str | None) -> str | None:
     return "".join(c for c in value if ord(c) >= 32) if value else value
 
 
+def _canonical_url(request, **kwargs):
+    if request.matched_route:
+        try:
+            return request.route_url(request.matched_route.name, **kwargs)
+        except KeyError:
+            pass
+
+
 def includeme(config):
     config.add_request_method(_camo_url, name="camo_url")
+    config.add_request_method(_canonical_url, name="canonical_url")

warehouse/locale/messages.pot

@@ -948,14 +948,14 @@ msgstr ""
 #: warehouse/templates/404.html:34 warehouse/templates/500.html:28
 #: warehouse/templates/500.html:29
 #: warehouse/templates/accounts/two-factor.html:55
-#: warehouse/templates/base.html:280 warehouse/templates/base.html:281
-#: warehouse/templates/base.html:282 warehouse/templates/base.html:283
-#: warehouse/templates/base.html:293 warehouse/templates/base.html:294
-#: warehouse/templates/base.html:307 warehouse/templates/base.html:308
-#: warehouse/templates/base.html:310 warehouse/templates/base.html:319
-#: warehouse/templates/base.html:321 warehouse/templates/base.html:322
-#: warehouse/templates/base.html:323 warehouse/templates/base.html:333
-#: warehouse/templates/base.html:346
+#: warehouse/templates/base.html:285 warehouse/templates/base.html:286
+#: warehouse/templates/base.html:287 warehouse/templates/base.html:288
+#: warehouse/templates/base.html:298 warehouse/templates/base.html:299
+#: warehouse/templates/base.html:312 warehouse/templates/base.html:313
+#: warehouse/templates/base.html:315 warehouse/templates/base.html:324
+#: warehouse/templates/base.html:326 warehouse/templates/base.html:327
+#: warehouse/templates/base.html:328 warehouse/templates/base.html:338
+#: warehouse/templates/base.html:351
 #: warehouse/templates/includes/accounts/profile-callout.html:18
 #: warehouse/templates/includes/file-details.html:101
 #: warehouse/templates/index.html:100 warehouse/templates/index.html:104
@@ -1111,7 +1111,7 @@ msgid "Main navigation"
 msgstr ""
 
 #: warehouse/templates/base.html:41 warehouse/templates/base.html:55
-#: warehouse/templates/base.html:277
+#: warehouse/templates/base.html:282
 #: warehouse/templates/includes/current-user-indicator.html:63
 #: warehouse/templates/pages/help.html:119
 #: warehouse/templates/pages/sitemap.html:27
@@ -1183,16 +1183,16 @@ msgstr ""
 msgid "RSS: 40 newest packages"
 msgstr ""
 
-#: warehouse/templates/base.html:162
+#: warehouse/templates/base.html:167
 msgid "Skip to main content"
 msgstr ""
 
-#: warehouse/templates/base.html:165
+#: warehouse/templates/base.html:170
 msgid "Switch to mobile version"
 msgstr ""
 
-#: warehouse/templates/base.html:174 warehouse/templates/base.html:183
-#: warehouse/templates/base.html:193
+#: warehouse/templates/base.html:179 warehouse/templates/base.html:188
+#: warehouse/templates/base.html:198
 #: warehouse/templates/includes/flash-messages.html:30
 #: warehouse/templates/includes/session-notifications.html:20
 #: warehouse/templates/manage/account.html:843
@@ -1210,174 +1210,174 @@ msgstr ""
 msgid "Warning"
 msgstr ""
 
-#: warehouse/templates/base.html:176
+#: warehouse/templates/base.html:181
 msgid "You are using an unsupported browser, upgrade to a newer version."
 msgstr ""
 
-#: warehouse/templates/base.html:185
+#: warehouse/templates/base.html:190
 msgid ""
 "You are using TestPyPI – a separate instance of the Python Package Index "
 "that allows you to try distribution tools and processes without affecting"
 " the real index."
 msgstr ""
 
-#: warehouse/templates/base.html:195
+#: warehouse/templates/base.html:200
 msgid ""
 "Some features may not work without JavaScript. Please try enabling it if "
 "you encounter problems."
 msgstr ""
 
-#: warehouse/templates/base.html:233 warehouse/templates/base.html:254
+#: warehouse/templates/base.html:238 warehouse/templates/base.html:259
 #: warehouse/templates/error-base-with-search.html:20
 #: warehouse/templates/index.html:43
 msgid "Search PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:234 warehouse/templates/base.html:255
+#: warehouse/templates/base.html:239 warehouse/templates/base.html:260
 #: warehouse/templates/error-base-with-search.html:21
 #: warehouse/templates/index.html:46
 msgid "Search projects"
 msgstr ""
 
-#: warehouse/templates/base.html:238 warehouse/templates/base.html:259
+#: warehouse/templates/base.html:243 warehouse/templates/base.html:264
 #: warehouse/templates/error-base-with-search.html:24
 #: warehouse/templates/index.html:50
 msgid "Search"
 msgstr ""
 
-#: warehouse/templates/base.html:278
+#: warehouse/templates/base.html:283
 msgid "Help navigation"
 msgstr ""
 
-#: warehouse/templates/base.html:280
+#: warehouse/templates/base.html:285
 msgid "Installing packages"
 msgstr ""
 
-#: warehouse/templates/base.html:281
+#: warehouse/templates/base.html:286
 msgid "Uploading packages"
 msgstr ""
 
-#: warehouse/templates/base.html:282
+#: warehouse/templates/base.html:287
 msgid "User guide"
 msgstr ""
 
-#: warehouse/templates/base.html:283
+#: warehouse/templates/base.html:288
 msgid "Project name retention"
 msgstr ""
 
-#: warehouse/templates/base.html:284
+#: warehouse/templates/base.html:289
 msgid "FAQs"
 msgstr ""
 
-#: warehouse/templates/base.html:290 warehouse/templates/pages/sitemap.html:37
+#: warehouse/templates/base.html:295 warehouse/templates/pages/sitemap.html:37
 msgid "About PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:291
+#: warehouse/templates/base.html:296
 msgid "About PyPI navigation"
 msgstr ""
 
-#: warehouse/templates/base.html:293
+#: warehouse/templates/base.html:298
 msgid "PyPI Blog"
 msgstr ""
 
-#: warehouse/templates/base.html:294
+#: warehouse/templates/base.html:299
 msgid "Infrastructure dashboard"
 msgstr ""
 
-#: warehouse/templates/base.html:295 warehouse/templates/pages/sitemap.html:40
+#: warehouse/templates/base.html:300 warehouse/templates/pages/sitemap.html:40
 #: warehouse/templates/pages/stats.html:16
 msgid "Statistics"
 msgstr ""
 
-#: warehouse/templates/base.html:296
+#: warehouse/templates/base.html:301
 msgid "Logos & trademarks"
 msgstr ""
 
-#: warehouse/templates/base.html:297
+#: warehouse/templates/base.html:302
 msgid "Our sponsors"
 msgstr ""
 
-#: warehouse/templates/base.html:303
+#: warehouse/templates/base.html:308
 msgid "Contributing to PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:304
+#: warehouse/templates/base.html:309
 msgid "How to contribute navigation"
 msgstr ""
 
-#: warehouse/templates/base.html:306
+#: warehouse/templates/base.html:311
 msgid "Bugs and feedback"
 msgstr ""
 
-#: warehouse/templates/base.html:307
+#: warehouse/templates/base.html:312
 msgid "Contribute on GitHub"
 msgstr ""
 
-#: warehouse/templates/base.html:308
+#: warehouse/templates/base.html:313
 msgid "Translate PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:309
+#: warehouse/templates/base.html:314
 msgid "Sponsor PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:310
+#: warehouse/templates/base.html:315
 msgid "Development credits"
 msgstr ""
 
-#: warehouse/templates/base.html:316 warehouse/templates/pages/sitemap.html:23
+#: warehouse/templates/base.html:321 warehouse/templates/pages/sitemap.html:23
 msgid "Using PyPI"
 msgstr ""
 
-#: warehouse/templates/base.html:317
+#: warehouse/templates/base.html:322
 msgid "Using PyPI navigation"
 msgstr ""
 
-#: warehouse/templates/base.html:319
+#: warehouse/templates/base.html:324
 #: warehouse/templates/manage/organization/activate_subscription.html:33
 msgid "Terms of Service"
 msgstr ""
 
-#: warehouse/templates/base.html:320
+#: warehouse/templates/base.html:325
 msgid "Report security issue"
 msgstr ""
 
-#: warehouse/templates/base.html:321
+#: warehouse/templates/base.html:326
 msgid "Code of conduct"
 msgstr ""
 
-#: warehouse/templates/base.html:322
+#: warehouse/templates/base.html:327
 msgid "Privacy Notice"
 msgstr ""
 
-#: warehouse/templates/base.html:323
+#: warehouse/templates/base.html:328
 msgid "Acceptable Use Policy"
 msgstr ""
 
-#: warehouse/templates/base.html:333
+#: warehouse/templates/base.html:338
 msgid "Status:"
 msgstr ""
 
-#: warehouse/templates/base.html:334
+#: warehouse/templates/base.html:339
 msgid "all systems operational"
 msgstr ""
 
-#: warehouse/templates/base.html:338
+#: warehouse/templates/base.html:343
 msgid ""
 "Developed and maintained by the Python community, for the Python "
 "community."
 msgstr ""
 
-#: warehouse/templates/base.html:340
+#: warehouse/templates/base.html:345
 msgid "Donate today!"
 msgstr ""
 
-#: warehouse/templates/base.html:347 warehouse/templates/pages/sitemap.html:16
+#: warehouse/templates/base.html:352 warehouse/templates/pages/sitemap.html:16
 msgid "Site map"
 msgstr ""
 
-#: warehouse/templates/base.html:353
+#: warehouse/templates/base.html:358
 msgid "Switch to desktop version"
 msgstr ""
 

warehouse/templates/accounts/profile.html

@@ -21,7 +21,7 @@
 {% endif %}
 {% endblock %}
 
-{% block canonical_url %}{{ request.route_url('accounts.profile', username=user.username) }}{% endblock %}
+{% block canonical_url %}{{ request.canonical_url(username=user.username) }}{% endblock %}
 
 {% block content %}
 <div class="horizontal-section horizontal-section--medium">

warehouse/templates/base.html

@@ -107,10 +107,15 @@
     <link rel="alternate" type="application/rss+xml" title="{% trans %}RSS: 40 newest packages{% endtrans %}" href="{{ request.route_path('rss.packages') }}">
     {% block additional_rss %}{% endblock %}
     {# Allow overriding the canonical URL if we have explicitly set it.
-     # Otherwise, use the URL to the current matched route as the canonical URL,
+     # Otherwise, try to use the URL to the current matched route as the canonical URL,
      # which strips anchor tags and query parameters.
+     # If that fails, don't include a canonical URL
      #}
-    <link rel="canonical" href="{% block canonical_url %}{{ request.route_url(request.matched_route.name) }}{% endblock %}">
+    {% if self.canonical_url() %}
+    <link rel="canonical" href="{% block canonical_url %}{% endblock %}">
+    {% elif request.canonical_url() %}
+    <link rel="canonical" href="{{ request.canonical_url() }}">
+    {% endif %}
 
     <meta property="og:url" content="{% if request.matched_route %}{{ request.current_route_url() }}{% else %}{{ request.url }}{% endif %}">
     <meta property="og:site_name" content="PyPI">

warehouse/templates/organizations/profile.html

@@ -15,7 +15,7 @@
 
 {% block title %}{% trans orgname=organization.display_name %}Profile of {{ orgname }}{% endtrans %}{% endblock %}
 
-{% block canonical_url %}{{ request.route_url('organizations.profile', organization=organization.normalized_name) }}{% endblock %}
+{% block canonical_url %}{{ request.canonical_url(organization=organization.normalized_name) }}{% endblock %}
 
 {% block content %}
 <div class="horizontal-section horizontal-section--medium">