Merge lp:~thumper/launchpad/fix-unicode-error-in-urlparse into lp:launchpad

Proposed by Tim Penhey
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 12795
Proposed branch: lp:~thumper/launchpad/fix-unicode-error-in-urlparse
Merge into: lp:launchpad
Diff against target: 430 lines (+125/-136)
6 files modified
lib/canonical/launchpad/interfaces/validation.py (+0/-105)
lib/lp/app/validators/tests/test_validators.py (+10/-3)
lib/lp/app/validators/url.py (+112/-25)
lib/lp/blueprints/interfaces/specification.py (+1/-1)
lib/lp/registry/browser/announcement.py (+1/-1)
lib/lp/registry/interfaces/productseries.py (+1/-1)
To merge this branch: bzr merge lp:~thumper/launchpad/fix-unicode-error-in-urlparse
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+57103@code.launchpad.net

Commit message

[r=lifeless][bug=35077,325268] Non ascii urls are not considered valid.

Description of the change

I moved the validate_url and valid_webref functions from
canonical.launchpad.interfaces.validation into
lp.app.validators.url, thus satisfying bug 35077.

valid_branch_url wasn't being used anywhere, so I deleted it.

A few lint cleanup removals in that file too.

To keep with the existing validator tests, I put them in the
docstrings too.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/canonical/launchpad/interfaces/validation.py'
--- lib/canonical/launchpad/interfaces/validation.py 2011-03-23 16:28:51 +0000
+++ lib/canonical/launchpad/interfaces/validation.py 2011-04-11 09:39:32 +0000
@@ -7,9 +7,6 @@
77
8__all__ = [8__all__ = [
9 'can_be_nominated_for_series',9 'can_be_nominated_for_series',
10 'validate_url',
11 'valid_webref',
12 'valid_branch_url',
13 'non_duplicate_branch',10 'non_duplicate_branch',
14 'valid_bug_number',11 'valid_bug_number',
15 'valid_cve_sequence',12 'valid_cve_sequence',
@@ -24,7 +21,6 @@
2421
25from cgi import escape22from cgi import escape
26from textwrap import dedent23from textwrap import dedent
27import urllib
2824
29from zope.app.form.interfaces import WidgetsError25from zope.app.form.interfaces import WidgetsError
30from zope.component import getUtility26from zope.component import getUtility
@@ -32,9 +28,7 @@
32from lazr.restful.error import expose28from lazr.restful.error import expose
3329
34from canonical.launchpad import _30from canonical.launchpad import _
35from canonical.launchpad.interfaces.account import IAccount
36from canonical.launchpad.interfaces.emailaddress import (31from canonical.launchpad.interfaces.emailaddress import (
37 IEmailAddress,
38 IEmailAddressSet,32 IEmailAddressSet,
39 )33 )
40from canonical.launchpad.interfaces.launchpad import ILaunchBag34from canonical.launchpad.interfaces.launchpad import ILaunchBag
@@ -44,7 +38,6 @@
44from lp.app.validators import LaunchpadValidationError38from lp.app.validators import LaunchpadValidationError
45from lp.app.validators.cve import valid_cve39from lp.app.validators.cve import valid_cve
46from lp.app.validators.email import valid_email40from lp.app.validators.email import valid_email
47from lp.app.validators.url import valid_absolute_url
4841
4942
50def can_be_nominated_for_series(series):43def can_be_nominated_for_series(series):
@@ -64,101 +57,6 @@
64 return True57 return True
6558
6659
67# XXX matsubara 2006-03-15 bug=35077:
68# The validations functions that deals with URLs should be in
69# validators/ and we should have them as separete constraints in trusted.sql.
70def validate_url(url, valid_schemes):
71 """Returns a boolean stating whether 'url' is a valid URL.
72
73 A URL is valid if:
74 - its URL scheme is in the provided 'valid_schemes' list, and
75 - it has a non-empty host name.
76
77 None and an empty string are not valid URLs::
78
79 >>> validate_url(None, [])
80 False
81 >>> validate_url('', [])
82 False
83
84 The valid_schemes list is checked::
85
86 >>> validate_url('http://example.com', ['http'])
87 True
88 >>> validate_url('http://example.com', ['https', 'ftp'])
89 False
90
91 A URL without a host name is not valid:
92
93 >>> validate_url('http://', ['http'])
94 False
95
96 """
97 if not url:
98 return False
99 scheme, host = urllib.splittype(url)
100 if not scheme in valid_schemes:
101 return False
102 if not valid_absolute_url(url):
103 return False
104 return True
105
106
107def valid_webref(web_ref):
108 """Returns True if web_ref is a valid download URL, or raises a
109 LaunchpadValidationError.
110
111 >>> valid_webref('http://example.com')
112 True
113 >>> valid_webref('https://example.com/foo/bar')
114 True
115 >>> valid_webref('ftp://example.com/~ming')
116 True
117 >>> valid_webref('sftp://example.com//absolute/path/maybe')
118 True
119 >>> valid_webref('other://example.com/moo')
120 Traceback (most recent call last):
121 ...
122 LaunchpadValidationError: ...
123 """
124 if validate_url(web_ref, ['http', 'https', 'ftp', 'sftp']):
125 # Allow ftp so valid_webref can be used for download_url, and so
126 # it doesn't lock out weird projects where the site or
127 # screenshots are kept on ftp.
128 return True
129 else:
130 raise LaunchpadValidationError(_(dedent("""
131 Not a valid URL. Please enter the full URL, including the
132 scheme (for instance, http:// for a web URL), and ensure the
133 URL uses either http, https or ftp.""")))
134
135
136def valid_branch_url(branch_url):
137 """Returns True if web_ref is a valid download URL, or raises a
138 LaunchpadValidationError.
139
140 >>> valid_branch_url('http://example.com')
141 True
142 >>> valid_branch_url('https://example.com/foo/bar')
143 True
144 >>> valid_branch_url('ftp://example.com/~ming')
145 True
146 >>> valid_branch_url('sftp://example.com//absolute/path/maybe')
147 True
148 >>> valid_branch_url('other://example.com/moo')
149 Traceback (most recent call last):
150 ...
151 LaunchpadValidationError: ...
152 """
153 if validate_url(branch_url, ['http', 'https', 'ftp', 'sftp', 'bzr+ssh']):
154 return True
155 else:
156 raise LaunchpadValidationError(_(dedent("""
157 Not a valid URL. Please enter the full URL, including the
158 scheme (for instance, http:// for a web URL), and ensure the
159 URL uses http, https, ftp, sftp, or bzr+ssh.""")))
160
161
162def non_duplicate_branch(value):60def non_duplicate_branch(value):
163 """Ensure that this branch hasn't already been linked to this bug."""61 """Ensure that this branch hasn't already been linked to this bug."""
164 current_bug = getUtility(ILaunchBag).bug62 current_bug = getUtility(ILaunchBag).bug
@@ -224,9 +122,6 @@
224 """Check that the given email is valid and not registered to122 """Check that the given email is valid and not registered to
225 another launchpad account.123 another launchpad account.
226 """124 """
227 from canonical.launchpad.webapp.publisher import canonical_url
228 from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
229
230 _validate_email(email)125 _validate_email(email)
231 _check_email_availability(email)126 _check_email_availability(email)
232 return True127 return True
233128
=== modified file 'lib/lp/app/validators/tests/test_validators.py'
--- lib/lp/app/validators/tests/test_validators.py 2011-02-23 22:27:41 +0000
+++ lib/lp/app/validators/tests/test_validators.py 2011-04-11 09:39:32 +0000
@@ -5,7 +5,11 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8from doctest import DocTestSuite8from doctest import (
9 DocTestSuite,
10 ELLIPSIS,
11 NORMALIZE_WHITESPACE,
12 )
9from unittest import TestSuite13from unittest import TestSuite
1014
11from canonical.launchpad.testing.systemdocs import (15from canonical.launchpad.testing.systemdocs import (
@@ -20,7 +24,8 @@
2024
21 # Include the doctests in __init__.py.25 # Include the doctests in __init__.py.
22 from lp.app import validators26 from lp.app import validators
23 suite.addTest(DocTestSuite(validators))27 suite.addTest(
28 DocTestSuite(validators, optionflags=ELLIPSIS | NORMALIZE_WHITESPACE))
2429
25 from lp.app.validators import email, name, url, version30 from lp.app.validators import email, name, url, version
26 suite.addTest(suitefor(email))31 suite.addTest(suitefor(email))
@@ -33,7 +38,9 @@
3338
34def suitefor(module):39def suitefor(module):
35 """Make a doctest suite with common setUp and tearDown functions."""40 """Make a doctest suite with common setUp and tearDown functions."""
36 suite = DocTestSuite(module, setUp=setUp, tearDown=tearDown)41 suite = DocTestSuite(
42 module, setUp=setUp, tearDown=tearDown,
43 optionflags=ELLIPSIS | NORMALIZE_WHITESPACE)
37 # We have to invoke the LaunchpadFunctionalLayer in order to44 # We have to invoke the LaunchpadFunctionalLayer in order to
38 # initialize the ZCA machinery, which is a pre-requisite for using45 # initialize the ZCA machinery, which is a pre-requisite for using
39 # login().46 # login().
4047
=== modified file 'lib/lp/app/validators/url.py'
--- lib/lp/app/validators/url.py 2011-02-23 20:26:53 +0000
+++ lib/lp/app/validators/url.py 2011-04-11 09:39:32 +0000
@@ -3,14 +3,24 @@
33
4__metaclass__ = type4__metaclass__ = type
55
6__all__ = [
7 'builder_url_validator',
8 'valid_absolute_url',
9 'valid_builder_url',
10 'valid_webref',
11 'validate_url',
12 ]
13
6from textwrap import dedent14from textwrap import dedent
15import urllib
716
8from canonical.launchpad import _17from canonical.launchpad import _
18from canonical.launchpad.webapp.url import urlparse
9from lp.app.validators import LaunchpadValidationError19from lp.app.validators import LaunchpadValidationError
1020
1121
12def valid_absolute_url(name):22def valid_absolute_url(name):
13 """validate an absolute URL.23 """Validate an absolute URL.
1424
15 It looks like this function has been deprecated by25 It looks like this function has been deprecated by
16 canonical.launchpad.interfaces.validation.26 canonical.launchpad.interfaces.validation.
@@ -18,25 +28,27 @@
18 We define this as something that can be parsed into a URL that has both28 We define this as something that can be parsed into a URL that has both
19 a protocol and a network address.29 a protocol and a network address.
2030
21 >>> valid_absolute_url('sftp://chinstrap.ubuntu.com/foo/bar')31 >>> valid_absolute_url('sftp://chinstrap.ubuntu.com/foo/bar')
22 True32 True
23 >>> valid_absolute_url('http://www.example.com')33 >>> valid_absolute_url('http://www.example.com')
24 True34 True
25 >>> valid_absolute_url('whatever:/uxample.com/blah')35 >>> valid_absolute_url('whatever:/uxample.com/blah')
26 False36 False
2737 >>> valid_absolute_url('whatever://example.com/blah')
28 # XXX: 2010-04-26, Salgado, bug=570244: This test only works against38 True
29 # python2.6 but we still need to run on python2.5, so we should uncomment39
30 # it only when we no longer need to run on 2.5.40 Unicode urls are ascii encoded, and a failure here means it isn't valid.
31 >>> #valid_absolute_url('whatever://example.com/blah')41
32 True42 >>> valid_absolute_url(u'http://www.example.com/test...')
43 True
44 >>> valid_absolute_url(u'http://www.example.com/test\u2026')
45 False
46
33 """47 """
34 # Have to import urlparse locally since imports from helpers.py48 try:
35 # causes this module to be imported, and we can't import stuff from49 (scheme, netloc, path, params, query, fragment) = urlparse(name)
36 # webapp at that point, since webapp imports stuff from helpers.py50 except UnicodeEncodeError:
37 # as well.51 return False
38 from canonical.launchpad.webapp.url import urlparse
39 (scheme, netloc, path, params, query, fragment) = urlparse(name)
40 # note that URL checking is also done inside the database, in52 # note that URL checking is also done inside the database, in
41 # trusted.sql, the valid_absolute_url function, and that code uses53 # trusted.sql, the valid_absolute_url function, and that code uses
42 # stdlib urlparse, not our customized version.54 # stdlib urlparse, not our customized version.
@@ -44,6 +56,7 @@
44 return False56 return False
45 return True57 return True
4658
59
47def valid_builder_url(url):60def valid_builder_url(url):
48 """validate a url for a builder.61 """validate a url for a builder.
4962
@@ -56,13 +69,12 @@
56 False69 False
57 >>> valid_builder_url('ftp://foo.com/')70 >>> valid_builder_url('ftp://foo.com/')
58 False71 False
72
59 """73 """
60 # Have to import urlparse locally since imports from helpers.py74 try:
61 # causes this module to be imported, and we can't import stuff from75 (scheme, netloc, path, params, query, fragment) = urlparse(url)
62 # webapp at that point, since webapp imports stuff from helpers.py76 except UnicodeEncodeError:
63 # as well.77 return False
64 from canonical.launchpad.webapp.url import urlparse
65 (scheme, netloc, path, params, query, fragment) = urlparse(url)
66 if scheme != 'http':78 if scheme != 'http':
67 return False79 return False
68 if params or query or fragment:80 if params or query or fragment:
@@ -71,6 +83,7 @@
71 return False83 return False
72 return True84 return True
7385
86
74def builder_url_validator(url):87def builder_url_validator(url):
75 """Return True if the url is valid, or raise a LaunchpadValidationError"""88 """Return True if the url is valid, or raise a LaunchpadValidationError"""
76 if not valid_builder_url(url):89 if not valid_builder_url(url):
@@ -79,3 +92,77 @@
79 http://host/ or http://host:port/ only.92 http://host/ or http://host:port/ only.
80 """), mapping={'url': url}))93 """), mapping={'url': url}))
81 return True94 return True
95
96
97def validate_url(url, valid_schemes):
98 """Returns a boolean stating whether 'url' is a valid URL.
99
100 A URL is valid if:
101 - its URL scheme is in the provided 'valid_schemes' list, and
102 - it has a non-empty host name.
103
104 None and an empty string are not valid URLs::
105
106 >>> validate_url(None, [])
107 False
108 >>> validate_url('', [])
109 False
110
111 The valid_schemes list is checked::
112
113 >>> validate_url('http://example.com', ['http'])
114 True
115 >>> validate_url('http://example.com', ['https', 'ftp'])
116 False
117
118 A URL without a host name is not valid:
119
120 >>> validate_url('http://', ['http'])
121 False
122
123 Unicode urls are converted to ascii for checking. Failure to convert
124 results in failure.
125
126 >>> validate_url(u'http://example.com', ['http'])
127 True
128 >>> validate_url(u'http://example.com/test\u2026', ['http'])
129 False
130
131 """
132 if not url:
133 return False
134 scheme, host = urllib.splittype(url)
135 if not scheme in valid_schemes:
136 return False
137 if not valid_absolute_url(url):
138 return False
139 return True
140
141
142def valid_webref(web_ref):
143 """Returns True if web_ref is a valid download URL, or raises a
144 LaunchpadValidationError.
145
146 >>> valid_webref('http://example.com')
147 True
148 >>> valid_webref('https://example.com/foo/bar')
149 True
150 >>> valid_webref('ftp://example.com/~ming')
151 True
152 >>> valid_webref('sftp://example.com//absolute/path/maybe')
153 True
154 >>> valid_webref('other://example.com/moo')
155 Traceback (most recent call last):
156 ...
157 LaunchpadValidationError: ...
158 """
159 if validate_url(web_ref, ['http', 'https', 'ftp', 'sftp']):
160 # Allow ftp so valid_webref can be used for download_url, and so
161 # it doesn't lock out weird projects where the site or
162 # screenshots are kept on ftp.
163 return True
164 else:
165 raise LaunchpadValidationError(_(dedent("""
166 Not a valid URL. Please enter the full URL, including the
167 scheme (for instance, http:// for a web URL), and ensure the
168 URL uses either http, https or ftp.""")))
82169
=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py 2011-03-28 20:54:50 +0000
+++ lib/lp/blueprints/interfaces/specification.py 2011-04-11 09:39:32 +0000
@@ -46,8 +46,8 @@
46 )46 )
4747
48from canonical.launchpad import _48from canonical.launchpad import _
49from canonical.launchpad.interfaces.validation import valid_webref
50from lp.app.validators import LaunchpadValidationError49from lp.app.validators import LaunchpadValidationError
50from lp.app.validators.url import valid_webref
51from lp.blueprints.enums import (51from lp.blueprints.enums import (
52 SpecificationDefinitionStatus,52 SpecificationDefinitionStatus,
53 SpecificationGoalStatus,53 SpecificationGoalStatus,
5454
=== modified file 'lib/lp/registry/browser/announcement.py'
--- lib/lp/registry/browser/announcement.py 2011-02-02 15:43:31 +0000
+++ lib/lp/registry/browser/announcement.py 2011-04-11 09:39:32 +0000
@@ -33,7 +33,6 @@
33 FeedsMixin,33 FeedsMixin,
34 RootAnnouncementsFeedLink,34 RootAnnouncementsFeedLink,
35 )35 )
36from canonical.launchpad.interfaces.validation import valid_webref
37from canonical.launchpad.webapp.authorization import check_permission36from canonical.launchpad.webapp.authorization import check_permission
38from canonical.launchpad.webapp.batching import BatchNavigator37from canonical.launchpad.webapp.batching import BatchNavigator
39from canonical.launchpad.webapp.menu import (38from canonical.launchpad.webapp.menu import (
@@ -50,6 +49,7 @@
50 custom_widget,49 custom_widget,
51 LaunchpadFormView,50 LaunchpadFormView,
52 )51 )
52from lp.app.validators.url import valid_webref
53from lp.app.widgets.announcementdate import AnnouncementDateWidget53from lp.app.widgets.announcementdate import AnnouncementDateWidget
54from lp.registry.interfaces.announcement import IAnnouncement54from lp.registry.interfaces.announcement import IAnnouncement
55from lp.services.fields import (55from lp.services.fields import (
5656
=== modified file 'lib/lp/registry/interfaces/productseries.py'
--- lib/lp/registry/interfaces/productseries.py 2011-03-28 20:54:50 +0000
+++ lib/lp/registry/interfaces/productseries.py 2011-04-11 09:39:32 +0000
@@ -46,12 +46,12 @@
4646
47from canonical.launchpad import _47from canonical.launchpad import _
48from canonical.launchpad.interfaces.launchpad import IHasAppointedDriver48from canonical.launchpad.interfaces.launchpad import IHasAppointedDriver
49from canonical.launchpad.interfaces.validation import validate_url
50from canonical.launchpad.webapp.url import urlparse49from canonical.launchpad.webapp.url import urlparse
51from lp.app.errors import NameLookupFailed50from lp.app.errors import NameLookupFailed
52from lp.app.interfaces.launchpad import IServiceUsage51from lp.app.interfaces.launchpad import IServiceUsage
53from lp.app.validators import LaunchpadValidationError52from lp.app.validators import LaunchpadValidationError
54from lp.app.validators.name import name_validator53from lp.app.validators.name import name_validator
54from lp.app.validators.url import validate_url
55from lp.blueprints.interfaces.specificationtarget import ISpecificationGoal55from lp.blueprints.interfaces.specificationtarget import ISpecificationGoal
56from lp.bugs.interfaces.bugtarget import (56from lp.bugs.interfaces.bugtarget import (
57 IBugTarget,57 IBugTarget,