launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/invalid-services-url-980179 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #980179 in Launchpad itself: "ForbiddenAttribute: ('browserDefault', <lp.app.services.ServiceFactory object at 0x8ca01d0>) "
  https://bugs.launchpad.net/launchpad/+bug/980179

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/invalid-services-url-980179/+merge/102235

== Implementation ==

Make the ServiceFactory raise a NotFound error if someone attempt to navigate to +services and doesn't specify a service name.

== Tests ==

Add new tests to TestServiceFactory

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/configure.zcml
  lib/lp/app/services.py
  lib/lp/app/tests/test_services.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/invalid-services-url-980179/+merge/102235
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/invalid-services-url-980179 into lp:launchpad.

=== modified file 'lib/lp/app/configure.zcml'
--- lib/lp/app/configure.zcml	2012-02-24 05:14:46 +0000
+++ lib/lp/app/configure.zcml	2012-04-17 04:34:21 +0000
@@ -73,6 +73,8 @@
             interface="lp.app.interfaces.services.IServiceFactory"/>
         <allow
             interface="zope.publisher.interfaces.IPublishTraverse"/>
+        <allow
+            interface="zope.publisher.interfaces.browser.IBrowserPublisher"/>
     </securedutility>
     <securedutility
         class="lp.services.webservice.services.ServicesLink"

=== modified file 'lib/lp/app/services.py'
--- lib/lp/app/services.py	2012-02-23 10:13:48 +0000
+++ lib/lp/app/services.py	2012-04-17 04:34:21 +0000
@@ -10,6 +10,7 @@
 
 from zope.component import getUtility
 from zope.interface import implements
+from zope.publisher.interfaces import NotFound
 
 from lp.app.interfaces.services import (
     IService,
@@ -35,3 +36,7 @@
 
     def getService(self, service_name):
         return getUtility(IService, service_name)
+
+    def browserDefault(self, request):
+        # There is no valid traversal to +services.
+        raise NotFound(self, "+services", request)

=== modified file 'lib/lp/app/tests/test_services.py'
--- lib/lp/app/tests/test_services.py	2012-02-28 04:24:19 +0000
+++ lib/lp/app/tests/test_services.py	2012-04-17 04:34:21 +0000
@@ -6,6 +6,7 @@
 from lazr.restful.interfaces._rest import IHTTPResource
 from zope.component import getUtility
 from zope.interface.declarations import implements
+from zope.publisher.interfaces import NotFound
 
 from lp.app.interfaces.services import (
     IService,
@@ -45,3 +46,15 @@
             'https://launchpad.dev/api/devel/+services/fake')
         self.assertEqual(getUtility(IServiceFactory), context)
         self.assertEqual(fake_service, view)
+
+    def test_invalid_traversal(self):
+        # Test that traversal to +services without a service specified fails.
+        self.assertRaises(
+            NotFound, self.getUserBrowser,
+            'https://launchpad.dev/api/devel/+services')
+
+    def test_invalid_service(self):
+        # Test that traversal an invalid service name fails.
+        self.assertRaises(
+            NotFound, self.getUserBrowser,
+            'https://launchpad.dev/api/devel/+services/invalid')