From b3fc692568ad3f01528e726223126e3fe870b1c1 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Fri, 8 Aug 2025 12:15:25 +0200 Subject: [PATCH] lib: upgrade/multiplex handling Improvements around HTTP Upgrade: and multiplex hanndling: * add `Curl_conn_set_multiplex()` to set connection's multiplex bit and trigger "connchanged" events * call `Curl_conn_set_multiplex()` in filters' `CF_CTRL_CONN_INFO_UPDATE` implementation where other connection properties are updated. This prevents connection updates before the final filter chain is chosen. * rename enum `UPGR101_INIT` to `UPGR101_NONE` * rename connection bit `asks_multiplex` to `upgrade_in_progress` * trigger "connchanged" when `upgrade_in_progress` clears * rename `WebSockets` to `WebSocket` as it is the common term used in documentation Closes #18227 --- lib/connect.c | 10 ++++++ lib/connect.h | 3 ++ lib/curl_config.h.cmake | 2 +- lib/http.c | 71 ++++++++++++++++++++++------------------- lib/http2.c | 27 +++++++++------- lib/request.c | 2 +- lib/request.h | 7 ++-- lib/url.c | 5 +-- lib/urldata.h | 2 +- lib/vquic/curl_ngtcp2.c | 5 +-- lib/vquic/curl_osslq.c | 7 ++-- lib/vquic/curl_quiche.c | 7 ++-- lib/ws.c | 7 ++-- 13 files changed, 90 insertions(+), 65 deletions(-) diff --git a/lib/connect.c b/lib/connect.c index f0628d6206..1182a42d31 100644 --- a/lib/connect.c +++ b/lib/connect.c @@ -621,3 +621,13 @@ out: Curl_resolv_unlink(data, &data->state.dns[sockindex]); return result; } + +void Curl_conn_set_multiplex(struct connectdata *conn) +{ + if(!conn->bits.multiplex) { + conn->bits.multiplex = TRUE; + if(conn->attached_multi) { + Curl_multi_connchanged(conn->attached_multi); + } + } +} diff --git a/lib/connect.h b/lib/connect.h index 6a2487ff53..cb185b2c57 100644 --- a/lib/connect.h +++ b/lib/connect.h @@ -123,6 +123,9 @@ CURLcode Curl_conn_setup(struct Curl_easy *data, struct Curl_dns_entry *dns, int ssl_mode); +/* Set conn to allow multiplexing. */ +void Curl_conn_set_multiplex(struct connectdata *conn); + extern struct Curl_cftype Curl_cft_setup; #endif /* HEADER_CURL_CONNECT_H */ diff --git a/lib/curl_config.h.cmake b/lib/curl_config.h.cmake index 30183c2009..bc8c1cd487 100644 --- a/lib/curl_config.h.cmake +++ b/lib/curl_config.h.cmake @@ -148,7 +148,7 @@ /* disables SMTP */ #cmakedefine CURL_DISABLE_SMTP 1 -/* disabled WebSockets */ +/* disabled WebSocket */ #cmakedefine CURL_DISABLE_WEBSOCKETS 1 /* disables use of socketpair for curl_multi_poll */ diff --git a/lib/http.c b/lib/http.c index e01de6f477..ce31e6dff0 100644 --- a/lib/http.c +++ b/lib/http.c @@ -2289,7 +2289,7 @@ static CURLcode addexpect(struct Curl_easy *data, struct dynbuf *r, *announced_exp100 = FALSE; /* Avoid Expect: 100-continue if Upgrade: is used */ - if(data->req.upgr101 != UPGR101_INIT) + if(data->req.upgr101 != UPGR101_NONE) return CURLE_OK; /* For really small puts we do not use Expect: headers at all, and for @@ -2622,6 +2622,7 @@ static CURLcode http_check_new_conn(struct Curl_easy *data) info_version = "HTTP/2"; /* There is no ALPN here, but the connection is now definitely h2 */ conn->httpversion_seen = 20; + Curl_conn_set_multiplex(conn); } else info_version = "HTTP/1.x"; @@ -3571,10 +3572,6 @@ static CURLcode http_statusline(struct Curl_easy *data, infof(data, "HTTP 1.0, assume close after body"); connclose(conn, "HTTP/1.0 close after body"); } - else if(k->httpversion == 20 || - (k->upgr101 == UPGR101_H2 && k->httpcode == 101)) { - DEBUGF(infof(data, "HTTP/2 found, allow multiplexing")); - } k->http_bodyless = k->httpcode >= 100 && k->httpcode < 200; switch(k->httpcode) { @@ -3717,6 +3714,7 @@ static CURLcode http_on_response(struct Curl_easy *data, struct connectdata *conn = data->conn; CURLcode result = CURLE_OK; struct SingleRequest *k = &data->req; + bool conn_changed = FALSE; (void)buf; /* not used without HTTP2 enabled */ *pconsumed = 0; @@ -3757,47 +3755,54 @@ static CURLcode http_on_response(struct Curl_easy *data, */ http_exp100_got100(data); break; - case 101: - /* Switching Protocols only allowed from HTTP/1.1 */ + case 101: { + int upgr101_requested = k->upgr101; + if(k->httpversion_sent != 11) { /* invalid for other HTTP versions */ - failf(data, "unexpected 101 response code"); + failf(data, "server sent 101 response while not talking HTTP/1.1"); result = CURLE_WEIRD_SERVER_REPLY; goto out; } - if(k->upgr101 == UPGR101_H2) { - /* Switching to HTTP/2, where we will get more responses */ + + /* Whatever the success, upgrade was selected. */ + k->upgr101 = UPGR101_RECEIVED; + data->conn->bits.upgrade_in_progress = FALSE; + conn_changed = TRUE; + + /* To be fully conform, we would check the "Upgrade:" response header + * to mention the protocol we requested. */ + switch(upgr101_requested) { + case UPGR101_H2: + /* Switch to HTTP/2, where we will get more responses. + * blen bytes in bug are already h2 protocol bytes */ infof(data, "Received 101, Switching to HTTP/2"); - k->upgr101 = UPGR101_RECEIVED; - data->conn->bits.asks_multiplex = FALSE; - /* We expect more response from HTTP/2 later */ - k->header = TRUE; - k->headerline = 0; /* restart the header line counter */ - k->httpversion_sent = 20; /* It's an HTTP/2 request now */ - /* Any remaining `buf` bytes are already HTTP/2 and passed to - * be processed. */ result = Curl_http2_upgrade(data, conn, FIRSTSOCKET, buf, blen); if(result) goto out; *pconsumed += blen; - } + break; #ifndef CURL_DISABLE_WEBSOCKETS - else if(k->upgr101 == UPGR101_WS) { - /* verify the response. Any passed `buf` bytes are already in - * WebSockets format and taken in by the protocol handler. */ + case UPGR101_WS: + /* Switch to WebSocket, where we now stream ws frames. + * blen bytes in bug are already ws protocol bytes */ + infof(data, "Received 101, Switching to WebSocket"); result = Curl_ws_accept(data, buf, blen); if(result) goto out; *pconsumed += blen; /* ws accept handled the data */ - } + break; #endif - else { + default: /* We silently accept this as the final response. What are we * switching to if we did not ask for an Upgrade? Maybe the * application provided an `Upgrade: xxx` header? */ k->header = FALSE; + break; } + /* processed 101 */ break; + } default: /* The server may send us other 1xx responses, like informative * 103. This have no influence on request processing and we expect @@ -3809,12 +3814,10 @@ static CURLcode http_on_response(struct Curl_easy *data, /* k->httpcode >= 200, final response */ k->header = FALSE; - - if(k->upgr101 == UPGR101_H2) { - /* A requested upgrade was denied, poke the multi handle to possibly - allow a pending pipewait to continue */ - data->conn->bits.asks_multiplex = FALSE; - Curl_multi_connchanged(data->multi); + if(data->conn->bits.upgrade_in_progress) { + /* Asked for protocol upgrade, but it was not selected by the server */ + data->conn->bits.upgrade_in_progress = FALSE; + conn_changed = TRUE; } if((k->size == -1) && !k->chunk && !conn->bits.close && @@ -3863,9 +3866,9 @@ static CURLcode http_on_response(struct Curl_easy *data, #endif #ifndef CURL_DISABLE_WEBSOCKETS - /* All >=200 HTTP status codes are errors when wanting WebSockets */ + /* All >=200 HTTP status codes are errors when wanting WebSocket */ if(data->req.upgr101 == UPGR101_WS) { - failf(data, "Refused WebSockets upgrade: %d", k->httpcode); + failf(data, "Refused WebSocket upgrade: %d", k->httpcode); result = CURLE_HTTP_RETURNED_ERROR; goto out; } @@ -3985,6 +3988,10 @@ out: result = Curl_1st_err( result, http_write_header(data, last_hd, last_hd_len)); } + if(conn_changed) { + /* poke the multi handle to allow any pending pipewait to retry now */ + Curl_multi_connchanged(data->multi); + } return result; } diff --git a/lib/http2.c b/lib/http2.c index e2cdc9181f..c526bf0fa4 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -1829,7 +1829,7 @@ CURLcode Curl_http2_request_upgrade(struct dynbuf *req, free(base64); k->upgr101 = UPGR101_H2; - data->conn->bits.asks_multiplex = TRUE; + data->conn->bits.upgrade_in_progress = TRUE; return result; } @@ -2696,6 +2696,12 @@ static CURLcode cf_h2_cntrl(struct Curl_cfilter *cf, case CF_CTRL_DATA_DONE: http2_data_done(cf, data); break; + case CF_CTRL_CONN_INFO_UPDATE: + if(!cf->sockindex && cf->connected) { + cf->conn->httpversion_seen = 20; + Curl_conn_set_multiplex(cf->conn); + } + break; default: break; } @@ -2895,9 +2901,6 @@ CURLcode Curl_http2_switch(struct Curl_easy *data) return result; CURL_TRC_CF(data, cf, "switching connection to HTTP/2"); - data->conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ - Curl_multi_connchanged(data->multi); - if(cf->next) { bool done; return Curl_conn_cf_connect(cf, data, &done); @@ -2917,8 +2920,6 @@ CURLcode Curl_http2_switch_at(struct Curl_cfilter *cf, struct Curl_easy *data) return result; cf_h2 = cf->next; - cf->conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ - Curl_multi_connchanged(data->multi); if(cf_h2->next) { bool done; @@ -2936,7 +2937,6 @@ CURLcode Curl_http2_upgrade(struct Curl_easy *data, CURLcode result; DEBUGASSERT(Curl_conn_http_version(data, conn) < 20); - DEBUGASSERT(data->req.upgr101 == UPGR101_RECEIVED); result = http2_cfilter_add(&cf, data, conn, sockindex, TRUE); if(result) @@ -2946,6 +2946,10 @@ CURLcode Curl_http2_upgrade(struct Curl_easy *data, DEBUGASSERT(cf->cft == &Curl_cft_nghttp2); ctx = cf->ctx; + data->req.httpversion_sent = 20; /* it is an h2 request now */ + data->req.header = TRUE; /* we expect the real response to come in h2 */ + data->req.headerline = 0; /* restart the header line counter */ + if(nread > 0) { /* Remaining data from the protocol switch reply is already using * the switched protocol, ie. HTTP/2. We add that to the network @@ -2968,14 +2972,13 @@ CURLcode Curl_http2_upgrade(struct Curl_easy *data, " after upgrade: len=%zu", nread); } - conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ - Curl_multi_connchanged(data->multi); - if(cf->next) { bool done; - return Curl_conn_cf_connect(cf, data, &done); + result = Curl_conn_cf_connect(cf, data, &done); + if(!result) + cf->cft->cntrl(cf, data, CF_CTRL_CONN_INFO_UPDATE, 0, NULL); } - return CURLE_OK; + return result; } /* Only call this function for a transfer that already got an HTTP/2 diff --git a/lib/request.c b/lib/request.c index 8751ada559..2d5ad95212 100644 --- a/lib/request.c +++ b/lib/request.c @@ -139,7 +139,7 @@ void Curl_req_hard_reset(struct SingleRequest *req, struct Curl_easy *data) req->offset = 0; req->httpcode = 0; req->keepon = 0; - req->upgr101 = UPGR101_INIT; + req->upgr101 = UPGR101_NONE; req->sendbuf_hds_len = 0; req->timeofdoc = 0; req->location = NULL; diff --git a/lib/request.h b/lib/request.h index bce34de8ba..e12d5efdcb 100644 --- a/lib/request.h +++ b/lib/request.h @@ -42,11 +42,10 @@ enum expect100 { }; enum upgrade101 { - UPGR101_INIT, /* default state */ - UPGR101_WS, /* upgrade to WebSockets requested */ + UPGR101_NONE, /* default state */ + UPGR101_WS, /* upgrade to WebSocket requested */ UPGR101_H2, /* upgrade to HTTP/2 requested */ - UPGR101_RECEIVED, /* 101 response received */ - UPGR101_WORKING /* talking upgraded protocol */ + UPGR101_RECEIVED /* 101 response received */ }; diff --git a/lib/url.c b/lib/url.c index 6af2b7fb8b..41a63890e6 100644 --- a/lib/url.c +++ b/lib/url.c @@ -907,8 +907,8 @@ static bool url_match_fully_connected(struct connectdata *conn, struct url_conn_match *m) { if(!Curl_conn_is_connected(conn, FIRSTSOCKET) || - conn->bits.asks_multiplex) { - /* Not yet connected, or not yet decided if it multiplexes. The later + conn->bits.upgrade_in_progress) { + /* Not yet connected, or a protocol upgrade is in progress. The later * happens for HTTP/2 Upgrade: requests that need a response. */ if(m->may_multiplex) { m->seen_pending_conn = TRUE; @@ -1268,6 +1268,7 @@ static bool url_match_conn(struct connectdata *conn, void *userdata) if(!url_match_connect_config(conn, m)) return FALSE; + /* match for destination and protocol? */ if(!url_match_destination(conn, m)) return FALSE; diff --git a/lib/urldata.h b/lib/urldata.h index b2e83c4a0e..3c7e634b00 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -389,7 +389,7 @@ struct ConnectBits { #endif BIT(bound); /* set true if bind() has already been done on this socket/ connection */ - BIT(asks_multiplex); /* connection asks for multiplexing, but is not yet */ + BIT(upgrade_in_progress); /* protocol upgrade is in progress */ BIT(multiplex); /* connection is multiplexed */ BIT(tcp_fastopen); /* use TCP Fast Open */ BIT(tls_enable_alpn); /* TLS ALPN extension? */ diff --git a/lib/vquic/curl_ngtcp2.c b/lib/vquic/curl_ngtcp2.c index f902c190ef..0e05694992 100644 --- a/lib/vquic/curl_ngtcp2.c +++ b/lib/vquic/curl_ngtcp2.c @@ -464,7 +464,6 @@ static int cf_ngtcp2_handshake_completed(ngtcp2_conn *tconn, void *user_data) ctx->handshake_at = curlx_now(); ctx->tls_handshake_complete = TRUE; - cf->conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ Curl_vquic_report_handshake(&ctx->tls, cf, data); ctx->tls_vrfy_result = Curl_vquic_tls_verify_peer(&ctx->tls, cf, @@ -1989,8 +1988,10 @@ static CURLcode cf_ngtcp2_cntrl(struct Curl_cfilter *cf, break; } case CF_CTRL_CONN_INFO_UPDATE: - if(!cf->sockindex && cf->connected) + if(!cf->sockindex && cf->connected) { cf->conn->httpversion_seen = 30; + Curl_conn_set_multiplex(cf->conn); + } break; default: break; diff --git a/lib/vquic/curl_osslq.c b/lib/vquic/curl_osslq.c index c292072a65..5e9b072736 100644 --- a/lib/vquic/curl_osslq.c +++ b/lib/vquic/curl_osslq.c @@ -564,9 +564,6 @@ static CURLcode cf_osslq_verify_peer(struct Curl_cfilter *cf, struct Curl_easy *data) { struct cf_osslq_ctx *ctx = cf->ctx; - - cf->conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ - return Curl_vquic_tls_verify_peer(&ctx->tls, cf, data, &ctx->peer); } @@ -2205,8 +2202,10 @@ static CURLcode cf_osslq_cntrl(struct Curl_cfilter *cf, break; } case CF_CTRL_CONN_INFO_UPDATE: - if(!cf->sockindex && cf->connected) + if(!cf->sockindex && cf->connected) { cf->conn->httpversion_seen = 30; + Curl_conn_set_multiplex(cf->conn); + } break; default: break; diff --git a/lib/vquic/curl_quiche.c b/lib/vquic/curl_quiche.c index b88b4e97bd..523f04e33b 100644 --- a/lib/vquic/curl_quiche.c +++ b/lib/vquic/curl_quiche.c @@ -1234,8 +1234,10 @@ static CURLcode cf_quiche_cntrl(struct Curl_cfilter *cf, break; } case CF_CTRL_CONN_INFO_UPDATE: - if(!cf->sockindex && cf->connected) + if(!cf->sockindex && cf->connected) { cf->conn->httpversion_seen = 30; + Curl_conn_set_multiplex(cf->conn); + } break; default: break; @@ -1350,9 +1352,6 @@ static CURLcode cf_quiche_verify_peer(struct Curl_cfilter *cf, struct Curl_easy *data) { struct cf_quiche_ctx *ctx = cf->ctx; - - cf->conn->bits.multiplex = TRUE; /* at least potentially multiplexed */ - return Curl_vquic_tls_verify_peer(&ctx->tls, cf, data, &ctx->peer); } diff --git a/lib/ws.c b/lib/ws.c index b6ab28a35a..6a265fccc7 100644 --- a/lib/ws.c +++ b/lib/ws.c @@ -582,7 +582,7 @@ static void update_meta(struct websocket *ws, ws->recvframe.bytesleft = bytesleft; } -/* WebSockets decoding client writer */ +/* WebSocket decoding client writer */ struct ws_cw_ctx { struct Curl_cwriter super; struct bufq buf; @@ -1268,6 +1268,7 @@ CURLcode Curl_ws_request(struct Curl_easy *data, struct dynbuf *req) } data->state.http_hd_upgrade = TRUE; k->upgr101 = UPGR101_WS; + data->conn->bits.upgrade_in_progress = TRUE; return result; } @@ -1359,6 +1360,8 @@ CURLcode Curl_ws_accept(struct Curl_easy *data, goto out; ws_dec_writer = NULL; /* owned by transfer now */ + k->header = FALSE; /* we will not get more response headers */ + if(data->set.connect_only) { size_t nwritten; /* In CONNECT_ONLY setup, the payloads from `mem` need to be received @@ -1806,7 +1809,7 @@ out: static CURLcode ws_setup_conn(struct Curl_easy *data, struct connectdata *conn) { - /* WebSockets is 1.1 only (for now) */ + /* WebSocket is 1.1 only (for now) */ data->state.http_neg.accept_09 = FALSE; data->state.http_neg.only_10 = FALSE; data->state.http_neg.wanted = CURL_HTTP_V1x; -- 2.47.3