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

[Robert Collins]

Handle bad url bytestrings by url quoting. Also release 0.6.1.

A followon-from https://code.launchpad.net/~rye/python-oops-tools/local-charset-in-url/+merge/80330 -- I've implemented StevenK's points, and mine.

[Robert Collins (lifeless)]

Also includes a release.

review: Approve

=== modified file 'src/oopstools/NEWS.txt'
--- src/oopstools/NEWS.txt	2011-10-27 03:40:45 +0000
+++ src/oopstools/NEWS.txt	2011-10-31 02:12:23 +0000
@@ -5,6 +5,9 @@
 NEXT
 ====
 
+0.6.1
+=====
+
 * Added AMQP support via the bin/amqp2disk script. (Robert Collins)
 
 * Bumped oops-amqp rev to 0.0.3 for bugfixes. (Robert Collins)
@@ -15,7 +18,11 @@
 * amqp2disk -v will print the received OOPS ids on the console, for
   entertainment and delight. (Robert Collins)
 
-* OOPS reports with unicode url values are now handled during oops loading: the
+* OOPS reports with non-ascii URL values are handled by url escaping the URL
+  bytestring (this is separate to handling of unicode URL values).
+  (Roman Yepishev, Robert Collins, #881400)
+
+* OOPS reports with unicode URL values are now handled during oops loading: the
   unicode string is utf8 encoded (an arbitrary choice) and url escaped.
   (Robert Collins, #879309)
 

=== modified file 'src/oopstools/oops/models.py'
--- src/oopstools/oops/models.py	2011-10-25 02:25:14 +0000
+++ src/oopstools/oops/models.py	2011-10-31 02:12:23 +0000
@@ -371,6 +371,16 @@
         # We have gotten a ringer, URL's are bytestrings. Encode to UTF8 to get
         # a bytestring and urllib.quote to get a url.
         url = urllib.quote(url.encode('utf8'))
+    else:
+        try:
+            url.decode('ascii')
+        except UnicodeDecodeError:
+            # The URL is not ASCII and thus definitely not a URL.
+            # There may be a byte in local encoding or it may be UTF8 - we need
+            # to quote it to make it a valid URL - this is better than
+            # rejecting the OOPS, or having a URL that isn't in the DB.
+            url = urllib.quote(url)
+
     informational = oops.get('informational', 'False').lower() == 'true'
     oops_date = oops.get('time')
     if oops_date is None:

=== modified file 'src/oopstools/oops/test/test_dboopsloader.py'
--- src/oopstools/oops/test/test_dboopsloader.py	2011-10-24 05:21:01 +0000
+++ src/oopstools/oops/test/test_dboopsloader.py	2011-10-31 02:12:23 +0000
@@ -154,3 +154,14 @@
         report = { 'url': 'foo', 'id': 'testnotopichandling'}
         oops = parsed_oops_to_model_oops(report, 'bug_880641')
         self.assertEqual('', oops.pageid)
+
+    def test_broken_url_handling(self):
+        # This URL is not a valid URL - URL's are a subset of ASCII.
+        broken_url = '/somep\xe1th'
+        report = { 'url': broken_url, 'id': 'testbrokenurl'}
+        # We handle such URL's by url quoting them, failing to do so being a
+        # (not uncommon) producer mistake.
+        expected_url = urllib.quote(broken_url)
+        oops = parsed_oops_to_model_oops(report, 'test_broken_url_handling')
+        self.assertEqual(expected_url, oops.url)
+

=== modified file 'src/oopstools/version.txt'
--- src/oopstools/version.txt	2011-10-13 20:18:51 +0000
+++ src/oopstools/version.txt	2011-10-31 02:12:23 +0000
@@ -1,1 +1,1 @@
-0.6
+0.6.1