search

nodejs / http-parser

http request/response parser for c
MIT License
6.33k stars 1.53k forks source link

url: treat empty port as default #483

Open ethomson opened 5 years ago

ethomson commented 5 years ago

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 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.

indutny commented 5 years ago

cc @nodejs/http : are we aware of any possible side effects that might happen in Node.js after landing this?

indutny commented 5 years ago

@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.

indutny commented 5 years ago

As a side note, I wonder if libgit2 might be interested in using llhttp instead of http_parser? The former is faster and verifiable, and is already used in the latest Node.js.

ethomson commented 4 years ago

Interesting...! Thanks for the pointer @indutny. I’ll take a look, particularly since I’m interested in pausing in my on_headers_complete callback. Would you happen to know offhand if llhttp fixes #97 or if that’s still an issue?

mgorny commented 4 years ago

@nodejs/http, ping.

sam-github commented 4 years ago

@nodejs/http