launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/less-greedy-sanitise-urls into lp:launchpad.

Commit message:
Make sanitise_urls match usernames and passwords non-greedily.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/less-greedy-sanitise-urls/+merge/341962

Otherwise log lines that contain multiple URLs the second or later of which requires sanitisation become astonishingly confusing.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/less-greedy-sanitise-urls into lp:launchpad.

=== modified file 'lib/lp/services/tests/test_utils.py'
--- lib/lp/services/tests/test_utils.py	2018-02-14 11:13:47 +0000
+++ lib/lp/services/tests/test_utils.py	2018-03-23 12:59:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for lp.services.utils."""
@@ -33,6 +33,7 @@
     load_bz2_pickle,
     obfuscate_structure,
     run_capturing_output,
+    sanitise_urls,
     save_bz2_pickle,
     traceback_info,
     utc_now,
@@ -383,3 +384,24 @@
         """Values are obfuscated recursively."""
         obfuscated = obfuscate_structure({'foo': (['a@xxxxxxxxxxx'],)})
         self.assertEqual({'foo': [['<email address hidden>']]}, obfuscated)
+
+
+class TestSanitiseURLs(TestCase):
+
+    def test_already_clean(self):
+        self.assertEqual('clean', sanitise_urls('clean'))
+
+    def test_removes_credentials(self):
+        self.assertEqual(
+            'http://<redacted>@example.com/',
+            sanitise_urls('http://user:secret@xxxxxxxxxxx/'))
+
+    def test_non_greedy(self):
+        self.assertEqual(
+            '{"one": "http://example.com/";, '
+            '"two": "http://<redacted>@example.com/", '
+            '"three": "http://<redacted>@example.org/"}',
+            sanitise_urls(
+                '{"one": "http://example.com/";, '
+                '"two": "http://alice:secret@xxxxxxxxxxx/";, '
+                '"three": "http://bob:hidden@xxxxxxxxxxx/"}'))

=== modified file 'lib/lp/services/utils.py'
--- lib/lp/services/utils.py	2017-12-19 17:16:38 +0000
+++ lib/lp/services/utils.py	2018-03-23 12:59:39 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Generic Python utilities.
@@ -382,5 +382,5 @@
     example).  This function removes them.
     """
     # Remove credentials from URLs.
-    password_re = re.compile('://([^:]*:[^@]*@)(\S+)')
+    password_re = re.compile('://([^:@/]*:[^@/]*@)(\S+)')
     return password_re.sub(r'://<redacted>@\2', s)