Merge lp:~leonardr/lazr.restful/invalid-uri into lp:lazr.restful

Proposed by Leonard Richardson
Status: Merged
Approved by: Aaron Bentley
Approved revision: 148
Merged at revision: 148
Proposed branch: lp:~leonardr/lazr.restful/invalid-uri
Merge into: lp:lazr.restful
Diff against target: 66 lines (+26/-4)
3 files modified
src/lazr/restful/example/base/tests/redirect.txt (+8/-0)
src/lazr/restful/example/base/traversal.py (+5/-0)
src/lazr/restful/publisher.py (+13/-4)
To merge this branch: bzr merge lp:~leonardr/lazr.restful/invalid-uri
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+36759@code.launchpad.net

Description of the change

This branch fixes bug 535078.

Let's say you access a web service page by making an Ajax call with your web browser. Let's say the underlying application redirects you to some other page. And let's say the Location header contains invalid characters like {} or [].

Without this branch, lazr.restful will try to parse the URL in that location header using lazr.uri, and lazr.uri will choke on the invalid characters. But it's really none of lazr.restful's business whether the Location header contains invalid characters. This branch changes lazr.restful to use urllib to parse the Location header instead of the more picky lazr.uri library.

To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

In most cases, a browser that follows the Location header will get a 404 error or some other error, because invalid characters in URLs usually mean something's wrong. But, again, it's not lazr.restful's job to raise an exception at this point. With this branch, the error is more likely to be comprehensible to the client.

Revision history for this message
Aaron Bentley (abentley) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/restful/example/base/tests/redirect.txt'
2--- src/lazr/restful/example/base/tests/redirect.txt 2009-04-02 15:44:00 +0000
3+++ src/lazr/restful/example/base/tests/redirect.txt 2010-09-27 17:12:45 +0000
4@@ -34,3 +34,11 @@
5 Location: http://.../cookbooks/Mastering%20the%20Art%20of%20French%20Cooking?ws.accept=application/json
6 ...
7
8+The redirect works even if the redirect URI contains characters not
9+valid in URIs. In this case, the redirect is to a URL that contains
10+curly braces (see traversal.py for details).
11+
12+ >>> print ajax.get("/cookbooks/featured-invalid")
13+ HTTP/1.1 301 Moved Permanently
14+ ...
15+ Location: http://.../Mastering%20the%20Art%20of%20French%20Cooking{invalid}?ws.accept=application/json
16
17=== modified file 'src/lazr/restful/example/base/traversal.py'
18--- src/lazr/restful/example/base/traversal.py 2009-09-03 14:04:03 +0000
19+++ src/lazr/restful/example/base/traversal.py 2010-09-27 17:12:45 +0000
20@@ -67,6 +67,11 @@
21 if name == 'featured':
22 url = absoluteURL(self.context.featured, request)
23 return RedirectResource(url, request)
24+ elif name == 'featured-invalid':
25+ # This is to enable a test case in which the redirect URL
26+ # contains characters not valid in URLs.
27+ url = absoluteURL(self.context.featured, request) + "{invalid}"
28+ return RedirectResource(url, request)
29 else:
30 return super(CookbookSetTraverse, self).publishTraverse(
31 request, name)
32
33=== modified file 'src/lazr/restful/publisher.py'
34--- src/lazr/restful/publisher.py 2010-03-03 13:08:12 +0000
35+++ src/lazr/restful/publisher.py 2010-09-27 17:12:45 +0000
36@@ -17,6 +17,7 @@
37
38 import traceback
39 import urllib
40+import urlparse
41
42 from zope.component import (
43 adapter, getMultiAdapter, getUtility, queryAdapter, queryMultiAdapter)
44@@ -189,11 +190,19 @@
45 accept = request.response.getHeader(
46 "Accept", "application/json")
47 qs_append = "ws.accept=" + urllib.quote(accept)
48- uri = URI(location)
49- if uri.query is None:
50- uri.query = qs_append
51+ # We don't use the URI class because it will raise
52+ # an exception if the Location contains invalid
53+ # characters. Invalid characters may indeed be a
54+ # problem, but let the problem be handled
55+ # somewhere else.
56+ (scheme, netloc, path, query, fragment) = (
57+ urlparse.urlsplit(location))
58+ if query == '':
59+ query = qs_append
60 else:
61- uri.query += '&' + qs_append
62+ query += '&' + qs_append
63+ uri = urlparse.urlunsplit(
64+ (scheme, netloc, path, query, fragment))
65 request.response.setHeader("Location", str(uri))
66 return value
67

Subscribers

People subscribed via source and target branches