close failed v2ray upgrade connections#4241
Conversation
22fe3b6 to
b2d51f6
Compare
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1c87b6c to
535434f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 535434f85f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if response.StatusCode != 101 || | ||
| !strings.EqualFold(response.Header.Get("Connection"), "upgrade") || | ||
| !strings.EqualFold(response.Header.Get("Upgrade"), "websocket") { | ||
| response.Body.Close() |
There was a problem hiding this comment.
Don't drain failed HTTP-upgrade responses
When a server rejects the upgrade with a non-101 response that includes a Content-Length or chunked body and then sends it slowly or not at all, response.Body.Close() can block while consuming the unread body. Because the deferred conn.Close() only runs after this call returns, DialContext can hang indefinitely instead of failing and closing the socket; for this error path it is safer to close the underlying connection first or skip closing the response body since the connection is being discarded.
Useful? React with 👍 / 👎.
Problem
Summary
Root cause
Both transport clients open a lower-level TCP connection before validating the upgrade handshake. Some failure paths returned without closing that connection, so repeated failed URL tests could retain socket/AFD handles on Windows even though they no longer appeared as active established TCP connections.
Validation