launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:redirection-view-default-ports into launchpad:master.

Commit message:
Normalize default ports in RedirectionView.context

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1902893 in Launchpad itself: "API redirections don't work when haproxy is terminating HTTPS"
  https://bugs.launchpad.net/launchpad/+bug/1902893

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/393345

If haproxy is terminating HTTPS, then HTTP_HOST will likely be set to something like api.launchpad.net:443, resulting in corresponding redirection targets in some cases.  However, VirtualHostConfig.rooturl omits the port unless it's configured explicitly.  Cope with this by normalizing redirection targets before checking them against the set of supported root URLs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:redirection-view-default-ports into launchpad:master.

diff --git a/lib/lp/registry/browser/tests/test_distribution.py b/lib/lp/registry/browser/tests/test_distribution.py
index d47eb99..5407286 100644
--- a/lib/lp/registry/browser/tests/test_distribution.py
+++ b/lib/lp/registry/browser/tests/test_distribution.py
@@ -127,7 +127,11 @@ class TestDistributionNavigation(TestCaseWithFactory):
             distroseries.distribution.name, distroseries.name)
         self.assertDereferences(
             distroseries_url, distroseries,
-            environ={"HTTPS": "on", "SERVER_URL": None})
+            environ={
+                "HTTPS": "on",
+                "HTTP_HOST": "api.launchpad.test:443",
+                "SERVER_URL": None,
+                })
 
         # Objects subordinate to the redirected series work too.
         distroarchseries = self.factory.makeDistroArchSeries(
@@ -138,7 +142,11 @@ class TestDistributionNavigation(TestCaseWithFactory):
             distroarchseries.architecturetag)
         self.assertDereferences(
             distroarchseries_url, distroarchseries,
-            environ={"HTTPS": "on", "SERVER_URL": None})
+            environ={
+                "HTTPS": "on",
+                "HTTP_HOST": "api.launchpad.test:443",
+                "SERVER_URL": None,
+                })
 
 
 class TestDistributionPage(TestCaseWithFactory):
diff --git a/lib/lp/registry/browser/tests/test_product.py b/lib/lp/registry/browser/tests/test_product.py
index 192980c..22a4aec 100644
--- a/lib/lp/registry/browser/tests/test_product.py
+++ b/lib/lp/registry/browser/tests/test_product.py
@@ -178,7 +178,11 @@ class TestProductNavigation(TestCaseWithFactory):
             productseries.product.name, productseries.name)
         self.assertDereferences(
             productseries_url, productseries,
-            environ={"HTTPS": "on", "SERVER_URL": None})
+            environ={
+                "HTTPS": "on",
+                "HTTP_HOST": "api.launchpad.test:443",
+                "SERVER_URL": None,
+                })
 
         # Objects subordinate to the redirected series work too.
         productrelease = self.factory.makeProductRelease(
@@ -188,7 +192,11 @@ class TestProductNavigation(TestCaseWithFactory):
             productrelease.version)
         self.assertDereferences(
             productrelease_url, productrelease,
-            environ={"HTTPS": "on", "SERVER_URL": None})
+            environ={
+                "HTTPS": "on",
+                "HTTP_HOST": "api.launchpad.test:443",
+                "SERVER_URL": None,
+                })
 
 
 class TestProductConfiguration(BrowserTestCase):
diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
index 17c9d81..e0eb82e 100644
--- a/lib/lp/services/webapp/publisher.py
+++ b/lib/lp/services/webapp/publisher.py
@@ -1134,13 +1134,19 @@ class RedirectionView(URLDereferencingMixin):
             raise AttributeError(
                 "RedirectionView.context is not supported for URLs with "
                 "query strings.")
+        # Normalize default ports.
+        if (parsed_target.scheme, parsed_target.port) in {
+                ("http", 80), ("https", 443)}:
+            target_root = (parsed_target.scheme, parsed_target.hostname)
+        else:
+            target_root = (parsed_target.scheme, parsed_target.netloc)
         # This only supports the canonical root hostname/port for each site,
         # not any althostnames, but we assume that internally-generated
         # redirections always use the canonical hostname/port.
         supported_roots = {
             urlparse(section.rooturl)[:2]
             for section in allvhosts.configs.values()}
-        if parsed_target[:2] not in supported_roots:
+        if target_root not in supported_roots:
             raise AttributeError(
                 "RedirectionView.context is only supported for URLs served "
                 "by the main Launchpad application, not '%s'." % self.target)