Skip to content

Commit

Permalink
Merge pull request #71 from plone/thet-fix-linkwidget
Browse files Browse the repository at this point in the history
Fix urlparsing with external links, especially if they're shorter tha…
  • Loading branch information
agitator authored Jun 13, 2017
2 parents 0b97cb4 + 259df88 commit 1f9e10a
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 15 deletions.
32 changes: 21 additions & 11 deletions plone/app/z3cform/converters.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from plone.app.z3cform.interfaces import IQueryStringWidget
from plone.app.z3cform.interfaces import IRelatedItemsWidget
from plone.app.z3cform.interfaces import ISelectWidget
from plone.app.z3cform.utils import replace_link_variables_by_paths
from plone.app.z3cform import utils
from plone.uuid.interfaces import IUUID
from Products.CMFCore.utils import getToolByName
from Products.CMFPlone.utils import safe_callable
Expand All @@ -27,6 +27,7 @@

import json
import pytz
import urlparse


@adapter(IDate, IDateWidget)
Expand Down Expand Up @@ -313,12 +314,14 @@ class LinkWidgetDataConverter(BaseDataConverter):

def toWidgetValue(self, value):
value = super(LinkWidgetDataConverter, self).toWidgetValue(value)
result = {'internal': u'',
'external': u'',
'email': u'',
'email_subject': u''}
uuid = None
result = {
'internal': u'',
'external': u'',
'email': u'',
'email_subject': u''
}
if value.startswith('mailto:'):
# Handle mail URLs
value = value[7:] # strip mailto from beginning
if '?subject=' in value:
email, email_subject = value.split('?subject=')
Expand All @@ -327,12 +330,19 @@ def toWidgetValue(self, value):
else:
result['email'] = value
else:
if '/resolveuid/' in value:
uuid = None
portal = getSite()
is_same_domain = utils.is_same_domain(value, portal.absolute_url())
is_absolute = utils.is_absolute(value)
if '/resolveuid/' in value and (not is_absolute or is_same_domain):
# Take the UUID part of a resolveuid url, but onl if it's on
# the same domain.
result['internal'] = value.rsplit('/', 1)[-1]
else:
portal = getSite()
path = replace_link_variables_by_paths(portal, value)
path = path[len(portal.absolute_url())+1:].encode('ascii', 'ignore') # noqa
elif not is_absolute or is_absolute and is_same_domain:
# Handdle relative URLs or absolute URLs on the same domain.
path = urlparse.urlparse(value).path
path = utils.replace_link_variables_by_paths(portal, path)
path = path.encode('ascii', 'ignore')
obj = portal.unrestrictedTraverse(path=path, default=None)
if obj is not None:
uuid = IUUID(obj, None)
Expand Down
35 changes: 31 additions & 4 deletions plone/app/z3cform/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,34 @@ def test_complex(self):
self.assertEqual(test_out, test_compare)


def test_suite():
return unittest.TestSuite([
unittest.makeSuite(TestUnitCallCallables),
])
class TestUtils(unittest.TestCase):

def test_is_absolute(self):
from plone.app.z3cform.utils import is_absolute

self.assertTrue(is_absolute('https://plone.org/'))
self.assertTrue(is_absolute('http://plone.org/'))
self.assertTrue(is_absolute('webdav://plone.org/'))
self.assertTrue(not is_absolute('./path/to/site'))
self.assertTrue(not is_absolute('/resolveuid/'))

def test_is_same_domain(self):
from plone.app.z3cform.utils import is_same_domain

# Those use the same protocol and are on the same domaain
self.assertTrue(is_same_domain(
'https://plone.org/doc1',
'https://plone.org/doc2/doc3'
))

# These are two completly different URLs
self.assertTrue(not is_same_domain(
'https://domain1.com',
'https://anotherdomain.com'
))

