launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/archive-sourcefiles-private into lp:launchpad.

Commit message:
Fix Archive:+sourcefiles for private archives.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/archive-sourcefiles-private/+merge/345442
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/archive-sourcefiles-private into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/archive.py'
--- lib/lp/soyuz/browser/archive.py	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/browser/archive.py	2018-05-11 18:13:12 +0000
@@ -58,6 +58,7 @@
     SimpleTerm,
     SimpleVocabulary,
     )
+from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
 
 from lp import _
@@ -99,7 +100,10 @@
 from lp.services.database.bulk import load_related
 from lp.services.helpers import english_list
 from lp.services.job.model.job import Job
-from lp.services.librarian.browser import FileNavigationMixin
+from lp.services.librarian.browser import (
+    DeletedProxiedLibraryFileAlias,
+    FileNavigationMixin,
+    )
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
@@ -120,6 +124,7 @@
     IStructuredString,
     )
 from lp.services.webapp.menu import NavigationMenu
+from lp.services.webapp.publisher import RedirectionView
 from lp.services.worlddata.interfaces.country import ICountrySet
 from lp.soyuz.adapters.archivedependencies import (
     default_component_dependency_name,
@@ -478,9 +483,23 @@
         version = self.request.stepstogo.consume()
         filename = self.request.stepstogo.consume()
 
-        return self.context.getSourceFileByName(
+        if not check_permission('launchpad.View', self.context):
+            raise Unauthorized()
+
+        library_file = self.context.getSourceFileByName(
             sourcepackagename, version, filename)
 
+        # Deleted library files result in a NotFound-like error.
+        if library_file.deleted:
+            raise DeletedProxiedLibraryFileAlias(filename, self.context)
+
+        # There can be no further path segments.
+        if len(self.request.stepstogo) > 0:
+            return None
+
+        return RedirectionView(
+            library_file.getURL(include_token=True), self.request)
+
 
 class ArchiveMenuMixin:
 

=== modified file 'lib/lp/soyuz/browser/publishing.py'
--- lib/lp/soyuz/browser/publishing.py	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/browser/publishing.py	2018-05-11 18:13:12 +0000
@@ -17,6 +17,7 @@
 from lazr.delegates import delegate_to
 from zope.interface import implementer
 
+from lp.archiveuploader.utils import re_isadeb
 from lp.services.librarian.browser import (
     FileNavigationMixin,
     ProxiedLibraryFileAlias,
@@ -287,8 +288,7 @@
             custom_dict = {}
             custom_dict["filename"] = library_file.filename
             custom_dict["filesize"] = library_file.content.filesize
-            if (library_file.filename.endswith('.deb') or
-                library_file.filename.endswith('.udeb')):
+            if re_isadeb.match(library_file.filename):
                 custom_dict['class'] = 'binary'
                 custom_dict["url"] = ProxiedLibraryFileAlias(
                     library_file, self.context.archive).http_url

=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt	2018-05-04 21:59:32 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt	2018-05-11 18:13:12 +0000
@@ -236,17 +236,14 @@
 Retrieve file information for using the direct HTTP browsing API.
 
     >>> login('foo.bar@xxxxxxxxxxxxx')
-    >>> file_content = dsc_file.read()
-    >>> file_size = str(dsc_file.content.filesize)
-    >>> file_mimetype = dsc_file.mimetype
-    >>> file_lp_url = str(
-    ...     'http://launchpad.dev/~no-priv/+archive/ubuntu/p3a/+files/%s' %
-    ...     dsc_file.filename)
+    >>> dsc_file_lp_url = (
+    ...     'http://launchpad.dev/~no-priv/+archive/ubuntu/p3a/+sourcefiles/'
+    ...     'test-pkg/1.0/%s' % dsc_file.filename)
     >>> logout()
 
 Sample Person can't access the file.
 
-    >>> browser.open(file_lp_url)
+    >>> browser.open(dsc_file_lp_url)
     Traceback (most recent call last):
     ...
     Unauthorized
@@ -256,12 +253,32 @@
     >>> print http(r"""
     ... GET %s HTTP/1.1
     ... Authorization: Basic no-priv@xxxxxxxxxxxxx:test
-    ... """ % (file_lp_url.replace('http://launchpad.dev', '')))
+    ... """ % (dsc_file_lp_url.replace('http://launchpad.dev', '')))
     HTTP/1.1 303 See Other
     ...
     Location: https://...restricted.../test-pkg_1.0.dsc?token=...
     ...
 
+Binary files are served via '+files' rather than '+sourcefiles'.
+
+    >>> login('foo.bar@xxxxxxxxxxxxx')
+    >>> deb_file_lp_url = (
+    ...     'http://launchpad.dev/~no-priv/+archive/ubuntu/p3a/+files/%s' %
+    ...     deb_file.filename)
+    >>> logout()
+    >>> browser.open(deb_file_lp_url)
+    Traceback (most recent call last):
+    ...
+    Unauthorized
+    >>> print http(r"""
+    ... GET %s HTTP/1.1
+    ... Authorization: Basic no-priv@xxxxxxxxxxxxx:test
+    ... """ % (deb_file_lp_url.replace('http://launchpad.dev', '')))
+    HTTP/1.1 303 See Other
+    ...
+    Location: https://...restricted.../test-bin_1.0_all.deb?token=...
+    ...
+
 If the associated PPA and the `LibraryFileAlias` are public, the +files/
 proxy redirects to the public http url. We'll copy the test sources and
 binaries across to no-priv's public ppa.
@@ -286,8 +303,8 @@
     False
     >>> file_librarian_url = dsc_file.http_url
     >>> file_lp_url = str(
-    ...     'http://launchpad.dev/~no-priv/+archive/ubuntu/ppa/+files/%s' %
-    ...     dsc_file.filename)
+    ...     'http://launchpad.dev/~no-priv/+archive/ubuntu/ppa/+sourcefiles/'
+    ...     'test-pkg/1.0/%s' % dsc_file.filename)
 
     >>> transaction.commit()
     >>> logout()
@@ -318,9 +335,8 @@
 sure that '+files' isn't understood as the PPA name, but instead
 redirect to the files for the default named PPA.
 
-    >>> file_lp_url_without_ppa_name = file_lp_url.replace('/ubuntu/ppa', '')
-    >>> print file_lp_url_without_ppa_name
-    http://launchpad.dev/~no-priv/+archive/+files/test-pkg_1.0.dsc
+    >>> file_lp_url_without_ppa_name = (
+    ...     'http://launchpad.dev/~no-priv/+archive/+files/test-pkg_1.0.dsc')
 
     >>> print http(r"""
     ... GET %s HTTP/1.1
@@ -390,7 +406,7 @@
 LP proxy URL a proper NotFound error is raised.
 
     >>> print file_lp_url
-    http://launchpad.dev/~no-priv/+archive/ubuntu/ppa/+files/test-pkg_1.0.dsc
+    http://launchpad.dev/~no-priv/+archive/ubuntu/ppa/+sourcefiles/test-pkg/1.0/test-pkg_1.0.dsc
 
     >>> not_found_file = http(r"""
     ... GET %s HTTP/1.1
@@ -410,6 +426,6 @@
     Lost something?
     ...
     NotFound:
-    Object: <Archive at ...>, name: u'test-pkg_1.0.dsc'...
+    Object: <Archive at ...>, name: u'test-pkg'...
     ...