fix: isolate rejected UDP targets from SOCKS associations - #25
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Exact-head CI, CodeQL, and Linux netem verification remain pending.
Review effort: Lite
Findings: None
What changed in this PR
This PR isolates rejected UDP targets so shared SOCKS5 associations remain usable.
Changes:
- Classifies recoverable target failures while preserving causes.
- Keeps SOCKS associations alive for rejected datagrams.
- Adds regression tests and documents target and health semantics.
| File | Description |
|---|---|
internal/tunnel/web_udp.go |
Classifies recoverable target errors. |
internal/tunnel/web_udp_test.go |
Extends capacity behavior tests. |
internal/tunnel/web_udp_target_error_test.go |
Tests error classification and cause retention. |
internal/tunnel/web_udp_socks_isolation_test.go |
Tests SOCKS/H3 association isolation. |
internal/tunnel/web_client_udp_health_test.go |
Verifies H3 health semantics. |
internal/transport/transport.go |
Defines the target-unavailable error contract. |
internal/proxy/socks5.go |
Drops recoverable target datagrams without closing associations. |
internal/proxy/socks5_udp_target_error_test.go |
Tests recoverable and terminal SOCKS errors. |
docs/WEB_COVER.md |
Documents per-target behavior. |
docs/PROTOCOL.md |
Documents protocol and health semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Summary
Keep a multi-target SOCKS5 UDP association usable when opening one H3 CONNECT-UDP target is rejected.
Previously, direct H3 PacketConn could keep a healthy target after a signed target failure, but the SOCKS frontend treated that Send error as terminal and closed the whole association. This interrupted unrelated healthy traffic while the physical H3 connection remained available.
transport.ErrPacketTargetUnavailable.errors.Is/errors.As; keep signed-response path-health evidence unchanged.Validation
make checkandmake race: passed locally.1740df2production overlay with the same new regression failed both 502 and 503 cases with control TCP EOF. Separate real old-version diagnostics verified the H3 connection itself stayed alive.Evidence is local bounded functionality/error-isolation testing, not a broad weak-network, passive-identification or Internet-performance claim. No historical remote host, full corpus, or release used.
Exact head
729befc20c83477e6012048bf9be8705a52b1fdbpassed CI (35807912577, all 10 jobs), CodeQL (35807912416), and Linux netem (35807912489). Container integration, OCI index build and the namespace/netem suite all completed successfully. No unresolved review threads at merge check. Post-merge mainf31400434562d4426e38d76009d287a40fd10106also passed CI (35808176990, all 10 jobs including Docker integration/OCI build), CodeQL (35808176937), and Linux netem (35808177027, actual namespace suite plus diagnostics upload). Main treed7abfb0d58914b83284c561ea602d40847cb1ba9equals the tested PR tree; local main is synchronized and clean. Copilot review reported no findings; its initial pending-CI note is superseded by these completed exact-head/main gates. No release created.