Fix HTTP tunnel #2448
Fix HTTP tunnel #2448
Conversation
According to https://datatracker.ietf.org/doc/html/draft-luotonen-web-proxy-tunneling-01#section-3.2, the expected response to a `CONNECT` should be `HTTP/1.1 200 Connection established` instead of `HTTP/1.1 200 OK`. Allow both responses for now (as still a lot of proxy implementations fail to follow the standard).
A HTTP(S), SOCKS, etc. proxy is traditionally specified by an URI of form `scheme://[user:pass@]host[:port]`. Accept both formats for backwards compatibility.
|
LGTM |
| { | ||
| value = value.Substring(5); | ||
| if (!Format.TryParseEndPoint(value, out var ep)) | ||
| // For backwards compatibility with `http:address_with_port`. |
There was a problem hiding this comment.
IMO we should add tests for this change, i.e. that we get the expected values back out - even if that means poking at internal state after the parse
There was a problem hiding this comment.
I added the new format to the roundtrip test. This should be sufficient to confirm that the resulting Tunnel is the same for both input formats (HttpProxyTunnel.ToString() internally uses this.EndPoint.ToString() which means address and port must be the same to end up with the same string).
IMO it would make sense to consider deprecating the old format for the following reasons:
- Roundtrip is inconsistent now, if you use the URI format
- URI format is standard
- URI format supports authentication (
username+password) if needed at some point of time
Edit: If you prefer, we can apply the parsing change in a different PR (or remove it at all - it's up to you).
Edit 2: Not sure why the unrelated test fails after my latest commit. Seems a random failure, maybe you can re-run the CI.
|
Seems like the CI is broken 👀 third run failed as well. |
|
@flobernd updated release notes and landed on - thank you!! |
According to https://datatracker.ietf.org/doc/html/draft-luotonen-web-proxy-tunneling-01#section-3.2, the expected response to a
CONNECTshould beHTTP/1.1 200 Connection establishedinstead ofHTTP/1.1 200 OK. Allow both responses for now (as still a lot of proxy implementations fail to follow the standard).A HTTP(S), SOCKS, etc. proxy is traditionally specified by an URI of form
scheme://[user:pass@]host[:port]. Accept both formats for backwards compatibility.