Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 27 additions & 0 deletions Lib/test/test_urlparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,15 @@ def test_splittype(self):
self.assertEqual(splittype('type:'), ('type', ''))
self.assertEqual(splittype('type:opaque:string'), ('type', 'opaque:string'))

# bpo-30713: The newline character U+000A is invalid in URLs
for url in (
'\ntype:string',
'ty\npe:string',
):
self.assertEqual(splittype(url), (None, url))
self.assertEqual(splittype('data:xxx\nyyy'), ('data', 'xxx\nyyy'))
self.assertEqual(splittype('data:xxxyyy\n'), ('data', 'xxxyyy\n'))

def test_splithost(self):
splithost = urllib.parse.splithost
self.assertEqual(splithost('//www.example.org:80/foo/bar/baz.html'),
Expand Down Expand Up @@ -1010,6 +1019,15 @@ def test_splithost(self):
self.assertEqual(splithost("//example.net/file#"),
('example.net', '/file#'))

# bpo-30713: The newline character U+000A is invalid in URLs
for url in (
'\n//hostname/url',
'//host\nname/url',
'//hostname/u\nrl',
'//hostname/url\n',
):
self.assertEqual(splithost(url), (None, url))

def test_splituser(self):
splituser = urllib.parse.splituser
self.assertEqual(splituser('User:[email protected]:080'),
Expand Down Expand Up @@ -1052,6 +1070,15 @@ def test_splitport(self):
self.assertEqual(splitport('[::1]'), ('[::1]', None))
self.assertEqual(splitport(':88'), ('', '88'))

# bpo-30713: The newline character U+000A is invalid in URLs
for url in (
'\nparrot:88',
'par\nrot:88',
'parrot:8\n8',
'parrot:88\n',
):
self.assertEqual(splitport(url), (url, None))

def test_splitnport(self):
splitnport = urllib.parse.splitnport
self.assertEqual(splitnport('parrot:88'), ('parrot', 88))
Expand Down
12 changes: 6 additions & 6 deletions Lib/urllib/parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -934,9 +934,9 @@ def splittype(url):
"""splittype('type:opaquestring') --> 'type', 'opaquestring'."""
global _typeprog
if _typeprog is None:
_typeprog = re.compile('([^/:]+):(.*)', re.DOTALL)
_typeprog = re.compile('([^/:\n]+):(.*)', re.DOTALL)
Copy link
Member

Choose a reason for hiding this comment

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

What is wrong with '\n' in a type? In any case it is compared with fixed set of supported schemes.

Copy link
Member Author

Choose a reason for hiding this comment

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

What is wrong with '\n' in a type?

It doesn't make sense to me to have a newline in a type, it really looks like an attempt to compromise a server.


match = _typeprog.match(url)
match = _typeprog.fullmatch(url)
Copy link
Member

Choose a reason for hiding this comment

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

There is no difference between match() and fullmatch() here.

if match:
scheme, data = match.groups()
return scheme.lower(), data
Expand All @@ -947,9 +947,9 @@ def splithost(url):
"""splithost('//host[:port]/path') --> 'host[:port]', '/path'."""
global _hostprog
if _hostprog is None:
_hostprog = re.compile('//([^/#?]*)(.*)', re.DOTALL)
_hostprog = re.compile('//([^/#?\n]*)(.*)')
Copy link
Member

Choose a reason for hiding this comment

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

This will caused just returning a tuple (None, url) as in the case when the host is not specified. The second item can contain '\n'. This doesn't mean splithost() rejects the url.

If you want to make splithost() rejecting URLs with newlines, check explicitly '\n' in url and raise an exception. But I think this is not the best place of doing such checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't mean splithost() rejects the url.

Right now, we don't raise an exception if an URL looks "invalid". So I tried to fit into the current behaviour: return (None, invalid_url). Maybe we should raise an exception instead?


match = _hostprog.match(url)
match = _hostprog.fullmatch(url)
if match:
host_port, path = match.groups()
if path and path[0] != '/':
Expand All @@ -973,9 +973,9 @@ def splitport(host):
"""splitport('host:port') --> 'host', 'port'."""
global _portprog
if _portprog is None:
_portprog = re.compile('(.*):([0-9]*)$', re.DOTALL)
_portprog = re.compile('(.*):([0-9]*)')
Copy link
Member

Choose a reason for hiding this comment

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

The same as for splithost(). splithost('example.org\n') will return a tuple ('example.org\n', None).


match = _portprog.match(host)
match = _portprog.fullmatch(host)
if match:
host, port = match.groups()
if port:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
The splittype(), splitport() and splithost() functions of the urllib.parse
module now reject URLs which contain a newline character, but splittype()
accepts newlines after the type.