lazr.restful

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

  1. invalid-uri
  2. Merge into trunk
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
=== modified file 'src/lazr/restful/example/base/tests/redirect.txt'
--- src/lazr/restful/example/base/tests/redirect.txt 2009-04-02 15:44:00 +0000
+++ src/lazr/restful/example/base/tests/redirect.txt 2010-09-27 17:12:45 +0000
@@ -34,3 +34,11 @@
34 Location: http://.../cookbooks/Mastering%20the%20Art%20of%20French%20Cooking?ws.accept=application/json34 Location: http://.../cookbooks/Mastering%20the%20Art%20of%20French%20Cooking?ws.accept=application/json
35 ...35 ...
3636
37The redirect works even if the redirect URI contains characters not
38valid in URIs. In this case, the redirect is to a URL that contains
39curly braces (see traversal.py for details).
40
41 >>> print ajax.get("/cookbooks/featured-invalid")
42 HTTP/1.1 301 Moved Permanently
43 ...
44 Location: http://.../Mastering%20the%20Art%20of%20French%20Cooking{invalid}?ws.accept=application/json
3745
=== modified file 'src/lazr/restful/example/base/traversal.py'
--- src/lazr/restful/example/base/traversal.py 2009-09-03 14:04:03 +0000
+++ src/lazr/restful/example/base/traversal.py 2010-09-27 17:12:45 +0000
@@ -67,6 +67,11 @@
67 if name == 'featured':67 if name == 'featured':
68 url = absoluteURL(self.context.featured, request)68 url = absoluteURL(self.context.featured, request)
69 return RedirectResource(url, request)69 return RedirectResource(url, request)
70 elif name == 'featured-invalid':
71 # This is to enable a test case in which the redirect URL
72 # contains characters not valid in URLs.
73 url = absoluteURL(self.context.featured, request) + "{invalid}"
74 return RedirectResource(url, request)
70 else:75 else:
71 return super(CookbookSetTraverse, self).publishTraverse(76 return super(CookbookSetTraverse, self).publishTraverse(
72 request, name)77 request, name)
7378
=== modified file 'src/lazr/restful/publisher.py'
--- src/lazr/restful/publisher.py 2010-03-03 13:08:12 +0000
+++ src/lazr/restful/publisher.py 2010-09-27 17:12:45 +0000
@@ -17,6 +17,7 @@
1717
18import traceback18import traceback
19import urllib19import urllib
20import urlparse
2021
21from zope.component import (22from zope.component import (
22 adapter, getMultiAdapter, getUtility, queryAdapter, queryMultiAdapter)23 adapter, getMultiAdapter, getUtility, queryAdapter, queryMultiAdapter)
@@ -189,11 +190,19 @@
189 accept = request.response.getHeader(190 accept = request.response.getHeader(
190 "Accept", "application/json")191 "Accept", "application/json")
191 qs_append = "ws.accept=" + urllib.quote(accept)192 qs_append = "ws.accept=" + urllib.quote(accept)
192 uri = URI(location)193 # We don't use the URI class because it will raise
193 if uri.query is None:194 # an exception if the Location contains invalid
194 uri.query = qs_append195 # characters. Invalid characters may indeed be a
196 # problem, but let the problem be handled
197 # somewhere else.
198 (scheme, netloc, path, query, fragment) = (
199 urlparse.urlsplit(location))
200 if query == '':
201 query = qs_append
195 else:202 else:
196 uri.query += '&' + qs_append203 query += '&' + qs_append
204 uri = urlparse.urlunsplit(
205 (scheme, netloc, path, query, fragment))
197 request.response.setHeader("Location", str(uri))206 request.response.setHeader("Location", str(uri))
198 return value207 return value
199208

Subscribers

People subscribed via source and target branches