bug report (fuzz testing)

[kennethreitz]

'http+unix://%2Fvar%2Frun%2Fsocket/path?key=value'

rfc3986/src/rfc3986/abnf_regexp.py

Line 74 in 4102c50

REGULAR_NAME_RE = REG_NAME = '(({0})*|[{1}]*)'.format(

[sethmlarson]

@sigmavirus24 Took a quick look at this and tried it locally. It parses the host and leaves the percent-encoded-ness of host as %2Fvar%2Frun%2Fsocket.

Should this issue be fixed by percent decoding the host section within authority when normalize() is called? If this is done then we'll receive http+unix:///var/run/socket/path?key=value if we call unsplit(). Should we only provide the percent-decoded host when that specific property is asked for?

[sigmavirus24]

I think %2Fvar%2Frun%2Fsocket is actually the host and should be parsed as that. I don't recall what @kennethreitz was expecting here. This is no longer something I remember as to why it's a bug :/

[sethmlarson]

I can investigate what the behavior was around when this issue was filed. I think how it's parsing right now is actually correct. Users should have decode percent-encoded hosts.

[sethmlarson]

Looks like it parses the same now as when this issue was filed, there hasn't been a release since July 2017.

[sigmavirus24]

What happens if we do rfc3986.urlparse('http+unix://%2Fvar%2Frun%2Fsocket/path?key=value').uri_reference.normalize()? I think I found notes that we were losing the host at some point somewhere?

[sethmlarson]

Glad you found some notes!

>>> rfc3986.urlparse('http+unix://%2Fvar%2Frun%2Fsocket/path?key=value').reference.normalize()
URIReference(scheme='http+unix', authority=None, path='/path', query='key=value', fragment=None)

So we're losing the whole authority section, that's definitely not good. I'll look at this.

Edit: Running this results in an InvalidAuthority exception which points to potential problem with rfc3986.misc.SUBAUTHORITY_MATCHER.

>>> rfc3986.urlparse('http+unix://%2Fvar%2Frun%2Fsocket/path?key=value').reference.authority_info()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/slarson/.pyenv/versions/3.7.0/lib/python3.7/site-packages/rfc3986/uri.py", line 159, in authority_info
    raise exc.InvalidAuthority(self.authority.encode(self.encoding))
rfc3986.exceptions.InvalidAuthority: The authority (b'%2Fvar%2Frun%2Fsocket') is not valid.

[sigmavirus24]

Yeah, so it's entirely likely that I had pinpointed this to a problem with REG_NAME or perhaps SUBAUTHORITY_MATCHER isn't using it?

[sethmlarson]

Would a pattern like:

REGULAR_NAME_RE = REG_NAME = '((?:{0}|[{1}])*)'.format(
    '%[0-9A-Fa-f]{2}', SUB_DELIMITERS_RE + UNRESERVED_RE
)
# Equal to: ((?:%[0-9A-Fa-f]{2}|[!$&'()\*+,;=A-Za-z0-9._~\-])*)

be more acceptable? Currently REG_NAME is

r'((%[0-9A-Fa-f]{2})*|[!$&'()\*+,;=A-Za-z0-9._~\-]*)'

Which looks like it's forcing either all percent-encoded bytes or all non-percent-encoded bytes which is why we're having trouble with this hostname with both.

I'll run some tests on this tonight but I think this is the issue.

[sethmlarson]

Also side-note: I think we're being too lax on the port section within the authority regex. If there's a ":" character without a port following that shouldn't be a valid authority per the RFC.

[sigmavirus24]

That seems acceptable. I wonder if we need tests specifically for the regular expressions to show that they work the way we intended when we write them 🙃

I'm open to being more strict about URIs too but I think we might want to open a separate issue for that.

[sethmlarson]

Definitely. I'm going to give all the regular expressions another look and see if I see anything else out of place. Maybe even run an ABNF -> regex tool and compare what comes out with what we have? I'll open an issue with all my findings.

[sigmavirus24]

ABNF -> Regex tools tend to reduce the understandability with respect to maintenance. Hence why I tried my best to make things understandable the way they are.

[sethmlarson]

I'll iterate on what we currently have. An idea might be to use ABNF -> regex and plug that into hypothesis.strategies.from_regex() and see what interesting inputs are created?