launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04659
[Merge] lp:~benji/launchpad/bug-162868-2 into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/bug-162868-2 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #162868 in Launchpad itself: "Macro package URLs return OOPSes"
https://bugs.launchpad.net/launchpad/+bug/162868
For more details, see:
https://code.launchpad.net/~benji/launchpad/bug-162868-2/+merge/71942
Bug 162868 describes the fact that templates containing macros are
registered in LP such that they can be invoked via URL. That's not
intended or good, since they then generate and OOPS.
This branch introduces a "Macro" view class that can be used for such
registrations and prevents them from being URL accessible.
I would have liked to refactor all of our macro use to a different
pattern but we've introduced many dependencies upon the quirks of this
particular approach over the years and it's not reasonable to unwind
them all right now. Instead this approach gives us an easy way to
stop over-exposing the macro templates.
Tests: bin/test -c -t lp.app.browser.tests.test_macro_view
Lint: the make lint report is clean.
QA: visit one of the URLs given in the bug description and verify that
you get a NotFound page instead of an OOPS.
--
https://code.launchpad.net/~benji/launchpad/bug-162868-2/+merge/71942
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/bug-162868-2 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/zcml/feeds.zcml'
--- lib/canonical/launchpad/zcml/feeds.zcml 2010-10-03 15:30:06 +0000
+++ lib/canonical/launchpad/zcml/feeds.zcml 2011-08-17 19:59:25 +0000
@@ -33,7 +33,7 @@
<!-- Macros -->
<browser:page
- for="*"
+ for="canonical.lazr.interfaces.feed.IFeedEntry"
name="feed-entry-atom"
permission="zope.Public"
template="../../lazr/feed/templates/feed-entry-atom.pt"
=== modified file 'lib/lp/app/browser/configure.zcml'
--- lib/lp/app/browser/configure.zcml 2011-08-09 02:06:15 +0000
+++ lib/lp/app/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -13,12 +13,14 @@
name="+base-layout-macros"
template="../templates/base-layout-macros.pt"
permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"
/>
<browser:page
for="*"
name="+main-template-macros"
template="../templates/base-layout-macros.pt"
permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"
/>
<browser:page
for="*"
@@ -26,6 +28,7 @@
layer="canonical.launchpad.layers.LaunchpadLayer"
permission="zope.Public"
template="../templates/launchpad-form.pt"
+ class="lp.app.browser.launchpad.Macro"
/>
<browser:page
for="*"
@@ -33,6 +36,7 @@
layer="canonical.launchpad.layers.LaunchpadLayer"
permission="zope.Public"
template="../templates/launchpad-widget-macros.pt"
+ class="lp.app.browser.launchpad.Macro"
/>
<browser:page
for="*"
@@ -100,14 +104,14 @@
permission="zope.Public" />
<browser:page
- for="*"
+ for="lp.app.browser.root.LaunchpadSearchView"
name="+search-form"
class="lp.app.browser.root.LaunchpadSearchFormView"
template="../templates/launchpad-search-form.pt"
permission="zope.Public" />
<browser:page
- for="*"
+ for="lp.app.browser.root.LaunchpadSearchView"
name="+primary-search-form"
class="lp.app.browser.root.LaunchpadPrimarySearchFormView"
template="../templates/launchpad-search-form.pt"
@@ -208,6 +212,7 @@
name="+forbidden-page-macros"
template="../templates/launchpad-forbidden-macros.pt"
permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"
/>
=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py 2011-07-08 14:22:14 +0000
+++ lib/lp/app/browser/launchpad.py 2011-08-17 19:59:25 +0000
@@ -15,6 +15,7 @@
'LaunchpadRootNavigation',
'LinkView',
'LoginStatus',
+ 'Macro',
'MaintenanceMessage',
'NavigationMenuTabs',
'SoftTimeoutView',
@@ -49,6 +50,7 @@
from zope.publisher.interfaces.browser import IBrowserPublisher
from zope.publisher.interfaces.xmlrpc import IXMLRPCRequest
from zope.security.interfaces import Unauthorized
+from zope.traversing.interfaces import ITraversable
from canonical.config import config
from canonical.launchpad.helpers import intOrZero
@@ -342,7 +344,67 @@
return len(self.items) > 1 and not has_major_heading
-class MaintenanceMessage:
+class Macro:
+ """Keeps templates that are registered as pages from being URL accessable.
+
+ The standard pattern in LP is to register templates that contain macros as
+ views on all objects:
+
+ <browser:page
+ for="*"
+ name="+main-template-macros"
+ template="../templates/base-layout-macros.pt"
+ permission="zope.Public"
+ />
+
+ While being a bit of a big hammer, that pattern also has the side effect
+ of making the template URL traversable from any object. Therefore
+ requests like these would all "work":
+
+ http://launchpad.net/+main-template-macros
+ http://launchpad.net/ubuntu/+main-template-macros
+ http://launchpad.net/ubuntu/+main-template-macros
+ https://blueprints.launchpad.dev/ubuntu/hoary/+main-template-macros
+
+ Obviously, those requests wouldn't do anything useful and would instead
+ generate an OOPS.
+
+ It would be nice to use a different pattern for macros instead, but we've
+ grown dependent on some of the peculiatrities of registering macro
+ templates in this way.
+
+ This class was created in order to prevent macro templates from being
+ accessable via URL without having to make nontrivial changes to the many,
+ many templates that use macros. To use the class add a "class" parameter
+ to macro template registrations:
+
+ <browser:page
+ for="*"
+ name="+main-template-macros"
+ template="../templates/base-layout-macros.pt"
+ class="lp.app.browser.launchpad.Macro"
+ permission="zope.Public"
+ />
+ """
+ implements(IBrowserPublisher, ITraversable)
+
+ def __init__(self, context, request):
+ self.context = context
+
+ def traverse(self, name, furtherPath):
+ return self.index.macros[name]
+
+ def browserDefault(self, request):
+ return self, ()
+
+ def publishTraverse(self, request, name):
+ raise NotFound(self.context, self.__name__)
+
+ def __call__(self):
+ raise NotFound(self.context, self.__name__)
+
+
+class MaintenanceMessage(Macro):
"""Display a maintenance message if the control file is present and
it contains a valid iso format time.
=== added file 'lib/lp/app/browser/tests/test_macro_view.py'
--- lib/lp/app/browser/tests/test_macro_view.py 1970-01-01 00:00:00 +0000
+++ lib/lp/app/browser/tests/test_macro_view.py 2011-08-17 19:59:25 +0000
@@ -0,0 +1,68 @@
+# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for traversal from the root branch object."""
+
+__metaclass__ = type
+
+from zope.publisher.interfaces import NotFound
+
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.testing import TestCaseWithFactory
+from lp.testing.publication import test_traverse
+
+
+class TestMacroNontraversability(TestCaseWithFactory):
+ """Macros should not be URL accessable (see bug 162868)."""
+
+ layer = DatabaseFunctionalLayer
+
+ # Names of some macros that are tested to ensure that they're not
+ # accessable via URL. This is not an exhaustive list.
+ macro_names = (
+ 'feed-entry-atom',
+ '+base-layout-macros',
+ '+main-template-macros',
+ 'launchpad_form',
+ 'launchpad_widget_macros',
+ '+forbidden-page-macros',
+ '+search-form',
+ '+primary-search-form"',
+ 'form-picker-macros',
+ '+filebug-macros',
+ '+bugtarget-macros-search',
+ 'bugcomment-macros',
+ 'bug-attachment-macros',
+ '+portlet-malone-bugmail-filtering-faq',
+ '+bugtask-macros-tableview',
+ 'bugtask-macros-cve',
+ '+bmp-macros',
+ 'branch-form-macros',
+ '+bmq-macros',
+ '+announcement-macros',
+ '+person-macros',
+ '+milestone-macros',
+ '+distributionmirror-macros',
+ '+timeline-macros',
+ '+macros',
+ '+rosetta-status-legend',
+ '+translations-macros',
+ '+object-reassignment',
+ )
+
+ @staticmethod
+ def is_not_found(path):
+ def traverse_and_call():
+ view = test_traverse(path)[1]
+ view()
+ try:
+ traverse_and_call()
+ except NotFound:
+ return True
+ else:
+ return False
+
+ def test_macro_names_not_traversable(self):
+ for name in self.macro_names:
+ self.assertTrue(self.is_not_found('http://launchpad.dev/' + name),
+ 'macro name %r should not be URL accessable' % name)
=== modified file 'lib/lp/app/widgets/configure.zcml'
--- lib/lp/app/widgets/configure.zcml 2011-04-15 02:52:12 +0000
+++ lib/lp/app/widgets/configure.zcml 2011-08-17 19:59:25 +0000
@@ -24,6 +24,7 @@
for="*"
name="form-picker-macros"
permission="zope.Public"
- template="templates/form-picker-macros.pt"/>
+ template="templates/form-picker-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
</configure>
=== modified file 'lib/lp/bugs/browser/configure.zcml'
--- lib/lp/bugs/browser/configure.zcml 2011-08-03 05:00:46 +0000
+++ lib/lp/bugs/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -80,7 +80,8 @@
for="*"
name="+filebug-macros"
permission="zope.Public"
- template="../templates/bugtarget-macros-filebug.pt"/>
+ template="../templates/bugtarget-macros-filebug.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
name="+filebug"
for="lp.bugs.interfaces.bugtarget.IBugTarget"
@@ -139,7 +140,8 @@
name="+bugtarget-macros-search"
for="*"
permission="zope.Public"
- template="../templates/bugtarget-macros-search.pt"/>
+ template="../templates/bugtarget-macros-search.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
name="+unlinkbug"
for="lp.bugs.interfaces.buglink.IBugLinkTarget"
@@ -163,12 +165,14 @@
for="*"
name="bugcomment-macros"
permission="zope.Public"
- template="../templates/bugcomment-macros.pt"/>
+ template="../templates/bugcomment-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="*"
name="bug-attachment-macros"
permission="zope.Public"
- template="../templates/bug-attachment-macros.pt"/>
+ template="../templates/bug-attachment-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:defaultView
for="lp.bugs.interfaces.bugmessage.IBugComment"
name="+index"/>
@@ -255,7 +259,7 @@
facet="bugs"
template="../templates/buglisting-embedded-advanced-search.pt"/>
<browser:page
- for="*"
+ for="lp.bugs.interfaces.structuralsubscription.IStructuralSubscriptionTarget"
name="+portlet-malone-bugmail-filtering-faq"
template="../templates/malone-portlet-bugmail-filtering-faq.pt"
permission="zope.Public"/>
@@ -498,12 +502,14 @@
for="*"
name="+bugtask-macros-tableview"
permission="zope.Public"
- template="../templates/bugtask-macros-tableview.pt"/>
+ template="../templates/bugtask-macros-tableview.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="*"
name="bugtask-macros-cve"
permission="zope.Public"
- template="../templates/bugtask-macros-cve.pt"/>
+ template="../templates/bugtask-macros-cve.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:defaultView
for="lp.bugs.interfaces.bugtask.IBugTask"
name="+index"/>
=== modified file 'lib/lp/code/browser/configure.zcml'
--- lib/lp/code/browser/configure.zcml 2011-07-21 02:18:54 +0000
+++ lib/lp/code/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -356,7 +356,8 @@
layer="lp.code.publisher.CodeLayer"
name="+bmp-macros"
permission="zope.Public"
- template="../templates/branchmergeproposal-macros.pt"/>
+ template="../templates/branchmergeproposal-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:menus
classes="
BranchMergeProposalContextMenu
@@ -490,7 +491,8 @@
layer="lp.code.publisher.CodeLayer"
name="branch-form-macros"
permission="zope.Public"
- template="../templates/branch-form-macros.pt"/>
+ template="../templates/branch-form-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
name="+whiteboard"
for="lp.code.interfaces.branch.IBranch"
@@ -1343,7 +1345,8 @@
layer="lp.code.publisher.CodeLayer"
name="+bmq-macros"
permission="zope.Public"
- template="../templates/branchmergequeue-macros.pt"/>
+ template="../templates/branchmergequeue-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
</facet>
=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml 2011-07-29 14:00:21 +0000
+++ lib/lp/registry/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -775,10 +775,11 @@
permission="zope.Public"
template="../templates/announcements-all.pt"/>
<browser:page
- for="*"
+ for="lp.registry.interfaces.announcement.IAnnouncement"
name="+announcement-macros"
permission="zope.Public"
- template="../templates/announcement-macros.pt" />
+ template="../templates/announcement-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<adapter
provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
for="lp.registry.interfaces.announcement.IAnnouncement"
@@ -942,7 +943,8 @@
for="*"
name="+person-macros"
permission="zope.Public"
- template="../templates/person-macros.pt"/>
+ template="../templates/person-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.registry.interfaces.person.IPerson"
permission="zope.Public"
@@ -1334,7 +1336,8 @@
for="*"
name="+milestone-macros"
permission="zope.Public"
- template="../templates/milestone-macros.pt"/>
+ template="../templates/milestone-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:pages
for="lp.registry.interfaces.milestone.IMilestone"
class="lp.registry.browser.milestone.MilestoneView"
@@ -2239,7 +2242,8 @@
for="*"
name="+distributionmirror-macros"
permission="zope.Public"
- template="../templates/distributionmirror-macros.pt" />
+ template="../templates/distributionmirror-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.registry.interfaces.distributionmirror.IDistributionMirror"
permission="zope.Public"
@@ -2307,6 +2311,7 @@
name="+timeline-macros"
template="../templates/timeline-macros.pt"
permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"
/>
<browser:page
for="lp.registry.interfaces.sourcepackage.ISourcePackage"
=== modified file 'lib/lp/soyuz/browser/configure.zcml'
--- lib/lp/soyuz/browser/configure.zcml 2011-07-08 14:22:14 +0000
+++ lib/lp/soyuz/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -234,8 +234,7 @@
classes="ArchiveNavigation" />
<browser:navigation
module="lp.soyuz.browser.processor"
- classes="
- ProcessorFamilySetNavigation ProcessorSetNavigation"/>
+ classes="ProcessorFamilySetNavigation ProcessorSetNavigation"/>
<browser:url
for="lp.soyuz.interfaces.archive.IPPA"
path_expression="string:+archive"
@@ -250,7 +249,8 @@
for="*"
name="+macros"
permission="zope.Public"
- template="../templates/archive-macros.pt"/>
+ template="../templates/archive-macros.pt"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.soyuz.interfaces.archive.IArchive"
permission="launchpad.View"
=== modified file 'lib/lp/translations/browser/configure.zcml'
--- lib/lp/translations/browser/configure.zcml 2011-08-04 14:15:13 +0000
+++ lib/lp/translations/browser/configure.zcml 2011-08-17 19:59:25 +0000
@@ -47,18 +47,13 @@
layer="lp.translations.publisher.TranslationsLayer"
name="+products-with-translations"
template="../templates/rosetta-products.pt"/>
- <browser:pages
+ <browser:page
for="*"
+ name="+rosetta-status-legend"
+ template="../templates/rosetta-status-legend.pt"
facet="translations"
- class="lp.translations.browser.translations.RosettaApplicationView"
- permission="zope.Public">
- <browser:page
- name="+portlet-preflangs"
- template="../templates/object-portlet-preflangs.pt"/>
- <browser:page
- name="+rosetta-status-legend"
- template="../templates/rosetta-status-legend.pt"/>
- </browser:pages>
+ permission="zope.Public"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.translations.interfaces.rosettastats.IRosettaStats"
class="lp.translations.browser.translations.RosettaStatsView"
@@ -73,7 +68,8 @@
facet="translations"
permission="zope.Public"
template="../templates/translations-macros.pt"
- layer="lp.translations.publisher.TranslationsLayer"/>
+ layer="lp.translations.publisher.TranslationsLayer"
+ class="lp.app.browser.launchpad.Macro"/>
<browser:page
for="lp.translations.interfaces.translationpolicy.ITranslationPolicy"
facet="translations"
@@ -503,7 +499,7 @@
template="../templates/translationgroup-reassignment.pt"
layer="lp.translations.publisher.TranslationsLayer"/>
<browser:page
- for="*"
+ for="lp.translations.interfaces.translationgroup.ITranslationGroup"
permission="launchpad.Edit"
name="+object-reassignment"
template="../../app/templates/object-reassignment.pt"
