Fix SOCKS4 proxy error message and IOCP accept-context socket (#5486)#5684
Conversation
- A rejected SOCKS4 proxy connection reported an "HTTP proxy error" (copy-pasted from the HTTP proxy path); it now names the SOCKS4 proxy. - TcpAcceptor passed the accepted socket as the SO_UPDATE_ACCEPT_CONTEXT option value; per the WinSock AcceptEx contract it must be the listening socket. Pass _fd instead of _acceptFd. Addresses zeroc-ice#5486 (items 1, 12).
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes misleading proxy error reporting and corrects WinSock IOCP accept-context handling per AcceptEx contract.
Changes:
- Update SOCKS4 rejection path to report “SOCKS4 proxy error” instead of “HTTP proxy error”.
- Fix
SO_UPDATE_ACCEPT_CONTEXTusage to pass the listening socket_fdrather than the accepted socket_acceptFd.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cpp/src/Ice/TcpAcceptor.cpp | Corrects SO_UPDATE_ACCEPT_CONTEXT option value to use the listening socket per WinSock contract. |
| cpp/src/Ice/NetworkProxy.cpp | Fixes SOCKS4 proxy rejection message to reference the correct proxy type. |
| } | ||
| if (setsockopt(_acceptFd, SOL_SOCKET, SO_UPDATE_ACCEPT_CONTEXT, (char*)&_acceptFd, sizeof(_acceptFd)) == | ||
| SOCKET_ERROR) | ||
| // SO_UPDATE_ACCEPT_CONTEXT takes the listening socket (_fd) as its option value, not the accepted |
There was a problem hiding this comment.
A comment that explains the new correct path/API usage is fine (within reason). But no need to mention what "not do do".
There was a problem hiding this comment.
🤖 Claude: Done in 0d02e9c — dropped the comment entirely. Now that the call is correct it is a standard setsockopt we would not normally annotate.
Addresses #5486 (items 1, 12). One of a set of PRs splitting the grouped transport audit.
TcpAcceptorpassed the accepted socket as theSO_UPDATE_ACCEPT_CONTEXToption value; per the WinSockAcceptExcontract it must be the listening socket. It now passes_fd(the listening socket) instead of_acceptFd.Item 1 compiles on macOS; item 12 is Windows-only (verified against the Microsoft
AcceptExdocs). Codex-audited.