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

Fails to parse port 80 only #28

Closed
thekashifmalik opened this issue Sep 12, 2014 · 5 comments
Closed

Fails to parse port 80 only #28

thekashifmalik opened this issue Sep 12, 2014 · 5 comments

Comments

@thekashifmalik
Copy link

When port 80 is in the passed in &str the resulting Url object shows a port value of None.

@SimonSapin
Copy link
Member

The idea is to do like browsers’s URLUtils.port which is the empty string when the port number is the same as the scheme’s default (80 for HTTP).

There’s a .port_or_default() convenience method, which may be what you’re looking for.

I’ve considered changing "port" to be the current "port or default", and have a "port is default" boolean or something. What do you think?

@thekashifmalik
Copy link
Author

I would defer to the 'Principle of least astonishment' and say that a URL library in isolation should parse all ports equally.

Behaviour like URLUtils.port makes sense to me in a certain environment (like an HTTP browser that has certain common cases), but not in a general URL library. I would certainly encourage the change.

If anything, I would say maybe there should be a method .is_default() -> bool / is_default_for_scheme() -> bool instead of a method that returns an Option<u16> to check for default port values.

@SimonSapin
Copy link
Member

I’m very skeptical of this so-called "principle of least astonishment". What’s astonishing to someone might be obvious and make perfect sense to someone else. Everyone has different experience and makes different assumptions.

Regardless, a more important concern for this library is interoperability. Any deviation like this is a potential number of sites that work fine in other browsers but are broken in Servo.

@banool
Copy link

banool commented Jun 30, 2022

6 years later and just reporting that I'm finding myself quite astonished here.

Program:

use url::Url;

fn main() {
    let url1 = Url::parse("http://localhost:5000").unwrap();
    println!("Port: {:?}", url1.port());
    let url2 = Url::parse("http://localhost:80").unwrap();
    println!("Port: {:?}", url2.port());
}

Output:

Port: Some(5000)
Port: None

I think is very unintuitive, I clearly set the port and expect it to be there. Erasing it for 80 is magic.

@banool
Copy link

banool commented Jul 6, 2022

For people coming along later and looking for a better option, use url.port_or_known_default(), it behaves how you'd actually expect it to behave.

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

3 participants