launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/snap-name-extraction into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/snap-name-extraction/+merge/294964

Extract Snap name from snapcraft.yaml (Git only).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/snap-name-extraction into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitref.py'
--- lib/lp/code/interfaces/gitref.py	2016-01-12 03:54:38 +0000
+++ lib/lp/code/interfaces/gitref.py	2016-05-17 21:10:50 +0000
@@ -314,6 +314,14 @@
         "Whether there are recent changes in this repository that have not "
         "yet been scanned.")
 
+    def getBlob(filename):
+        """Get a blob by file name from the related Git repository at this
+        specific Git revision.
+
+        :param filename: Relative path of a file in the repository.
+        :return: A binary string with the blob content.
+        """
+
 
 class IGitRefBatchNavigator(ITableBatchNavigator):
     pass

=== modified file 'lib/lp/code/model/gitref.py'
--- lib/lp/code/model/gitref.py	2016-02-06 02:20:04 +0000
+++ lib/lp/code/model/gitref.py	2016-05-17 21:10:50 +0000
@@ -257,6 +257,10 @@
         hook = SourcePackageRecipe.preLoadDataForSourcePackageRecipes
         return DecoratedResultSet(recipes, pre_iter_hook=hook)
 
+    def getBlob(self, filename):
+        """See `IGitRef`."""
+        return self.repository.getBlob(filename, self.name)
+
 
 @implementer(IGitRef)
 class GitRef(StormBase, GitRefMixin):

=== modified file 'lib/lp/code/model/tests/test_gitref.py'
--- lib/lp/code/model/tests/test_gitref.py	2015-10-12 12:58:32 +0000
+++ lib/lp/code/model/tests/test_gitref.py	2016-05-17 21:10:50 +0000
@@ -12,6 +12,7 @@
 from lp.app.enums import InformationType
 from lp.app.interfaces.informationtype import IInformationType
 from lp.app.interfaces.launchpad import IPrivacy
+from lp.code.interfaces.githosting import IGitHostingClient
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.testing import (
     ANONYMOUS,
@@ -20,6 +21,8 @@
     TestCaseWithFactory,
     verifyObject,
     )
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import DatabaseFunctionalLayer
 from lp.testing.pages import webservice_for_person
 
@@ -64,6 +67,13 @@
         self.assertTrue(ref.private)
         self.assertEqual(InformationType.USERDATA, ref.information_type)
 
+    def test_getBlob(self):
+        [ref] = self.factory.makeGitRefs(paths=[u"refs/heads/foo"])
+        ref.repository.getBlob = FakeMethod()
+        ref.getBlob('src/README.txt')
+        self.assertEqual(
+            ref.repository.getBlob.calls, [(('src/README.txt', 'foo'), {})])
+
 
 class TestGitRefWebservice(TestCaseWithFactory):
     """Tests for the webservice."""

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2016-05-06 09:45:45 +0000
+++ lib/lp/snappy/browser/snap.py	2016-05-17 21:10:50 +0000
@@ -20,6 +20,7 @@
     copy_field,
     use_template,
     )
+import yaml
 from zope.component import getUtility
 from zope.interface import Interface
 from zope.schema import (
@@ -44,11 +45,13 @@
     LaunchpadRadioWidget,
     )
 from lp.code.browser.widgets.gitref import GitRefWidget
+from lp.code.errors import GitRepositoryScanFault
 from lp.code.interfaces.gitref import IGitRef
 from lp.registry.enums import VCSType
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.features import getFeatureFlag
 from lp.services.helpers import english_list
