Merge lp:~cjwatson/launchpad/less-greedy-sanitise-urls into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18580
Proposed branch: lp:~cjwatson/launchpad/less-greedy-sanitise-urls
Merge into: lp:launchpad
Diff against target: 59 lines (+25/-3)
2 files modified
lib/lp/services/tests/test_utils.py (+23/-1)
lib/lp/services/utils.py (+2/-2)
To merge this branch: bzr merge lp:~cjwatson/launchpad/less-greedy-sanitise-urls
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve
Launchpad code reviewers Pending
Review via email: mp+341962@code.launchpad.net

Commit message

Make sanitise_urls match usernames and passwords non-greedily.

Description of the change

Otherwise log lines that contain multiple URLs the second or later of which requires sanitisation become astonishingly confusing.

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/services/tests/test_utils.py'
2--- lib/lp/services/tests/test_utils.py 2018-02-14 11:13:47 +0000
3+++ lib/lp/services/tests/test_utils.py 2018-03-23 12:59:39 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Tests for lp.services.utils."""
10@@ -33,6 +33,7 @@
11 load_bz2_pickle,
12 obfuscate_structure,
13 run_capturing_output,
14+ sanitise_urls,
15 save_bz2_pickle,
16 traceback_info,
17 utc_now,
18@@ -383,3 +384,24 @@
19 """Values are obfuscated recursively."""
20 obfuscated = obfuscate_structure({'foo': (['a@example.com'],)})
21 self.assertEqual({'foo': [['<email address hidden>']]}, obfuscated)
22+
23+
24+class TestSanitiseURLs(TestCase):
25+
26+ def test_already_clean(self):
27+ self.assertEqual('clean', sanitise_urls('clean'))
28+
29+ def test_removes_credentials(self):
30+ self.assertEqual(
31+ 'http://<redacted>@example.com/',
32+ sanitise_urls('http://user:secret@example.com/'))
33+
34+ def test_non_greedy(self):
35+ self.assertEqual(
36+ '{"one": "http://example.com/", '
37+ '"two": "http://<redacted>@example.com/", '
38+ '"three": "http://<redacted>@example.org/"}',
39+ sanitise_urls(
40+ '{"one": "http://example.com/", '
41+ '"two": "http://alice:secret@example.com/", '
42+ '"three": "http://bob:hidden@example.org/"}'))
43
44=== modified file 'lib/lp/services/utils.py'
45--- lib/lp/services/utils.py 2017-12-19 17:16:38 +0000
46+++ lib/lp/services/utils.py 2018-03-23 12:59:39 +0000
47@@ -1,4 +1,4 @@
48-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
49+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
50 # GNU Affero General Public License version 3 (see the file LICENSE).
51
52 """Generic Python utilities.
53@@ -382,5 +382,5 @@
54 example). This function removes them.
55 """
56 # Remove credentials from URLs.
57- password_re = re.compile('://([^:]*:[^@]*@)(\S+)')
58+ password_re = re.compile('://([^:@/]*:[^@/]*@)(\S+)')
59 return password_re.sub(r'://<redacted>@\2', s)