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

Allow empty authority in absolute URIs #698

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andrewbaxter
Copy link

Fixes #323

This removes the restriction preventing empty authority in URIs. The canonical example of this is file:/// which has a zero-length authority. While these may not be relevant for a web server, there are many domains that need these sorts of alternative URIs (for example Webview-based software which reads local resources).

https://datatracker.ietf.org/doc/html/rfc3986 pretty clearly describes the authority as optional in all situations AFAICT, but it mentions that schema-specific restrictions may be in place.

This is the case for HTTP addresses, described in https://www.rfc-editor.org/rfc/rfc9110.html#name-identifiers-in-http

The origin server for an "https" URI is identified by the authority component
...
A sender MUST NOT generate an "https" URI with an empty host identifier. A recipient that processes such a URI reference MUST reject it as invalid.

It's possible that some users of this library in a strictly-http context may be relying on this checking the validity of the HTTP identifier for them. I'm not sure how to handle this, maybe with a version bump? The legacy behavior could be considered a bug though.

Eventually a separate HttpUri type or something that performs the additional checks when constructed might be useful.

@seanmonstar
Copy link
Member

I'm not immediately against this change, but I am a bit skeptical. Notably, this crate is purposefully meant for the HTTP usecase, not a general URL parser. (HttpUri seems like it already exists, http::Uri).

@seanmonstar
Copy link
Member

Also, recently opened: #696.

@andrewbaxter
Copy link
Author

andrewbaxter commented May 10, 2024

FWIW I opened this because I thought @carllerche had given the okay for it in #323 , but you're right, this is http::Uri after all.

Despite varied requirements, the rust ecosystem still hasn't seen a second Uri crate and this is used pretty widely - I'm not sure the ecosystem could support another implementation of a Uri crate. Is there a way some of the core functionality could be shared with another crate with a more permissive frontend in such a way that the types would be trivially convertible?

On the other hand, I don't think any software relies on this check for correct operation... I think this error is at most used to detect user mistakes early. And in that case, I think other code that consumes Uri probably already performs such a check - for instance, "".to_socket_addrs().unwrap() returns Error { kind: InvalidInput, message: "invalid socket address" } which conveys more or less the same information. From a user perspective you'd troubleshoot "" the same way you'd troubleshoot any other incorrect host name.

Copy link

@PawelBis PawelBis left a comment

Choose a reason for hiding this comment

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

Great work. Can we get this merged? It is really necessary for webview.

PawelBis added a commit to PawelBis/http that referenced this pull request Jul 18, 2024
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

Successfully merging this pull request may close these issues.

parse::<Uri> fails to parse uris with triple slash after scheme
3 participants