Skip to content

[3.8] bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. (GH-25595) #25726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ambv merged 5 commits into from
May 5, 2021
Merged

[3.8] bpo-43882 - urllib.parse should sanitize urls containing ASCII newline and tabs. (GH-25595) #25726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
56cdbe0
bpo-43882 - urllib.parse should sanitize urls containing ASCII newlin…
orsenthil Apr 29, 2021
957ee9d
Update version to 3.8.10 for 3.8 branch
orsenthil Apr 29, 2021
9a1d1f7
Fix 3.8 tests.
orsenthil Apr 30, 2021
ce95fae
Address Review Comment. Strip Unsafe Chars very early, and include ca…
orsenthil May 3, 2021
94da00c
Address review comments.
orsenthil May 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address Review Comment. Strip Unsafe Chars very early, and include ca… 
…che key.
  • Loading branch information
@orsenthil
orsenthil committed May 3, 2021
commit ce95fae2e673a633e315322c54735629e371d7ea
33 changes: 21 additions & 12 deletions Lib/test/test_urlparse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,42 +614,51 @@ def test_urlsplit_attributes(self):

def test_urlsplit_remove_unsafe_bytes(self):
# Remove ASCII tabs and newlines from input, for http common case scenario.
url = "http://www.python.org/java\nscript:\talert('msg\r\n')/#frag"
url = "h\nttp://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment"
p = urllib.parse.urlsplit(url)
self.assertEqual(p.scheme, "http")
self.assertEqual(p.netloc, "www.python.org")
self.assertEqual(p.path, "/javascript:alert('msg')/")
self.assertEqual(p.query, "")
self.assertEqual(p.fragment, "frag")
self.assertEqual(p.query, "query=something")
self.assertEqual(p.fragment, "fragment")
self.assertEqual(p.username, None)
self.assertEqual(p.password, None)
self.assertEqual(p.hostname, "www.python.org")
self.assertEqual(p.port, None)
self.assertEqual(p.geturl(), "http://www.python.org/javascript:alert('msg')/#frag")
self.assertEqual(p.geturl(), "http://www.python.org/javascript:alert('msg')/?query=something#fragment")

# Remove ASCII tabs and newlines from input as bytes, for http common case scenario.
url = b"http://www.python.org/java\nscript:\talert('msg\r\n')/#frag"
url = b"h\nttp://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment"
p = urllib.parse.urlsplit(url)
self.assertEqual(p.scheme, b"http")
self.assertEqual(p.netloc, b"www.python.org")
self.assertEqual(p.path, b"/javascript:alert('msg')/")
self.assertEqual(p.query, b"")
self.assertEqual(p.fragment, b"frag")
self.assertEqual(p.query, b"query=something")
self.assertEqual(p.fragment, b"fragment")
self.assertEqual(p.username, None)
self.assertEqual(p.password, None)
self.assertEqual(p.hostname, b"www.python.org")
self.assertEqual(p.port, None)
self.assertEqual(p.geturl(), b"http://www.python.org/javascript:alert('msg')/#frag")
self.assertEqual(p.geturl(), b"http://www.python.org/javascript:alert('msg')/?query=something#fragment")

# any scheme
url = "x-new-scheme://www.python.org/java\nscript:\talert('msg\r\n')/#frag"
url = "x-new-scheme\t://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment"
p = urllib.parse.urlsplit(url)
self.assertEqual(p.geturl(), "x-new-scheme://www.python.org/javascript:alert('msg')/#frag")
self.assertEqual(p.geturl(), "x-new-scheme://www.python.org/javascript:alert('msg')/?query=something#fragment")

# Remove ASCII tabs and newlines from input as bytes, any scheme.
url = b"x-new-scheme://www.python.org/java\nscript:\talert('msg\r\n')/#frag"
url = b"x-new-scheme\t://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment"

@orsenthil orsenthil May 3, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated test cases to verify removal in all parts of the URL.

p = urllib.parse.urlsplit(url)
self.assertEqual(p.geturl(), b"x-new-scheme://www.python.org/javascript:alert('msg')/#frag")
self.assertEqual(p.geturl(), b"x-new-scheme://www.python.org/javascript:alert('msg')/?query=something#fragment")

# Unsafe bytes is not returned from urlparse cache.
# scheme is stored after parsing, sending an scheme with unsafe bytes *will not* return an unsafe scheme
url = "https://www.python.org/java\nscript:\talert('msg\r\n')/?query\n=\tsomething#frag\nment"
scheme = "htt\nps"
Comment thread
gpshead marked this conversation as resolved.
for _ in range(2):
p = urllib.parse.urlsplit(url, scheme=scheme)
self.assertEqual(p.scheme, "https")
self.assertEqual(p.geturl(), "https://www.python.org/javascript:alert('msg')/?query=something#fragment")

def test_attributes_bad_port(self):
"""Check handling of invalid ports."""
Expand Down
5 changes: 2 additions & 3 deletions Lib/urllib/parse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,8 @@ def urlsplit(url, scheme='', allow_fragments=True):
Note that we don't break the components up in smaller bits
(e.g. netloc is a single string) and we don't expand % escapes."""
url, scheme, _coerce_result = _coerce_args(url, scheme)
url = _remove_unsafe_bytes_from_url(url)
scheme = _remove_unsafe_bytes_from_url(scheme)
Comment thread
gpshead marked this conversation as resolved.
allow_fragments = bool(allow_fragments)
key = url, scheme, allow_fragments, type(url), type(scheme)
cached = _parse_cache.get(key, None)
Expand All @@ -451,7 +453,6 @@ def urlsplit(url, scheme='', allow_fragments=True):
if '?' in url:
url, query = url.split('?', 1)
_checknetloc(netloc)
url = _remove_unsafe_bytes_from_url(url)
v = SplitResult('http', netloc, url, query, fragment)
_parse_cache[key] = v
return _coerce_result(v)
Expand All @@ -466,8 +467,6 @@ def urlsplit(url, scheme='', allow_fragments=True):
# not a port number
scheme, url = url[:i].lower(), rest

url = _remove_unsafe_bytes_from_url(url)

if url[:2] == '//':
netloc, url = _splitnetloc(url, 2)
if (('[' in netloc and ']' not in netloc) or
Expand Down