-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Require scheme in URIs #10884
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
Require scheme in URIs #10884
Conversation
|
What about empty string? ( We may allow empty string to allow users to build |
that's a good point. I'll update my PR. |
| assert String.Chars.to_string(uri) == "http://google.com" | ||
|
|
||
| uri_no_host = URI.parse("/foo/bar") | ||
| assert String.Chars.to_string(uri_no_host) == "/foo/bar" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this test, because it doesn't make sense after the change any more. The path by itself isn't a valid URI.
|
Per URI.parse/1 docs
So making the function stricter would be a breaking change. For this reason, we are discussing deprecating it in favour of a new function that would perform the proper validations, see #10865 |
|
The URI module can parse both relative and absolute URLs, see section 4.2. I will clarify this in the docs to avoid further confusion. :) Thank you! |
I ran into an issue involving unexpected behavior from the URI module. The URI module does not appear to follow the specification with respect to the scheme of a URL. This can cause inconsistencies when operating with other tools and libraries that accept URIs.
According to the spec:
However, the URI module doesn't check if the scheme is provided, nor does it set a default scheme. If you don't specify a manual scheme, it comes out like
//abc.com. I think it should throw anArgumentErrorto match the existing behavior for checking the correctness of the path component before turning the URI into a string.