+from lp.services.scripts import log
 from lp.services.webapp import (
     canonical_url,
     ContextMenu,
@@ -326,11 +329,31 @@
 
     @property
     def initial_values(self):
+        name = None
+        if IGitRef.providedBy(self.context):
+            # Try to extract Snap name from snapcraft.yaml file.
+            try:
+                blob = self.context.getBlob('snapcraft.yaml')
+                # Beware of unsafe yaml.load()!
+                name = yaml.safe_load(blob).get('name')
+            except GitRepositoryScanFault:
+                log.info("Failed to get Snap manifest from Git %s",
+                          self.context.unique_name)
+            except (KeyboardInterrupt, SystemExit):
+                raise
+            except:
+                log.debug("Failed to parse Snap manifest at %s",
+                          self.context.unique_name)
+            if name is None:
+                self.widget_errors['name'] = (
+                    "Failed to find a valid snapcraft.yaml file.")
+
         # XXX cjwatson 2015-09-18: Hack to ensure that we don't end up
         # accidentally selecting ubuntu-rtm/14.09 or similar.
         # ubuntu.currentseries will always be in BuildableDistroSeries.
         series = getUtility(ILaunchpadCelebrities).ubuntu.currentseries
         return {
+            'name': name,
             'owner': self.user,
             'distro_series': series,
             }

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2016-05-06 09:45:45 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2016-05-17 21:10:50 +0000
@@ -14,12 +14,14 @@
 
 from fixtures import FakeLogger
 from mechanize import LinkNotFoundError
+import mock
 import pytz
 import soupmatchers
 from testtools.matchers import (
     MatchesSetwise,
     MatchesStructure,
     )
+import yaml
 from zope.component import getUtility
 from zope.publisher.interfaces import NotFound
 from zope.security.interfaces import Unauthorized
@@ -28,6 +30,7 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.processor import IProcessorSet
+from lp.code.errors import GitRepositoryScanFault
 from lp.registry.enums import PersonVisibility
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
@@ -57,6 +60,7 @@
     TestCaseWithFactory,
     time_counter,
     )
+from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import (
     DatabaseFunctionalLayer,
     LaunchpadFunctionalLayer,
@@ -68,8 +72,8 @@
 from lp.testing.pages import (
     extract_text,
     find_main_content,
+    find_tag_by_id,
     find_tags_by_class,
-    find_tag_by_id,
     )
 from lp.testing.publication import test_traverse
 from lp.testing.views import (
@@ -270,6 +274,41 @@
             extract_text(find_tag_by_id(browser.contents, "privacy"))
         )
 
+    def test_initial_name_extraction_git(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.getBlob = FakeMethod(result='name: test-snap')
+        view = create_initialized_view(git_ref, "+new-snap")
+        initial_values = view.initial_values
+        self.assertIn('name', initial_values)
+        self.assertEqual('test-snap', initial_values['name'])
+
+    def test_initial_name_extraction_git_repo_error(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.getBlob = FakeMethod(failure=GitRepositoryScanFault)
+        view = create_initialized_view(git_ref, "+new-snap")
+        initial_values = view.initial_values
+        self.assertIn('name', initial_values)
+        self.assertEqual(None, initial_values['name'])
+
+    def test_initial_name_extraction_git_invalid_data(self):
+        for invalid_result in (None, 123, '', '[][]', '#name:test', ']'):
+            [git_ref] = self.factory.makeGitRefs()
+            git_ref.getBlob = FakeMethod(result=invalid_result)
+            view = create_initialized_view(git_ref, "+new-snap")
+            initial_values = view.initial_values
+            self.assertIn('name', initial_values)
+            self.assertEqual(None, initial_values['name'])
+
+    def test_initial_name_extraction_git_safe_yaml(self):
+        [git_ref] = self.factory.makeGitRefs()
+        git_ref.getBlob = FakeMethod(result='Malicious code encoded in YAML!')
+        view = create_initialized_view(git_ref, "+new-snap")
+        with mock.patch('yaml.load') as unsafe_load:
+            with mock.patch('yaml.safe_load') as safe_load:
+                initial_values = view.initial_values
+        self.assertEqual(0, unsafe_load.call_count)
+        self.assertEqual(1, safe_load.call_count)
+
 
 class TestSnapAdminView(BrowserTestCase):
 

=== modified file 'setup.py'
--- setup.py	2016-05-03 16:38:52 +0000
+++ setup.py	2016-05-17 21:10:50 +0000
@@ -82,6 +82,7 @@
         'python-memcached',
         'python-openid',
         'pytz',
+        'PyYAML',
         'rabbitfixture',
         'requests',
         'requests-toolbelt',