URI parsing

[kshpytsya]

$ python3
Python 3.6.4 (default, Jan  6 2018, 02:24:08)
[GCC 7.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import packaging.requirements
>>> packaging.requirements.Requirement("pip @ file://localhost/localbuilds/pip-1.3.1-py33-none-any.whl")
<Requirement('pip@ file://localhost/localbuilds/pip-1.3.1-py33-none-any.whl')>
>>> packaging.requirements.Requirement("pip @ file:///localbuilds/pip-1.3.1-py33-none-any.whl")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/uken/.local/lib64/python3.6/site-packages/packaging/requirements.py", line 101, in __init__
    raise InvalidRequirement("Invalid URL given")
packaging.requirements.InvalidRequirement: Invalid URL given

According to PEP 508 the following strings should all be parseable:

"pip @ file://localhost/localbuilds/pip-1.3.1-py33-none-any.whl"
"pip @ file:///localbuilds/pip-1.3.1-py33-none-any.whl"
"pip @ /localbuilds/pip-1.3.1-py33-none-any.whl"
"pip @ localbuilds/pip-1.3.1-py33-none-any.whl"

(I have fed them into the "test program" included in PEP 508 that uses Parsley for parsing. Note that there are several typos in the grammar in the document text)

The following logic may be flawed:

packaging/packaging/requirements.py

Lines 102 to 104 in d2ed39a

	if not (parsed_url.scheme and parsed_url.netloc) or (
	not parsed_url.scheme and not parsed_url.netloc):
	raise InvalidRequirement("Invalid URL given")

[jaraco]

Looking at what urllib does there:

>>> urllib.parse.urlparse('file:///localbuilds/pip-1.3.1')
ParseResult(scheme='file', netloc='', path='/localbuilds/pip-1.3.1', params='', query='', fragment='')
>>> urllib.parse.urlparse('file://localbuilds/pip-1.3.1')
ParseResult(scheme='file', netloc='localbuilds', path='/pip-1.3.1', params='', query='', fragment='')

It does seem that urllib.parse.urlparse may not be doing what is intended by that code (localbuilds is not a netloc).

[benoit-pierre]

Isn't the urllib.parse behaviour valid? According to RFC 8089, file URIs support an optional (and possibly empty authority), so file:///etc/fstab, file://localhost/etc/fstab and file:/etc/fstab all refer to the same location (but file:///localbuilds/pip-1.3.1 and file://localbuilds/pip-1.3.1 are not equivalent).

[cjerdonek]

There are at least a couple open issues around urllib.parse's parsing I know of:
https://bugs.python.org/issue22852
https://bugs.python.org/issue34276
They may or may not be related to what you are discussing here.

[Qix-]

Please see pypa/pip#6658. I just wasted several hours breaking down this issue to its fullest as it pertains to pip in particular.

The logic and error reporting can be drastically improved in this package, and I second @jaraco here - six's URL parsing library (or whatever it's using under the hood) is NOT compliant by allowing file:./foo/bar for example.

@benoit-pierre is 100% correct (#120 (comment)).

[cjerdonek]

Has an issue been filed with six?

[Qix-]

Nope. I'll open one. However, the error reporting can still be improved here. I might have time to open a PR today, we'll see :)

[pradyunsg]

@Qix- could you find some time to file an issue with six?

[Qix-]

@pradyunsg Yep, sorry for the delay.

benjaminp/six#300

[pradyunsg]

Okay, it seems like the bug originates from the standard library. Do we want to work around what the standard library does?

[Qix-]

I just tried the test parser with the standard library and the issue persists. Six is not bugged.

import urllib.parse as urllib_parse

def tryparse(url):
    print(url)
    parsed = urllib_parse.urlparse(url)
    unparsed = urllib_parse.urlunparse(parsed)
    parsed_again = urllib_parse.urlparse(unparsed)
    print(parsed)
    print(unparsed)
    print(parsed_again)

>>> tryparse('file:./foo/bar')
file:./foo/bar
ParseResult(scheme='file', netloc='', path='./foo/bar', params='', query='', fragment='')
file:///./foo/bar
ParseResult(scheme='file', netloc='', path='/./foo/bar', params='', query='', fragment='')

---

Looking at RFC 8089, it pretty clearly does not support local paths. This is something that I missed earlier. This means that, currently, a netloc and absolute path are required if the file: scheme is used. The absolute path bit was what was throwing me off, but this issue also points out the requirement of netloc which isn't right either.