# Here, different transport protocols are used. Returning False.
self.assertTrue(not is_same_domain(
'https://plone.org',
'http://plone.org'
))
65 changes: 65 additions & 0 deletions plone/app/z3cform/tests/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from plone.dexterity.fti import DexterityFTI
from plone.registry.interfaces import IRegistry
from plone.testing.zca import UNIT_TESTING
from plone.uuid.interfaces import IUUID
from Products.CMFPlone.interfaces import IMarkupSchema
from z3c.form.form import Form
from z3c.form.interfaces import IFormLayer
Expand Down Expand Up @@ -1502,3 +1503,67 @@ def test_link_widget__extract_email_including_mailto(self):
widget.extract(),
u'mailto:dev@plone.org'
)

def test_link_widget__data_converter(self):
from plone.app.z3cform.widget import LinkWidget
from plone.app.z3cform.converters import LinkWidgetDataConverter

field = TextLine(__name__='linkfield')
widget = LinkWidget(self.request)
converter = LinkWidgetDataConverter(field, widget)

self.portal.invokeFactory('Folder', 'test')
portal_url = self.portal.absolute_url()
portal_path = '/'.join(self.portal.getPhysicalPath())

# Test external URLs
self.assertEqual(
converter.toWidgetValue(u'https://plone.org')['external'],
u'https://plone.org'
)

# Test relative resolveuid URLs
self.assertEqual(
converter.toWidgetValue(u'/resolveuid/1234')['internal'],
u'1234'
)

# Test absolute resolveuid URLs on the same domain
self.assertEqual(
converter.toWidgetValue(portal_url + '/resolveuid/1234')['internal'], # noqa
u'1234'
)

# Test absolute resolveuid URLs on a different domain
self.assertEqual(
converter.toWidgetValue(u'http://anyurl/resolveuid/1234')['external'], # noqa
u'http://anyurl/resolveuid/1234'
)

# Test interrnal URL paths
self.assertEqual(
converter.toWidgetValue(portal_path + '/test')['internal'],
IUUID(self.portal.test)
)

# Test absolute interrnal URLs
self.assertEqual(
converter.toWidgetValue(portal_url + '/test')['internal'],
IUUID(self.portal.test)
)

# Test mail
self.assertEqual(
converter.toWidgetValue(u'mailto:me')['email'],
u'me'
)

# Test mail with subject
self.assertEqual(
converter.toWidgetValue(u'mailto:me?subject=jep')['email'],
u'me'
)
self.assertEqual(
converter.toWidgetValue(u'mailto:me?subject=jep')['email_subject'],
u'jep'
)
17 changes: 17 additions & 0 deletions plone/app/z3cform/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
from Products.CMFCore.interfaces import IFolderish
from zope.component.hooks import getSite

import urlparse


try:
from zope.globalrequest import getRequest
Expand Down Expand Up @@ -105,3 +107,18 @@ def _replace_variable_by_path(url, variable, obj):
)

return url


def is_absolute(url):
"""Return ``True``, if url is an absolute url.
See: https://stackoverflow.com/a/8357518/1337474
"""
return bool(urlparse.urlparse(url).netloc)


def is_same_domain(url1, url2):
"""Return ``True``, if url1 is on the same protocol and domain than url2.
"""
purl1 = urlparse.urlparse(url1)
purl2 = urlparse.urlparse(url2)
return purl1.scheme == purl2.scheme and purl1.netloc == purl2.netloc

2 comments on commit 1f9e10a

@jenkins-plone-org
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agitator Jenkins CI reporting about code analysis
See the full report here: http://jenkins.plone.org/job/package-plone.app.z3cform/32/violations

plone/app/z3cform/inline_validation.py:15:1: C901 'InlineValidationView.__call__' is too complex (15)
plone/app/z3cform/widget.py:724:23: P101 format string does contain unindexed parameters
plone/app/z3cform/converters.py:11:1: I001 isort found an import in the wrong position

Follow these instructions to reproduce it locally.

@jenkins-plone-org
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agitator Jenkins CI reporting about code analysis
See the full report here: http://jenkins.plone.org/job/package-plone.app.z3cform/33/violations

plone/app/z3cform/inline_validation.py:15:1: C901 'InlineValidationView.__call__' is too complex (15)
plone/app/z3cform/widget.py:724:23: P101 format string does contain unindexed parameters
plone/app/z3cform/converters.py:11:1: I001 isort found an import in the wrong position

Follow these instructions to reproduce it locally.

Please sign in to comment.