Skip to content

fix(websocket): address review findings on the ws/wss proxy path #13977

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bzp2010 merged 2 commits into from
Sep 22, 2026
Merged

fix(websocket): address review findings on the ws/wss proxy path #13977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2c62b8
fix(websocket): address review findings on the ws/wss proxy path
bzp2010 Sep 21, 2026
4415065
test(websocket): check request_type from a log phase function, not ex…
bzp2010 Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 74 additions & 23 deletions apisix/init.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,8 +310,7 @@ end


-- host per upstream.pass_host: pass = client's Host, rewrite = configured
-- upstream_host, node = picked node's host[:port]. Also used directly by the
-- websocket phase, which has no nginx variable to fall back on for "pass".
-- upstream_host, node = picked node's host[:port].
local function compute_upstream_host(api_ctx, picked_server)
local pass_host = api_ctx.pass_host or "pass"
if pass_host == "rewrite" then
Expand Down Expand Up @@ -347,6 +346,16 @@ local function set_upstream_headers(api_ctx, picked_server)
end


-- "example.com:443" -> "example.com", "[::1]:443" -> "::1": the name a TLS
-- handshake sends as SNI and verifies the certificate against, which must not
-- carry the port an upstream Host header may have. Same as nginx does for
-- proxy_ssl_name.
local function host_without_port(host)
local m = ngx_re_match(host, [=[^(?:\[([^\]]+)\]|([^:]+))]=], "jo")
return m and (m[1] or m[2]) or host
end


-- hop-by-hop headers, plus handshake headers connect() already sets itself
-- (host/protocols/origin opts, or generated Sec-WebSocket-Key/-Version).
local ws_skip_forward_headers = {
Expand Down Expand Up @@ -1068,6 +1077,7 @@ function _M.websocket_content_phase()
ngx.ctx = fetch_ctx()
local api_ctx = ngx.ctx.api_ctx
local up_conf = api_ctx.upstream_conf
local up_scheme = api_ctx.upstream_scheme
-- a Route's own `timeout` overrides upstream.timeout, same as
-- set_balancer_opts() does for the plain proxy_pass path
local route = api_ctx.matched_route
Expand All @@ -1082,7 +1092,7 @@ function _M.websocket_content_phase()

-- resolve upstream.tls once, same as https/grpcs in apisix/upstream.lua
local ssl_verify, client_cert, client_priv_key
if api_ctx.matched_upstream.scheme == "wss" and up_conf.tls then
if up_scheme == "wss" and up_conf.tls then
ssl_verify = up_conf.tls.verify

if up_conf.tls.client_cert or up_conf.tls.client_cert_id then
Expand Down Expand Up @@ -1126,7 +1136,7 @@ function _M.websocket_content_phase()
upstream_new_opts = {max_recv_len = upstream_max_len, max_send_len = client_max_len}
end

local ok, proxy, err = pcall(ws_proxy.new, {
local proxy_opts = {
aggregate_fragments = true,
recv_timeout = recv_timeout_ms,
client_new_opts = client_new_opts,
Expand Down Expand Up @@ -1162,19 +1172,23 @@ function _M.websocket_content_phase()
local new_frame = role_handler.get_frame()
return new_frame.payload, new_frame.code
end
})
if not ok then
ngx.log(ngx.ERR, "failed to create proxy: ", proxy)
return core.response.exit(500)
end
if not proxy then
ngx.log(ngx.ERR, "failed to create proxy: ", err)
return core.response.exit(500)
}

-- A client that got a non-101 answer is marked fatal and its socket
-- closed (see resty.websocket.client), so it cannot serve a retry against
-- another node: every connection attempt gets a fresh proxy.
local function new_proxy()
local ok, proxy, err = pcall(ws_proxy.new, proxy_opts)
if not ok then
return nil, proxy
end

return proxy, err
end

-- proxy:connect() only sends the 101 response to the downstream client
-- after it has successfully connected upstream, so it's safe to retry
-- against another node here without having committed to the client yet.
-- the 101 response goes to the downstream client only in connect_client(),
-- after an upstream connection has succeeded, so it's safe to retry against
-- another node here without having committed to the client yet.
local retries = up_conf.retries
if not retries or retries < 0 then
retries = #up_conf.nodes - 1
Expand All @@ -1196,24 +1210,41 @@ function _M.websocket_content_phase()
request_uri = api_ctx.var.uri .. (api_ctx.var.is_args or "") .. (api_ctx.var.args or "")
end

local proxy, ok, connect_err
local server = api_ctx.picked_server
local ok, connect_err
for attempt = 0, retries do
if attempt > 0 and retry_deadline and retry_deadline < ngx_now() then
ngx.log(ngx.ERR, "websocket proxy retry timeout, retry count: ", attempt,
", deadline: ", retry_deadline, " now: ", ngx_now())
return core.response.exit(502)
end

local err
proxy, err = new_proxy()
if not proxy then
ngx.log(ngx.ERR, "failed to create proxy: ", err)
return core.response.exit(500)
end

if connect_timeout_ms then
proxy.client:set_timeout(connect_timeout_ms)
end

local endpoint = str_format("%s://%s:%d%s", api_ctx.matched_upstream.scheme,
server.host, server.port, request_uri)
ok, connect_err = proxy:connect(endpoint, {
host = compute_upstream_host(api_ctx, server),
server_name = server.domain,
-- what proxy_pass would send as Host and use as SNI: honors a host
-- set by plugins such as proxy-rewrite, and follows a retried node
-- for pass_host = node
set_upstream_host(api_ctx, server)
local host = api_ctx.var.upstream_host
if not host or host == "" then
host = api_ctx.var.http_host
end

-- the request URI is left out of anything logged: its query string
-- may carry credentials
local node_addr = str_format("%s://%s:%d", up_scheme, server.host, server.port)
ok, connect_err = proxy:connect_upstream(node_addr .. request_uri, {
host = host,
server_name = host_without_port(host),
headers = ws_headers,
protocols = ws_protocols,
origin = ws_origin,
Expand All @@ -1225,7 +1256,7 @@ function _M.websocket_content_phase()
break
end

ngx.log(ngx.ERR, "failed to connect to websocket upstream ", endpoint,
ngx.log(ngx.ERR, "failed to connect to websocket upstream ", node_addr,
": ", connect_err)

-- no balancer_by_lua* here, so report the outcome ourselves; a parsed
Expand Down Expand Up @@ -1261,7 +1292,27 @@ function _M.websocket_content_phase()
return core.response.exit(502)
end

local done, err = proxy:execute()
-- The server side of the proxy answers the client's Sec-WebSocket-Protocol
-- offer by echoing it back as it stands, which would announce a subprotocol
-- the upstream never selected. Leave it exactly what the upstream picked,
-- or nothing, before completing the client handshake.
local resp_headers = proxy.client:get_resp_headers()
local selected = resp_headers and resp_headers.sec_websocket_protocol
if type(selected) == "table" then
selected = selected[1]
end
core.request.set_header(api_ctx, "Sec-WebSocket-Protocol", selected)

local done, err = proxy:connect_client()
if not done then
ngx.log(ngx.ERR, "failed to complete the client websocket handshake: ", err)
return core.response.exit(400)
end

-- there is no header filter phase on this path to do this on the 101
api_ctx.var.request_type = "websocket"

done, err = proxy:execute()
if not done then
ngx.log(ngx.ERR, "failed proxying: ", err)
return core.response.exit(502)
Expand Down
4 changes: 3 additions & 1 deletion apisix/plugins/traffic-split.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,9 @@ local function set_upstream(upstream_info, ctx)
end
core.log.info("upstream_key: ", upstream_key)
upstream.set(ctx, upstream_key, ctx.conf_version, up_conf)
if upstream_info.scheme == "https" then
-- the schemes handle_upstream() dispatches on ctx.upstream_scheme for
local scheme = upstream_info.scheme
if scheme == "https" or scheme == "ws" or scheme == "wss" then
upstream.set_scheme(ctx, up_conf)
end
return
Expand Down
8 changes: 8 additions & 0 deletions apisix/upstream.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,14 @@ local function check_upstream_conf(in_dp, conf)
then
return false, "`upstream_host` can't be empty when `pass_host` is `rewrite`"
end

-- the ws/wss client connects through a plain cosocket, which can only
-- trust the global lua_ssl_trusted_certificate, not a per-upstream store
if (conf.scheme == "ws" or conf.scheme == "wss")
and conf.tls and conf.tls.ca_certs
then
return false, "`tls.ca_certs` is not supported by the `ws`/`wss` scheme"
end
end

if conf.tls and conf.tls.client_cert then
Expand Down
2 changes: 1 addition & 1 deletion docs/en/latest/admin-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1020,7 +1020,7 @@ In addition to the equalization algorithm selections, Upstream also supports pas
| tls.client_key | False, can't be used with `tls.client_cert_id` | HTTPS certificate private key | Sets the client private key while connecting to a TLS Upstream. | |
| tls.client_cert_id | False, can't be used with `tls.client_cert` and `tls.client_key` | SSL | Set the referenced [SSL](#ssl) id. | |
| tls.verify | False | Boolean | Enables or disables verification of the Upstream certificate. Falls back to the nginx configuration when unset. Also used by the `kafka` scheme. | |
| tls.ca_certs | False | Array of HTTPS certificates | CA certificates used to verify the Upstream certificate, replacing the ones loaded from `ssl_trusted_certificate`. | |
| tls.ca_certs | False | Array of HTTPS certificates | CA certificates used to verify the Upstream certificate, replacing the ones loaded from `ssl_trusted_certificate`. Not supported when `scheme` is `ws` or `wss`, which only trust `ssl_trusted_certificate`. | |
| keepalive_pool.size | False | Auxiliary | Sets `keepalive` directive dynamically. | |
| keepalive_pool.idle_timeout | False | Auxiliary | Sets `keepalive_timeout` directive dynamically. | |
| keepalive_pool.requests | False | Auxiliary | Sets `keepalive_requests` directive dynamically. | |
Expand Down
2 changes: 1 addition & 1 deletion docs/zh/latest/admin-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ APISIX 的 Upstream 除了基本的负载均衡算法选择外，还支持对上
| tls.client_key | 否，不能和 `tls.client_cert_id` 一起使用 | https 证书私钥 | 设置跟上游通信时的客户端私钥，详细信息请参考下文。 | |
| tls.client_cert_id | 否，不能和 `tls.client_cert`、`tls.client_key` 一起使用 | SSL | 设置引用的 SSL id，详见 [SSL](#ssl)。 | |
| tls.verify | 否 | Boolean | 开启或关闭上游证书校验，不设置时沿用 nginx 的配置，详细信息请参考下文。Kafka 上游同样使用该字段。 | |
| tls.ca_certs | 否 | https 证书数组 | 用于校验上游证书的 CA 证书，设置后将取代 `ssl_trusted_certificate` 中加载的证书，详细信息请参考下文。 | |
| tls.ca_certs | 否 | https 证书数组 | 用于校验上游证书的 CA 证书，设置后将取代 `ssl_trusted_certificate` 中加载的证书，详细信息请参考下文。`scheme` 为 `ws` 或 `wss` 时不支持该字段，只会信任 `ssl_trusted_certificate`。 | |
|keepalive_pool.size | 否 | 辅助 | 动态设置 `keepalive` 指令，详细信息请参考下文。 |
|keepalive_pool.idle_timeout | 否 | 辅助 | 动态设置 `keepalive_timeout` 指令，详细信息请参考下文。 |
|keepalive_pool.requests | 否 | 辅助 | 动态设置 `keepalive_requests` 指令，详细信息请参考下文。 |
Expand Down
Loading
Loading