Fix HTTP tunnel by flobernd · Pull Request #2448 · StackExchange/StackExchange.Redis (original) (raw)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.