Skip to content
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.

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

Host ']' #90

Open
kenballus opened this issue Feb 7, 2023 · 6 comments
Open

Host ']' #90

kenballus opened this issue Feb 7, 2023 · 6 comments

Comments

@kenballus
Copy link
Contributor

The following malformed URL is accepted by rfc3986:

B://]

Although the character ']' is allowed in a host, it must be in the context of an IPv6 or an IPvFuture, which this is not.

This malformed URL is rejected by urllib, urllib3, hyperlink, yarl, furl, and Boost.URL.

@kenballus
Copy link
Contributor Author

Also a problem when host is '['

@sigmavirus24
Copy link
Collaborator

Happy to accept a fix here

@frenzymadness
Copy link

The question here is whether the validation should or should not be implicit when using uri_reference. The truth is that it parses the URI and returns an invalid result by default:

In [1]: from rfc3986 import uri_reference

In [2]: uri_reference("B://]")
Out[2]: URIReference(scheme='B', authority=']', path=None, query=None, fragment=None)

But you can simply check the validity of the result by the is_valid() method:

In [3]: _.is_valid()
Out[3]: False

If you want the validation to happen immediately during parsing, you can use ParseResult:

In [4]: from rfc3986 import ParseResult

In [5]: ParseResult.from_string("B://]")
…
InvalidAuthority: The authority (]) is not valid.

So, because the lib is already able to say that the URL is invalid, it's more a design decision if the validation should happen by default.

@sigmavirus24
Copy link
Collaborator

sigmavirus24 commented Apr 18, 2023

Pretty sure uri_reference was always intended to be non-validating so that the rest could be API compatible with urllib3 more or less.

Maybe we need to make that clearer in the docs

@frenzymadness
Copy link

@kenballus could you please clarify which approach did you use?

@kenballus
Copy link
Contributor Author

My mistake; using ParseResult.from_string fixes this. I should have read the docs more thoroughly.

Why is it that we have a non-validating parser? Some large projects (httpx) use this parser, and would probably be better off using ParseResult.from_string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants