Skip to content

Fix HTTP client proxy support #742

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

Merged
merged 5 commits into from
Feb 25, 2025
Merged

Conversation

DerGuteMoritz
Copy link
Collaborator

Fixes #731

Since the introduction of HTTP/2 support in the client, proxy handlers (requested via proxy-options)
would be installed only after the HTTP version was negotiated. In case of a HTTPS connection, this
would be attempted via ALPN which means the TLS handshake would have already taken place with the
actual destination host, bypassing the configured proxy. This resulted in an error (see #731).

With this patch, the proxy handlers are installed earlier again so that a configured proxy is also
used during the TLS handshake phase.
Turns out this worked perfectly fine already.
@DerGuteMoritz
Copy link
Collaborator Author

FYI: There's a huge diff there due to wrapping + reindent, so I recommend to hide whitespace when looking at the diff once more!

Copy link
Collaborator

@KingMob KingMob left a comment

Choose a reason for hiding this comment

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

Looks ok to me, but I'm rusty, and never did anything with proxies even when I was working on Aleph, so take with a grain of salt :D

@DerGuteMoritz DerGuteMoritz force-pushed the fix-http-client-proxy-support branch from 49cb9d2 to d498758 Compare February 10, 2025 19:58
* Ensure that proxy connection errors have precedence over subsequent errors so that
  they don't get wrapped in e.g. SSL handshake exceptions.
* Fix proxy connection timeout detection (the check wasn't correct anymore).
* Propagate ProxyConnectionTimeoutException directly instead of wrapping it in another
  ConnectionTimeoutException
* Add tests for proxy connection errors
@DerGuteMoritz DerGuteMoritz merged commit 472b2ea into master Feb 25, 2025
1 check passed
@DerGuteMoritz DerGuteMoritz deleted the fix-http-client-proxy-support branch February 25, 2025 09:01
@bitti
Copy link
Contributor

bitti commented Mar 2, 2025

Thanks for the great effort! I'm sorry that I didn't have time to contribute some tests. Maybe next time.

Also congragulations to the 10²th tag and the 19²th closed issue! These are nice round numbers.

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

Successfully merging this pull request may close these issues.

Proxy functionality broken in Aleph 0.7.0 and later
4 participants