Launchpad itself

Merge ~cjwatson/launchpad:redirection-view-default-ports into launchpad:master

  1. Git
  2. lp:~cjwatson/launchpad
  3. redirection-view-default-ports
  4. Merge into master
Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 8f93d6d103cd75adb6add55fadd2d37063f05761
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:redirection-view-default-ports
Merge into: launchpad:master
Diff against target: 85 lines (+27/-5)
3 files modified
lib/lp/registry/browser/tests/test_distribution.py (+10/-2)
lib/lp/registry/browser/tests/test_product.py (+10/-2)
lib/lp/services/webapp/publisher.py (+7/-1)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Review via email: mp+393345@code.launchpad.net

Commit message

Normalize default ports in RedirectionView.context

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/registry/browser/tests/test_distribution.py b/lib/lp/registry/browser/tests/test_distribution.py
2index d47eb99..5407286 100644
3--- a/lib/lp/registry/browser/tests/test_distribution.py
4+++ b/lib/lp/registry/browser/tests/test_distribution.py
5@@ -127,7 +127,11 @@ class TestDistributionNavigation(TestCaseWithFactory):
6 distroseries.distribution.name, distroseries.name)
7 self.assertDereferences(
8 distroseries_url, distroseries,
9- environ={"HTTPS": "on", "SERVER_URL": None})
10+ environ={
11+ "HTTPS": "on",
12+ "HTTP_HOST": "api.launchpad.test:443",
13+ "SERVER_URL": None,
14+ })
15
16 # Objects subordinate to the redirected series work too.
17 distroarchseries = self.factory.makeDistroArchSeries(
18@@ -138,7 +142,11 @@ class TestDistributionNavigation(TestCaseWithFactory):
19 distroarchseries.architecturetag)
20 self.assertDereferences(
21 distroarchseries_url, distroarchseries,
22- environ={"HTTPS": "on", "SERVER_URL": None})
23+ environ={
24+ "HTTPS": "on",
25+ "HTTP_HOST": "api.launchpad.test:443",
26+ "SERVER_URL": None,
27+ })
28
29
30 class TestDistributionPage(TestCaseWithFactory):
31diff --git a/lib/lp/registry/browser/tests/test_product.py b/lib/lp/registry/browser/tests/test_product.py
32index 192980c..22a4aec 100644
33--- a/lib/lp/registry/browser/tests/test_product.py
34+++ b/lib/lp/registry/browser/tests/test_product.py
35@@ -178,7 +178,11 @@ class TestProductNavigation(TestCaseWithFactory):
36 productseries.product.name, productseries.name)
37 self.assertDereferences(
38 productseries_url, productseries,
39- environ={"HTTPS": "on", "SERVER_URL": None})
40+ environ={
41+ "HTTPS": "on",
42+ "HTTP_HOST": "api.launchpad.test:443",
43+ "SERVER_URL": None,
44+ })
45
46 # Objects subordinate to the redirected series work too.
47 productrelease = self.factory.makeProductRelease(
48@@ -188,7 +192,11 @@ class TestProductNavigation(TestCaseWithFactory):
49 productrelease.version)
50 self.assertDereferences(
51 productrelease_url, productrelease,
52- environ={"HTTPS": "on", "SERVER_URL": None})
53+ environ={
54+ "HTTPS": "on",
55+ "HTTP_HOST": "api.launchpad.test:443",
56+ "SERVER_URL": None,
57+ })
58
59
60 class TestProductConfiguration(BrowserTestCase):
61diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
62index 17c9d81..e0eb82e 100644
63--- a/lib/lp/services/webapp/publisher.py
64+++ b/lib/lp/services/webapp/publisher.py
65@@ -1134,13 +1134,19 @@ class RedirectionView(URLDereferencingMixin):
66 raise AttributeError(
67 "RedirectionView.context is not supported for URLs with "
68 "query strings.")
69+ # Normalize default ports.
70+ if (parsed_target.scheme, parsed_target.port) in {
71+ ("http", 80), ("https", 443)}:
72+ target_root = (parsed_target.scheme, parsed_target.hostname)
73+ else:
74+ target_root = (parsed_target.scheme, parsed_target.netloc)
75 # This only supports the canonical root hostname/port for each site,
76 # not any althostnames, but we assume that internally-generated
77 # redirections always use the canonical hostname/port.
78 supported_roots = {
79 urlparse(section.rooturl)[:2]
80 for section in allvhosts.configs.values()}
81- if parsed_target[:2] not in supported_roots:
82+ if target_root not in supported_roots:
83 raise AttributeError(
84 "RedirectionView.context is only supported for URLs served "
85 "by the main Launchpad application, not '%s'." % self.target)

Subscribers

People subscribed via source and target branches

to status/vote changes: