Classify upstream proxy rejections by status code, not message text - #59
Merged
Merged
Conversation
ConnectFramingTest.aRejectionFromTheUpstreamProxyIsNotReportedAsSuccess failed about one CI run in fifty. The stand-in proxy answered 403 and the tunnel reported upstream-proxy-auth-failed: ProxyErrors searched the exception message for "407", and the message names the proxy's port, which that run was 40797. The test was right and the product was wrong. A real proxy on a port like 3407 had every refusal reported as rejected credentials, sending the operator to check a password that was fine. A SOCKS refusal naming a host such as credentials.example.com was misread the same way. An HTTP proxy's non-2xx answer is now an UpstreamProxyRejection carrying the parsed status, classified on the number: 407 is an auth failure and anything else is a refusal. This covers CONNECT, the WebSocket CONNECT reply and the get-mode upgrade. The remaining text matching, for SOCKS, checks for a refusal first, as its comment already claimed it did.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the flaky
ConnectFramingTest.aRejectionFromTheUpstreamProxyIsNotReportedAsSuccess(failed on 2026-09-25 and on #58's first CI run).Cause
The failing response from CI:
The stand-in proxy answered
403 Forbidden.ProxyErrors.classifyOnetreated any message containingproxyand407as a credentials failure, and the message includes the proxy's port, which that run was 40797.The test was right; the product was wrong. A proxy on a port like 3407 or 14070 has every refusal reported to users as
upstream-proxy-auth-failed. A SOCKS refusal whose destination is named likecredentials.example.comwas misread the same way.Fix
UpstreamProxyRejection extends IOExceptioncarries the parsed HTTP status. Thrown for a non-2xx CONNECT reply (CustomConnectHandler,WebsocketHandler) and for a proxy declining a get-mode WebSocket upgrade.ProxyErrorsclassifies it by the number:407givesupstream-proxy-auth-failed, anything else givesupstream-proxy-refused.WebsocketThroughProxyTestassertsupstream-proxy-refusedagainst a random port too, so it had the same latent flake.Tests
Three new
ProxyErrorsTestcases. Run against the old classifier, two of them fail, reproducing the 40797 case and the hostname case deterministically. With the fix,mvn verifypasses: 1005 tests, coverage gate met.