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

Option to keep default port #132

Open
lutovich opened this issue Apr 27, 2018 · 9 comments
Open

Option to keep default port #132

lutovich opened this issue Apr 27, 2018 · 9 comments

Comments

@lutovich
Copy link

Currently parsed URL does not contain port when its value is the default for the specified protocol:

parse("http://localhost:80").port === ''
parse("https://localhost:443").port === ''

It would be great to have an option/parameter to keep the port number even if it is the default for the protocol.

This would be useful when default port is defined by the environment/application requirements and is different from the standard. For example, an application might want to interpret http://localhost as http://localhost:8080 and not http://localhost:80. Only http://localhost:80 will be interpreted as http://localhost:80. Right now after URL has been parsed there is no way to know if port was not specified or default port was specified.

I'd be glad to provide a PR for this. Not sure if it fine to just add a 4th parameter to the URL function.

P.S. thanks for this library, it is really convenient and simple to use!

@3rd-Eden
Copy link
Member

Yes, no, maybe.

The current behavior is modeling the way node and browser parse URL's. We currently already support 3 different URL arguments, adding a 4th argument would really over bloat it given how small this specific use case would be. If were to decide to change to URL(str, options) pattern and break the API, I could see this something simple to add.

I guess we could change the API to str, options, not sure how useful the current API is.

Thoughts?

@lpinca
Copy link
Member

lpinca commented Apr 30, 2018

I'm not very happy with adding an additional parameter either as I think this is an edge case. In some cases we can't be consistent with both legacy and WHATWG URL parsers because they have different behaviours.

@lutovich
Copy link
Author

I definitely agree that adding 4th parameter is not pretty. URL(str, options) would be great but it's an incompatible API change and will require a major release. Do not know if it's acceptable for the library to make such a change.

@lutovich
Copy link
Author

Out of curiosity, why is hiding of the default port needed? Is it to be compatible with other browser/node URL parsers? Imho it is a bit of a surprising behavior, especially for parsing.

@lpinca
Copy link
Member

lpinca commented Apr 30, 2018

Yes, see https://url.spec.whatwg.org/#port-state.

@Christilut
Copy link

Came across this issue trying to find out why url-parse wouldn't parse my port number. Odd behavior if you ask me. Didn't see this in the docs either.

@3rd-Eden
Copy link
Member

3rd-Eden commented Oct 1, 2018

I'm more than happy to merge any documentation updates ;-)

@julien-f
Copy link

julien-f commented Mar 3, 2021

Hi,

Came across this issue, due to cURL using 1080 as default port for HTTP proxy.

An alternative solution would be to provide a new getter always returning the port, even if it's the one by default.

@iaktern
Copy link

iaktern commented Jul 23, 2021

Yes, see https://url.spec.whatwg.org/#port-state.

Hi, I assume you refer to this sentence:

  1. Set url’s port to null, if port is url’s scheme’s default port, and to port otherwise.

It would actually help, if port is really set to JS null :) Then we can differentiate if url-parse removed the port or that it was intentionally not set in the input:

new Url('https://github.com/foo/bar') => { port: '', ... }
new Url('https://github.com:80/foo/bar') => { port: null, ... }

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

6 participants