Set default proxy ports to spec

[DanGould]

"If the port number is not specified, 80 is always assumed for HTTP."
See: https://www.w3.org/Protocols/HTTP/AsImplemented.html

It was strange behavior to me to default to 8080

[algesten]

Hit @DanGould, welcome to ureq!

Is there a spec regarding this for proxies? I don't know why this is 8080 for the proxy. Could just be a mistake.

[DanGould]

Based on some examples [using 8080 in this codebase], I have a feeling it was just a mistake carried over in testing. The RFC that defines HTTP and the CONNECT method says 80 is assumed to be the default. HTTP proxy is just plain http with this method as I understand.

However, curl does default the http proxy to 1080 for "purely historical" reasons, but if the proxy is https (not yet supported by ureq) it defaults to 443 in my experience.

[DanGould]

Looks like SOCKS conventionally listens on port 1080 by spec actually, so I've adjusted the default behavior to match that too

[DanGould]

force pushed to fix clippy lints

[DanGould]

In further testing, I discovered that Url::parse discards the port if it is assumed to be the default port for the scheme. So, for example passing Url::parse(&"http://example.com:80")? to the proxy will strip the port and replace it with 8080 because port is None and before this change port: port.unwrap_or(8080), is called. The only way around this is by passing a Uri without a scheme such as Url::parse(&"example.com:80")?.

The fact that default port numbers are elided by parsing is documented in the Url::port(&self) method docs. "default port numbers are never reflected by the serialization"

I certainly interpret the behavior before the change this PR makes as a bug.

[algesten]

So what are we saying the behavior for http://example.com:80 should be with regards to the proxy port?

[DanGould]

So what are we saying the behavior for http://example.com:80 should be with regards to the proxy port?

Current behavior

• an Agent with Proxy::new("http://example.com:80") sends requests to example.com:8080 to be proxied
• an Agent with Proxy::new("example.com:80") sends requests to example.com:80 to be proxied
• an Agent with Proxy::new("http://example.com") sends requests to example.com:8080 to be proxied
• an Agent with Proxy::new("http://example.com:8080") sends requests to example.com:8080 to be proxied

Desired behavior

• an Agent with Proxy::new("http://example.com:80") sends requests to example.com:80 to be proxied
• an Agent with Proxy::new("example.com:80") sends requests to example.com:80 to be proxied
• an Agent with Proxy::new("http://example.com") sends requests to example.com:80 to be proxied
• an Agent with Proxy::new("http://example.com:8080") sends requests to example.com:8080 to be proxied

[algesten]

Alright! This is ready to merge then? Is this a potential breaking change?

[DanGould]

Yes. I think this is a breaking change for anyone depending on the 8080 default. That's an odd behavior to depend on but I wouldn't say it's impossible.

[algesten]

I think it can be argued that depending on 8080 being the default is not correct behavior.

[DanGould]

Thanks for sticking with me and merging this. I think this makes #719 a bit easier so I'll have a revisit to that now

[algesten]

Thank you!