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

diff --git a/lib/connect.c b/lib/connect.c
index f0628d6206eda6cdee7aadf58d1f239dee93b567..1182a42d31fa14637ee10530ab1b3539239469eb 100644 (file)
--- 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 6a2487ff53831982dbd76f6e337853a4a591119c..cb185b2c571b46a5ea2b92b622a03bce97dfbe96 100644 (file)
--- 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 30183c20092fe2ef1cd1e1b6b26c1000a63b4aac..bc8c1cd487cc8c5053e54e1f3aa013f016995f7c 100644 (file)
--- 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 e01de6f4772c9a0fe544d3bfc877113d1a63abe1..ce31e6dff005335071fb7021e439c5331611a8e2 100644 (file)
--- 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 e2cdc9181f5f22ce59a6decb8387542e474c51c3..c526bf0fa4c3b3cdce14503798412b6971e94486 100644 (file)
--- 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 8751ada55987b60a0caa072598a2ea58f780371a..2d5ad9521209d24c12e451e52c24384611370bde 100644 (file)
--- 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 bce34de8ba9e47ab8eedd907b053f424de1ecf3b..e12d5efdcb23441e9852aea907c1646c4006f713 100644 (file)
--- 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 6af2b7fb8b752ca507ee67b5fbbfed0ed0f8faea..41a63890e6ef66d5e815b36670de4a8ef94292ac 100644 (file)
--- 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 b2e83c4a0eef4984a325540b75aa4ffe4a1a92b4..3c7e634b00087ea973c2161bb3f6fc84e9d27cd5 100644 (file)
--- 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 f902c190ef584611ce347c6508eb28784f765358..0e05694992a0d24a511b6cc9dbe4ec675ee3fe2e 100644 (file)
--- 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 c292072a65b6b7526a73cae73253224cbcafa467..5e9b0727360bb65c8cc2a47c9fd4acbf58a7aae3 100644 (file)
--- 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 b88b4e97bd3dd412f91403b8747bf8133c973343..523f04e33bd99a4bb60ef7dcd4a46171a307823a 100644 (file)
--- 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 b6ab28a35ad0accfb0c3c87b84f1e5312e2a3f4f..6a265fccc700f72a03b373dc29a69853ea5ad886 100644 (file)
--- 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;