Author: dunlapm
Date: Mon Jul 28 23:29:52 2008
New Revision: 21909
Modified:
Products.CMFPlone/trunk/Products/CMFPlone/HISTORY.txt
Products.CMFPlone/trunk/Products/CMFPlone/URLTool.py
Products.CMFPlone/trunk/Products/CMFPlone/tests/testURLTool.py
Log:
Merged r21908 3.1 branch
Modified: Products.CMFPlone/trunk/Products/CMFPlone/HISTORY.txt
==============================================================================
--- Products.CMFPlone/trunk/Products/CMFPlone/HISTORY.txt (original)
+++ Products.CMFPlone/trunk/Products/CMFPlone/HISTORY.txt Mon Jul 28 23:29:52 2008
@@ -213,6 +213,14 @@
Plone 3.1.5 - Unreleased
+ - Corrected behavior in the URL tool to more intelligently handle relative
+ URLs in isURLInPortal. The new functionality requires the current context
+ to be passed in to figure out urls beginning with any number of the '../'
+ sequence. The current context is optional and the tool will default to the
+ old behavior if it is not present. Tests have been added to deomonstrate
+ the new behavior. This closes http://dev.plone.org/plone/ticket/6691
+ [dunlapm]
+
- Corrected transaction note in renameObjectsByPaths.
[hannosch]
Modified: Products.CMFPlone/trunk/Products/CMFPlone/URLTool.py
==============================================================================
--- Products.CMFPlone/trunk/Products/CMFPlone/URLTool.py (original)
+++ Products.CMFPlone/trunk/Products/CMFPlone/URLTool.py Mon Jul 28 23:29:52 2008
@@ -13,21 +13,53 @@
toolicon = 'skins/plone_images/link_icon.gif'
security.declarePublic('isURLInPortal')
- def isURLInPortal(self, url):
+ def isURLInPortal(self, url, context=None):
""" Check if a given url is on the same host and contains the portal
path. Used to ensure that login forms can determine relevant
- referrers (i.e. in portal). Also return true for relative urls,
- though techincally they may not be part of the portal, it's a good
- guess (just like assuming https://portal is in the same portal as
- http://portal).
+ referrers (i.e. in portal). Also return true for some relative urls
+ if context is passed in to allow for url parsing. When context is
+ not provided, assume that relative urls are in the portal. It is
+ assumed that http://portal is the same portal as https://portal.
"""
p_url = self()
- p_host_path = urlparse(p_url)[0:3]
- url_host_path = urlparse(url)[0:3]
+ portal_path = urlparse(p_url)[0:3]
+ url_path = urlparse(url)[0:3]
+ p = {#'protocol':portal_path[0],
+ 'host':portal_path[1],
+ 'path':portal_path[2]}
+ u = {#'protocol':url_path[0],
+ 'host':url_path[1],
+ 'path':url_path[2]}
# check for urls without protocol (i.e. relative urls), or urls with
# the same host and path.
- return (p_host_path[1] == url_host_path[1] and
- url_host_path[2].startswith(p_host_path[2])) or not url_host_path[0]
+
+ if not u['host'] and not u['path'].startswith('/'): #relatively relative url!
+ #url is a relative path that needs to be checked. URLs that start with / can be quickly checked
+ #urls that start with ../ need to have a bit of traversal
+ if u['path'].startswith('.'):
+ if context is None:
+ return True #old behavior
+ else: #gentlemen, start your parsing engines
+ if not context.isPrincipiaFolderish:
+ useurl = context.aq_parent.absolute_url()
+ else:
+ useurl = context.absolute_url()
+ currpath = urlparse(useurl)[2].split('/') #just the path
+ for node in u['path'].split('/'): #path is something like "../../target"
+ if node == '..':
+ if currpath:
+ currpath.pop()
+ else: #we have more ../ than in the current context, we can't be in the portal
+ return False
+ elif node == '.':
+ continue
+ else: #We shouldn't have to deal with crazy urls like ../../somefolder/../otherfolder/../content
+ #add the current node to give us a bit more breathing room, in case somone was silly and used the name of the portal in the relative path
+ return ('/'.join(currpath)+'/'+node).startswith(p['path'])
+ else: #url is in the form: path/to/another/object.jpg
+ return True
+ else:
+ return (p['host'] == u['host'] or not u['host']) and u['path'].startswith(p['path'])
URLTool.__doc__ = BaseTool.__doc__
Modified: Products.CMFPlone/trunk/Products/CMFPlone/tests/testURLTool.py
==============================================================================
--- Products.CMFPlone/trunk/Products/CMFPlone/tests/testURLTool.py (original)
+++ Products.CMFPlone/trunk/Products/CMFPlone/tests/testURLTool.py Mon Jul 28 23:29:52 2008
@@ -11,26 +11,47 @@
def afterSetUp(self):
self.url = self.portal.portal_url
-
+ self.folder.invokeFactory('Folder', id='foo')
+ self.folder.foo.invokeFactory('Document', id='doc1')
+
def test_isURLInPortal(self):
- url_tool = self.url
- self.failUnless(url_tool.isURLInPortal(
- 'http://nohost/%s/foo' % portal_name))
- self.failUnless(url_tool.isURLInPortal(
- 'http://nohost/%s' % portal_name))
- self.failIf(url_tool.isURLInPortal(
- 'http://nohost2/%s/foo' % portal_name))
- self.failUnless(url_tool.isURLInPortal(
- 'https://nohost/%s/bar' % portal_name))
- self.failIf(url_tool.isURLInPortal(
- 'http://nohost:8080/%s/baz' % portal_name))
- self.failIf(url_tool.isURLInPortal(
- 'http://nohost/'))
- # Relative urls always succeed
- self.failUnless(url_tool.isURLInPortal(
- '/images'))
- self.failUnless(url_tool.isURLInPortal(
- 'images/img1.jpg'))
+ iURLiP = self.url.isURLInPortal
+ self.failUnless(iURLiP(
+ 'http://nohost/%s/foo' % portal_name))
+ self.failUnless(iURLiP(
+ 'http://nohost/%s' % portal_name))
+ self.failIf(iURLiP(
+ 'http://nohost2/%s/foo' % portal_name))
+ self.failUnless(iURLiP(
+ 'https://nohost/%s/bar' % portal_name))
+ self.failIf(iURLiP(
+ 'http://nohost:8080/%s/baz' % portal_name))
+ self.failIf(iURLiP(
+ 'http://nohost/'))
+ self.failIf(iURLiP(
+ '/images'))
+ self.failUnless(iURLiP(
+ '/%s/foo' % portal_name))
+
+ def test_isURLInPortalRelative(self):
+ iURLiP = self.url.isURLInPortal
+ #non-root relative urls will need a current context to be passed in
+ self.failUnless(iURLiP(
+ 'images/img1.jpg'))
+ self.failUnless(iURLiP(
+ './images/img1.jpg'))
+ self.failUnless(iURLiP( #/plone/Members/test_user_1_/something
+ '../something', self.folder.foo.doc1))
+ self.failUnless(iURLiP( #/plone/Members/afolder
+ '../../afolder', self.folder.foo.doc1))
+ self.failUnless(iURLiP( #/plone/afolder
+ '../../../afolder', self.folder.foo.doc1))
+ self.failIf(iURLiP( #/afolder
+ '../../../../afolder', self.folder.foo.doc1))
+ self.failIf(iURLiP( #/../afolder? How do we have more ../'s than there are parts in the URL?
+ '../../../../../afolder', self.folder.foo.doc1))
+ self.failUnless(iURLiP( #/plone/afolder
+ '../../../../%s/afolder' % portal_name ,self.folder.foo.doc1))
def test_suite():
|