launchpad-reviewers team mailing list archive

[Roman Yepishev <roman.yepishev@xxxxxxxxxxxxx>]

Roman Yepishev has proposed merging lp:~rye/python-oops-tools/trim-url-lp915335 into lp:python-oops-tools.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #915335 in python-oops-tools: "OOPS loading will fail if URL is longer than allowed 500 characters"
  https://bugs.launchpad.net/python-oops-tools/+bug/915335

For more details, see:
https://code.launchpad.net/~rye/python-oops-tools/trim-url-lp915335/+merge/116659

This fixes the issue when a broken URL expands to more than 500 (MAX_URL_LEN) when quoted and makes the database unhappy.
-- 
https://code.launchpad.net/~rye/python-oops-tools/trim-url-lp915335/+merge/116659
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rye/python-oops-tools/trim-url-lp915335 into lp:python-oops-tools.

=== modified file 'src/oopstools/oops/models.py'
--- src/oopstools/oops/models.py	2011-12-08 04:57:25 +0000
+++ src/oopstools/oops/models.py	2012-07-25 14:28:57 +0000
@@ -433,7 +433,7 @@
     # Trim most_expensive_statements to 200 characters
     if most_expensive_statement is not None:
         most_expensive_statement = conform(most_expensive_statement, 200)
-    url = conform(oops.get('url') or '', MAX_URL_LEN)
+    url = oops.get('url') or ''
     if type(url) is unicode:
         # We have gotten a ringer, URL's are bytestrings. Encode to UTF8 to get
         # a bytestring and urllib.quote to get a url.
@@ -447,6 +447,7 @@
             # 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)
+    url = conform(url, MAX_URL_LEN)
 
     informational = oops.get('informational', 'False').lower() == 'true'
     oops_date = oops.get('time')

=== modified file 'src/oopstools/oops/test/test_dboopsloader.py'
--- src/oopstools/oops/test/test_dboopsloader.py	2011-11-17 16:11:12 +0000
+++ src/oopstools/oops/test/test_dboopsloader.py	2012-07-25 14:28:57 +0000
@@ -36,6 +36,7 @@
     DBOopsRootDirectory,
     Oops,
     parsed_oops_to_model_oops,
+    MAX_URL_LEN
     )
 
 
@@ -253,3 +254,18 @@
         }
         oops = parsed_oops_to_model_oops(report, 'bug-891647')
         self.assertEqual('APPSERVER', oops.appinstance.title)
+
+    def test_long_url_bug_915335(self):
+        # URLs that are longer than 500 characters should be truncated
+        # at all times.
+        url = 'http://' + '\xFF' * 250
+
+        report = {
+            'id': 'OOPS-68383510f5054932567230492013ce91',
+            'url': url
+        }
+
+        oops = parsed_oops_to_model_oops(report, 'bug-915335')
+        oops.save()
+        self.assertTrue(len(oops.url) < MAX_URL_LEN)
+