This repository has been archived by the owner on Nov 6, 2022. It is now read-only.
url: treat empty port as default #483
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When parsing URLs, treat an empty port (eg `http://hostname:/`) as if it were unspecified. RFC 3986 says: > URI producers and normalizers SHOULD omit the port component and its > ":" delimiter if port is empty or if its value would be the same as > that of the scheme's default. (Emphasis on the "SHOULD" is mine.) This indicates that URIs MAY be produced with an empty port and the `:` delimiter. Thus, we stop failing if we end host parsing at the port delimiter.
|
cc @nodejs/http : are we aware of any possible side effects that might happen in Node.js after landing this? |
|
@ethomson thank you for an excellent PR! The change looks great. It has to be considered in the context of Node.js too, however. Let's wait for @nodejs/http to get better understanding of interactions of this PR and Node core. |
|
As a side note, I wonder if libgit2 might be interested in using llhttp instead of |
|
@nodejs/http, ping. |
|
@nodejs/http |
bnoordhuis
reviewed
Mar 19, 2020
| @@ -2326,7 +2326,6 @@ http_parse_host(const char * buf, struct http_parser_url *u, int found_at) { | |||
| case s_http_host_v6: | |||
| case s_http_host_v6_zone_start: | |||
| case s_http_host_v6_zone: | |||
| case s_http_host_port_start: | |||
There was a problem hiding this comment.
This changes the behavior of http_parser_parse_url(is_connect=1) in that it'll now allow CONNECT hostname: as valid input, doesn't it?
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When parsing URLs, treat an empty port (eg
http://hostname:/) as if it were unspecified. RFC 3986 says:(Emphasis mine.) This indicates that URIs MAY be produced with an empty port and the
:delimiter.Thus, we stop failing if we end host parsing at the port delimiter.