bzp2010 opened a new pull request, #13977: URL: https://github.com/apache/apisix/pull/13977
### Description Review of a downstream port of #13939 turned up a set of problems on the `ws`/`wss` proxy path. Each one was checked against this tree first, and the tests added here fail without the fix. **Fixes** - **Scheme dispatch.** `websocket_content_phase` built its endpoint and made its TLS decision from the route's `matched_upstream.scheme`, while `handle_upstream` dispatches on `ctx.upstream_scheme`. An inline `ws`/`wss` upstream picked by `traffic-split` never set the latter, so it fell through to the plain HTTP proxy path. The content phase now uses `ctx.upstream_scheme`, and `traffic-split` sets it for `ws`/`wss` like it already does for `https`. - **Retry after a non-101 answer.** `resty.websocket.client` closes its socket and marks itself fatal on a non-101 response and never resets that, so a retry after e.g. a 503 could not carry a session. Every connection attempt now gets a fresh proxy. - **Host and SNI.** The handshake now sends the Host `proxy_pass` would send (a host set by `proxy-rewrite`, and the retried node's host with `pass_host: node`), and uses it with the port stripped as the SNI and certificate name. Previously a port in the upstream host made certificate verification fail. - **Subprotocol.** The server side of the proxy echoed the client's whole `Sec-WebSocket-Protocol` offer back. The client is now answered with the subprotocol the upstream selected, or none. - **Log hygiene.** The connect failure log no longer includes the request URI, whose query string may carry credentials. - **`request_type`.** It is set to `websocket` after the handshake, since this path has no header filter phase to do it. - **`tls.ca_certs`.** It was silently ignored for `ws`/`wss`, because the cosocket client can only trust the global `lua_ssl_trusted_certificate`. It is now rejected by the Admin API, and the docs say so. - **Client handshake.** It is completed separately from the upstream one, so a bad client handshake is a 400 and is no longer counted as an upstream failure. **Not fixed here (needs `api7-lua-resty-websocket`)** - `ws://[::1]:port` nodes are rejected: the host pattern in `client:connect` does not accept brackets. - `client:connect` does not reset `fatal`, which is what the fresh proxy per attempt works around. - The server side echoes the client's subprotocol offer, which is what the response header rewrite works around. - `tls.ca_certs` needs a per-connection trust store on the cosocket, which the client cannot offer. **Tests** Added cases for `wss` (verification off, verification with a port in the upstream host, a mismatching certificate), subprotocol negotiation, `traffic-split`, a `proxy-rewrite` host, retry after a non-101 answer, and `ca_certs` rejection. The passive health check test now uses a node that answers the handshake with a 503, which the active TCP check cannot flag on its own. `ws_handshake`/`ws_close` and `request_type` are asserted through the error log. ### Checklist - [x] I have explained the need for this PR and the problem it solves - [x] I have explained the changes or the new features added to this PR - [x] I have added tests corresponding to this change - [x] I have updated the documentation to reflect this change - [x] I have verified that this change is backward compatible -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
