launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06374
[Merge] lp:~wgrant/launchpad/bug-932518 into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bug-932518 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #932518 in Launchpad itself: "Downloads are served over insecure HTTP"
https://bugs.launchpad.net/launchpad/+bug/932518
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-932518/+merge/93127
This branch moves ProductReleaseFile downloads back to HTTPS, reverting the fix for bug #174186. The world is a bit more HTTPS-friendly 4 years later.
The PRF TALES formatter no longer forces links to HTTP, and the librarian redirector no longer respects the X-SCHEME header set by our Apache config.
--
https://code.launchpad.net/~wgrant/launchpad/bug-932518/+merge/93127
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-932518 into lp:launchpad.
=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py 2012-02-01 15:26:32 +0000
+++ lib/lp/app/browser/tales.py 2012-02-15 04:18:18 +0000
@@ -1581,8 +1581,7 @@
url = urlappend(canonical_url(self._release), '+download')
# Quote the filename to eliminate non-ascii characters which
# are invalid in the url.
- url = urlappend(url, urllib.quote(lfa.filename.encode('utf-8')))
- return str(URI(url).replace(scheme='http'))
+ return urlappend(url, urllib.quote(lfa.filename.encode('utf-8')))
class BranchFormatterAPI(ObjectFormatterAPI):
=== modified file 'lib/lp/registry/stories/product/xx-product-files.txt'
--- lib/lp/registry/stories/product/xx-product-files.txt 2012-01-15 11:06:57 +0000
+++ lib/lp/registry/stories/product/xx-product-files.txt 2012-02-15 04:18:18 +0000
@@ -480,85 +480,3 @@
>>> firefox_owner.getControl('Delete Files').click()
>>> get_feedback_messages(firefox_owner.contents)
[u'1 file has been deleted.']
-
-
-Scheme redirection
-==================
-
-Files may be downloaded over http or https with the URL for the
-librarian file constructed based upon the scheme of the original
-request. The Apache configuration for Launchpad production servers
-sets a header called 'X-SCHEME' to indicate the original scheme
-(http or https).
-
-If the configuration 'use_https' is False then http will be used
-regardless of the value of X-SCHEME. In order to exercise the
-use of X-SCHEME we must set 'use_https' to True and remember
-to reset it when we're done.
-
- >>> from lp.services.config import config
- >>> use_https_true_data = """
- ... [librarian]
- ... use_https: True
- ... """
- >>> config.push('use_https_true_data', use_https_true_data)
-
-If the original scheme is https the redirect URL should be https too.
-
- >>> from textwrap import dedent
- >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
- >>> link = firefox_owner.getLink('foo.txt')
- >>> url_path = urlparse(link.url)[2]
- >>> redirect_resp = http(dedent("""\
- ... GET %s HTTP/1.1
- ... X-SCHEME: https
- ... """ % url_path))
- >>> location = redirect_resp.getOutput().split("\n")[3]
- >>> redirect_url = location.split()[1]
- >>> print redirect_url
- https://.../foo.txt
-
-However if the original scheme is http then the redirect URL should be
-over http.
-
- >>> redirect_resp = http(dedent("""\
- ... GET %s HTTP/1.1
- ... X-SCHEME: http
- ... """ % url_path))
- >>> location = redirect_resp.getOutput().split("\n")[3]
- >>> redirect_url = location.split()[1]
- >>> print redirect_url
- http://.../foo.txt
-
-When 'use_https' is False the result will always be http.
-
- >>> config_data = config.pop('use_https_true_data')
- >>> use_https_false_data = """
- ... [vhosts]
- ... use_https: False
- ... """
- >>> config.push('use_https_false_data', use_https_false_data)
- >>> firefox_owner.open('http://launchpad.dev/firefox/+download')
- >>> link = firefox_owner.getLink('foo.txt')
- >>> url_path = urlparse(link.url)[2]
- >>> redirect_resp = http(dedent("""\
- ... GET %s HTTP/1.1
- ... X-SCHEME: https
- ... """ % url_path))
- >>> location = redirect_resp.getOutput().split("\n")[3]
- >>> redirect_url = location.split()[1]
- >>> print redirect_url
- http://.../foo.txt
-
- >>> redirect_resp = http(dedent("""\
- ... GET %s HTTP/1.1
- ... X-SCHEME: http
- ... """ % url_path))
- >>> location = redirect_resp.getOutput().split("\n")[3]
- >>> redirect_url = location.split()[1]
- >>> print redirect_url
- http://.../foo.txt
-
-Return 'use_https' to its original value to not mess up future tests.
-
- >>> config_data = config.pop('use_https_false_data')
=== modified file 'lib/lp/services/librarian/browser.py'
--- lib/lp/services/librarian/browser.py 2011-12-30 01:10:37 +0000
+++ lib/lp/services/librarian/browser.py 2012-02-15 04:18:18 +0000
@@ -49,14 +49,7 @@
# If the LFA is deleted, throw a 410.
if self.context.deleted:
raise GoneError("File deleted.")
- # Redirect based on the scheme of the request, as set by
- # Apache in the 'X-SCHEME' environment variable, which is
- # mapped to 'HTTP_X_SCHEME. Note that only some requests
- # for librarian files are allowed to come in via http as
- # most are forced to https via Apache redirection.
- self.request.response.redirect(
- self.context.getURL(
- secure=self.request.get('HTTP_X_SCHEME') != 'http'))
+ self.request.response.redirect(self.context.getURL())
class LibraryFileAliasMD5View(LaunchpadView):
=== modified file 'lib/lp/services/librarian/doc/librarian.txt'
--- lib/lp/services/librarian/doc/librarian.txt 2011-12-30 06:14:56 +0000
+++ lib/lp/services/librarian/doc/librarian.txt 2012-02-15 04:18:18 +0000
@@ -130,29 +130,6 @@
... ) is not None
True
-However, we can force the use of HTTP by setting the 'HTTP_X_SCHEME'
-header in the request to 'http', even when 'use_https' is True.
-
- >>> from zope.component import getMultiAdapter
- >>> from lp.services.webapp.servers import LaunchpadTestRequest
- >>> from urlparse import urlparse
- >>> request = LaunchpadTestRequest(
- ... environ={'REQUEST_METHOD': 'GET', 'HTTP_X_SCHEME': 'http'})
- >>> view = getMultiAdapter((alias,request), name='+index')
- >>> view.initialize()
- >>> print urlparse(request.response.getHeader('Location'))[0]
- http
-
-When the incoming scheme is 'https' then the redirect scheme is
-unaffected.
-
- >>> request = LaunchpadTestRequest(
- ... environ={'REQUEST_METHOD': 'GET', 'HTTP_X_SCHEME': 'https'})
- >>> view = getMultiAdapter((alias,request), name='+index')
- >>> view.initialize()
- >>> print urlparse(request.response.getHeader('Location'))[0]
- https
-
Reset 'use_https' to its original state.
>>> test_config_data = config.pop('test')
@@ -371,6 +348,7 @@
temporary measure until we're sure no restricted files leak into the
traversal hierarchy.
+ >>> from zope.component import getMultiAdapter
>>> view = getMultiAdapter((file_alias, request), name='+index')
>>> view.initialize()
Traceback (most recent call last):
@@ -568,7 +546,6 @@
A librarian file has a default view that should redirect to the download
URL.
- >>> from zope.component import getMultiAdapter
>>> from lp.services.webapp.servers import LaunchpadTestRequest
>>> req = LaunchpadTestRequest()
>>> alias = lfas.create(
=== modified file 'lib/lp/services/librarian/tests/test_libraryfilealiasview.py'
--- lib/lp/services/librarian/tests/test_libraryfilealiasview.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/librarian/tests/test_libraryfilealiasview.py 2012-02-15 04:18:18 +0000
@@ -23,8 +23,7 @@
lfa = self.factory.makeLibraryFileAlias()
removeSecurityProxy(lfa).content = None
self.assertTrue(lfa.deleted)
- request = LaunchpadTestRequest(
- environ={'REQUEST_METHOD': 'GET', 'HTTP_X_SCHEME' : 'http' })
+ request = LaunchpadTestRequest()
view = getMultiAdapter((lfa, request), name='+index')
self.assertRaisesWithContent(
GoneError, "'File deleted.'", view.initialize)
