python-oops-tools

Merge lp:~lifeless/python-oops-tools/bug-884265 into lp:python-oops-tools

  1. bug-884265
  2. Merge into trunk
Proposed by Robert Collins
Status: Merged
Approved by: Robert Collins
Approved revision: 14
Merged at revision: 13
Proposed branch: lp:~lifeless/python-oops-tools/bug-884265
Merge into: lp:python-oops-tools
Diff against target: 62 lines (+20/-4)
3 files modified
src/oopstools/NEWS.txt (+2/-1)
src/oopstools/oops/models.py (+8/-0)
src/oopstools/oops/test/test_dboopsloader.py (+10/-3)
To merge this branch: bzr merge lp:~lifeless/python-oops-tools/bug-884265
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+81932@code.launchpad.net

Commit message

Handle non-ascii HTTP headers and so forth by url escaping.

Description of the change

Fixup rye's req_vars branch and adjust per wgrants suggestion.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/oopstools/NEWS.txt'
2--- src/oopstools/NEWS.txt 2011-11-02 23:19:28 +0000
3+++ src/oopstools/NEWS.txt 2011-11-11 03:32:27 +0000
4@@ -12,7 +12,8 @@
5 OOPS-prefix (if they had one). (Robert Collins, #884571)
6
7 * OOPS reports that don't meet the normal rules for req_vars are handled
8- a bit better (Robert Collins, William Grant, #885416)
9+ a bit better.
10+ (Robert Collins, William Grant, Roman Yepishev, #885416, #884265)
11
12 0.6.1
13 =====
14
15=== modified file 'src/oopstools/oops/models.py'
16--- src/oopstools/oops/models.py 2011-11-02 20:25:00 +0000
17+++ src/oopstools/oops/models.py 2011-11-11 03:32:27 +0000
18@@ -315,6 +315,14 @@
19 value = ''
20 yield key, value
21 for key, value in ensure_tuple(req_vars):
22+ if isinstance(value, str):
23+ try:
24+ # We can get anything in HTTP headers
25+ # Trying to squeeze the bytes into ASCII plane
26+ value.decode('ascii')
27+ except UnicodeDecodeError:
28+ # If that fails, percent-quote them
29+ value = urllib.quote(value)
30 if key == 'REQUEST_METHOD':
31 # Truncate the REQUEST_METHOD if it's longer than 10 characters
32 # since a str longer than that is not allowed in the DB.
33
34=== modified file 'src/oopstools/oops/test/test_dboopsloader.py'
35--- src/oopstools/oops/test/test_dboopsloader.py 2011-11-02 20:25:00 +0000
36+++ src/oopstools/oops/test/test_dboopsloader.py 2011-11-11 03:32:27 +0000
37@@ -155,15 +155,22 @@
38 oops = parsed_oops_to_model_oops(report, 'bug_880641')
39 self.assertEqual('', oops.pageid)
40
41- def test_broken_url_handling(self):
42- # This URL is not a valid URL - URL's are a subset of ASCII.
43+ def test_broken_header_handling(self):
44 broken_url = '/somep\xe1th'
45- report = { 'url': broken_url, 'id': 'testbrokenurl'}
46+ user_agent = '\xf8\x07\x9e'
47+ report = { 'url': broken_url,
48+ 'id': 'testbrokenheader',
49+ 'req_vars': [
50+ [ 'HTTP_USER_AGENT', user_agent ]
51+ ]
52+ }
53 # We handle such URL's by url quoting them, failing to do so being a
54 # (not uncommon) producer mistake.
55 expected_url = urllib.quote(broken_url)
56+ expected_user_agent = urllib.quote(user_agent)
57 oops = parsed_oops_to_model_oops(report, 'test_broken_url_handling')
58 self.assertEqual(expected_url, oops.url)
59+ self.assertEqual(expected_user_agent, oops.user_agent)
60
61 def test_broken_reqvars_bug_885416(self):
62 # If the tuples in req_vars have only one element the oops still needs

Subscribers

People subscribed via source and target branches

to all changes: