Skip to content

Commit d4e6802

Browse files
committed
ws-over-h2: permessage-deflate fixes: short return, ext tx-drain wedge, client extension offer -- with selftest
Three fixes for permessage-deflate on ws-over-h2 (RFC 8441) encapsulated streams, plus a ctest-wired selftest: 1) The encapsulated tx path returned the h2 role's write result, which reflects the POST-COMPRESSION frame size -- with pmd active that is routinely smaller than what the caller passed in, so any well-compressed message made callers checking (wr < len) conclude a short write and kill a healthy connection. The lws_write() contract is to report how much of the CALLER's payload was accepted, and the h1 path already returns orig_len for exactly this reason; do the same, still propagating write errors as < 0. 2) When pmd's compressed output exceeds its chunk buffer, the ws role sets ws->tx_draining_ext and expects the next POLLOUT to send the remaining fragments -- the h1 path services this in rops_handle_POLLOUT_ws() priority 5, before letting the user write anything new. An h2-encapsulated child never reaches that path: its only POLLOUT servicing is the child loop in rops_perform_user_POLLOUT_h2(), which went straight to the user callback. Meanwhile lws_send_pipe_choked() reports choked while tx_draining_ext is set, so a user write loop gated on it never writes again either: the drain never advances and the stream wedges permanently. Service the drain from the child loop, ahead of the user callback. It slots in after the generic 'deal with partial at nwsi' branch that 774c64a ("ws-over-h2: deal with choked nwsi") routes carries-ws children through, preserving its ordering: a child on a backed-up network socket still defers there first, the same priority h1 connections get, and the extension drain only runs once the nwsi partial has cleared. 3) lws_h2_client_handshake() only emitted sec-websocket-version and sec-websocket-protocol, so an lws client could never negotiate permessage-deflate (or any extension) over h2 even against a willing server. RFC 8441 Sect 5 carries the ws handshake headers unchanged over extended CONNECT, so build the same offer list as lws_generate_client_ws_handshake() and emit it after the pseudo-headers; the response side already instantiates accepted extensions in the shared lws_client_ws_upgrade() path. api-test-ws-h2-pmd (needs LWS_WITHOUT_EXTENSIONS=OFF): single-process h2 ws server + client, both offering pmd; the server bulk-sends a 64KB deterministic pattern in 4KB messages gated only on lws_send_pipe_choked(), alternating trivially-compressible blocks (deflated frame far smaller than the payload) with incompressible hash noise (deflated output overflows pmd's default 1KB chunk buffer, forcing tx_draining_ext). It fails if the connection is not genuinely ws-over-h2, if the extended CONNECT did not offer pmd, if any lws_write() reports fewer bytes accepted than requested, if the pattern corrupts, or if the transfer stalls. Reverting any one of the three fixes makes it fail in the corresponding distinct way.
1 parent 9428500 commit d4e6802

5 files changed

Lines changed: 536 additions & 1 deletion

File tree

lib/roles/h2/http2.c

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2870,6 +2870,40 @@ lws_h2_client_handshake(struct lws *wsi)
28702870
goto fail_length;
28712871
}
28722872

2873+
#if !defined(LWS_WITHOUT_EXTENSIONS)
2874+
{
2875+
/*
2876+
* Offer the vhost's extensions the same way the h1
2877+
* upgrade does (RFC 8441 Sect 5 carries the ws
2878+
* headers unchanged): without this the client can
2879+
* never negotiate eg, permessage-deflate over h2.
2880+
*/
2881+
const struct lws_extension *ext =
2882+
wsi->a.vhost->ws.extensions;
2883+
char eb[256];
2884+
int el = 0;
2885+
2886+
while (ext && ext->callback) {
2887+
if (wsi->a.vhost->protocols[0].callback(wsi,
2888+
LWS_CALLBACK_CLIENT_CONFIRM_EXTENSION_SUPPORTED,
2889+
wsi->user_space, (char *)ext->name, 0)) {
2890+
ext++;
2891+
continue;
2892+
}
2893+
el += lws_snprintf(eb + (size_t)el,
2894+
sizeof(eb) - (size_t)el,
2895+
"%s%s", el ? "," : "",
2896+
ext->client_offer);
2897+
ext++;
2898+
}
2899+
if (el &&
2900+
lws_add_http_header_by_token(wsi,
2901+
WSI_TOKEN_EXTENSIONS,
2902+
(unsigned char *)eb, el, &p, end))
2903+
goto fail_length;
2904+
}
2905+
#endif
2906+
28732907
wsi->h23_stream_carries_ws = 1;
28742908
}
28752909
#endif

lib/roles/h2/ops-h2.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,33 @@ rops_perform_user_POLLOUT_h2(struct lws *wsi)
11391139
goto next_child;
11401140
}
11411141

1142+
#if defined(LWS_ROLE_WS) && !defined(LWS_WITHOUT_EXTENSIONS)
1143+
/*
1144+
* ws-over-h2: tx path extension with more to send (eg,
1145+
* permessage-deflate whose compressed output exceeded its
1146+
* chunk buffer). The h1 path services this from
1147+
* rops_handle_POLLOUT_ws() priority 5; this loop is the ONLY
1148+
* POLLOUT servicing an encapsulated child ever gets, so it
1149+
* must do the same -- while tx_draining_ext is set,
1150+
* lws_send_pipe_choked() reports choked, so the user
1151+
* callback will never write again and the connection wedges
1152+
* for good.
1153+
*/
1154+
if (lwsi_role_ws(w) && lwsi_state(w) == LRS_ESTABLISHED &&
1155+
w->ws && w->ws->tx_draining_ext) {
1156+
if (lws_write(w, NULL, 0, LWS_WRITE_CONTINUATION) < 0) {
1157+
lwsl_info("%s signalling to close\n", __func__);
1158+
lws_close_free_wsi(w, LWS_CLOSE_STATUS_NOSTATUS,
1159+
"h2 ws ext drain");
1160+
wa = &wsi->mux.child_list;
1161+
goto next_child;
1162+
}
1163+
lws_callback_on_writable(w);
1164+
wa = &wsi->mux.child_list;
1165+
goto next_child;
1166+
}
1167+
#endif
1168+
11421169
/* priority 2: pre compression-transform buffered output */
11431170

11441171
#if defined(LWS_WITH_HTTP_STREAM_COMPRESSION)

lib/roles/ws/ops-ws.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2001,10 +2001,24 @@ rops_write_role_protocol_ws(struct lws *wsi, unsigned char *buf, size_t len,
20012001
}
20022002
#endif
20032003

2004-
return lws_rops_func_fidx(encap->role_ops,
2004+
n = lws_rops_func_fidx(encap->role_ops,
20052005
LWS_ROPS_write_role_protocol).
20062006
write_role_protocol(wsi, buf - pre,
20072007
len + (unsigned int)pre, wp);
2008+
if (n < 0)
2009+
return n;
2010+
2011+
/*
2012+
* The lws_write() contract is to report how much of the
2013+
* CALLER's payload was accepted. len here is the
2014+
* post-extension (eg, permessage-deflate compressed) frame
2015+
* size, which is routinely SMALLER than what the caller
2016+
* passed in -- returning it (as this path used to) makes
2017+
* well-behaved callers conclude the write failed short and
2018+
* kill the connection. The h1 path returns orig_len for
2019+
* exactly this reason; do the same.
2020+
*/
2021+
return (int)orig_len;
20082022
}
20092023

20102024
switch ((*wp) & 0x1f) {
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
project(lws-api-test-ws-h2-pmd C)
2+
cmake_minimum_required(VERSION 3.10)
3+
find_package(libwebsockets CONFIG REQUIRED)
4+
list(APPEND CMAKE_MODULE_PATH ${LWS_CMAKE_DIR})
5+
include(CheckCSourceCompiles)
6+
include(LwsCheckRequirements)
7+
8+
set(SAMP lws-api-test-ws-h2-pmd)
9+
set(SRCS main.c)
10+
11+
set(requirements 1)
12+
require_lws_config(LWS_ROLE_WS 1 requirements)
13+
require_lws_config(LWS_ROLE_H2 1 requirements)
14+
require_lws_config(LWS_WITH_CLIENT 1 requirements)
15+
require_lws_config(LWS_WITH_SERVER 1 requirements)
16+
require_lws_config(LWS_WITH_TLS 1 requirements)
17+
require_lws_config(LWS_WITHOUT_EXTENSIONS 0 requirements)
18+
19+
if (requirements)
20+
add_executable(${SAMP} ${SRCS})
21+
22+
if (websockets_shared)
23+
target_link_libraries(${SAMP} websockets_shared ${LIBWEBSOCKETS_DEP_LIBS})
24+
add_dependencies(${SAMP} websockets_shared)
25+
else()
26+
target_link_libraries(${SAMP} websockets ${LIBWEBSOCKETS_DEP_LIBS})
27+
endif()
28+
29+
lws_get_free_port(PORT_WS_H2_PMD)
30+
31+
add_test(NAME api-test-ws-h2-pmd COMMAND lws-api-test-ws-h2-pmd -p ${PORT_WS_H2_PMD})
32+
set_tests_properties(api-test-ws-h2-pmd
33+
PROPERTIES
34+
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}/minimal-examples-lowlevel/api-tests/api-test-ws-h2-pmd
35+
TIMEOUT 60)
36+
37+
endif()

0 commit comments

Comments
 (0)