launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/unquote-urls-for-dmp into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/unquote-urls-for-dmp/+merge/161974

Decode URLs before we check redirection in distributionmirror_prober.
-- 
https://code.launchpad.net/~stevenk/launchpad/unquote-urls-for-dmp/+merge/161974
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/unquote-urls-for-dmp into lp:launchpad.

=== modified file 'lib/lp/registry/scripts/distributionmirror_prober.py'
--- lib/lp/registry/scripts/distributionmirror_prober.py	2012-07-02 15:47:19 +0000
+++ lib/lp/registry/scripts/distributionmirror_prober.py	2013-05-01 22:35:31 +0000
@@ -12,6 +12,7 @@
 import logging
 import os
 from StringIO import StringIO
+import urllib
 import urllib2
 import urlparse
 
@@ -318,7 +319,8 @@
 
         scheme, host, port, orig_path = _parse(self.url)
         scheme, host, port, new_path = _parse(url)
-        if orig_path.split('/')[-1] != new_path.split('/')[-1]:
+        if (urllib.unquote(orig_path.split('/')[-1])
+            != urllib.unquote(new_path.split('/')[-1])):
             # Server redirected us to a file which doesn't seem to be what we
             # requested.  It's likely to be a stupid server which redirects
             # instead of 404ing (https://launchpad.net/bugs/204460).

=== modified file 'lib/lp/registry/tests/test_distributionmirror_prober.py'
--- lib/lp/registry/tests/test_distributionmirror_prober.py	2013-04-17 08:23:31 +0000
+++ lib/lp/registry/tests/test_distributionmirror_prober.py	2013-05-01 22:35:31 +0000
@@ -577,6 +577,17 @@
         prober.redirect('http://foo.bar/baz/boo/notfound?file=package.deb')
         self.failUnless(prober.has_failed)
 
+    def test_does_not_raise_if_redirected_to_reencoded_file(self):
+        prober = self._createFactoryAndStubConnectAndTimeoutCall(
+            'http://foo.bar/baz/boo/package+foo.deb')
+
+        def failed(error):
+            prober.has_failed = True
+
+        prober.failed = failed
+        prober.redirect('http://foo.bar/baz/boo/package%2Bfoo.deb')
+        self.assertFalse(hasattr(prober, 'has_failed'))
+
     def test_connect_depends_on_localhost_only_config(self):
         # If localhost_only is True and the host to which we would connect is
         # not localhost, the connect() method is not called.