hyper: temporarily remove HTTP/2 support

The current design of the Hyper integration requires rebuilding the
Hyper clientconn for each request. However, building the clientconn
requires resending the HTTP/2 connection preface, which is incorrect
from a protocol perspective. That in turn causes servers to send GOAWAY
frames, effectively degrading performance to "no connection reuse" in
the best case. It may also be triggering some bugs where requests get
dropped entirely and reconnects take too long.

This doesn't rule out HTTP/2 support with Hyper, but it may take a
redesign of the Hyper integration in order to make things work.

Closes #12191

diff --git a/configure.ac b/configure.ac
index 83ad82231dc80a72d79865724f978ccf9579cc00..a2c5165034e99804a3f6c58d2d0198e68531d345 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -174,7 +174,7 @@ curl_headers_msg="enabled (--disable-headers-api)"
      curl_ws_msg="no      (--enable-websockets)"
     ssl_backends=
      curl_h1_msg="enabled (internal)"
-     curl_h2_msg="no      (--with-nghttp2, --with-hyper)"
+     curl_h2_msg="no      (--with-nghttp2)"
      curl_h3_msg="no      (--with-ngtcp2 --with-nghttp3, --with-quiche, --with-msh3)"
 
 enable_altsvc="yes"
@@ -803,7 +803,6 @@ if test X"$want_hyper" != Xno; then
           experimental="$experimental Hyper"
           AC_MSG_NOTICE([Hyper support is experimental])
           curl_h1_msg="enabled (Hyper)"
-          curl_h2_msg=$curl_h1_msg
           HYPER_ENABLED=1
           AC_DEFINE(USE_HYPER, 1, [if hyper is in use])
           AC_SUBST(USE_HYPER, [1])
@@ -4576,7 +4575,7 @@ if test "x$USE_TLS_SRP" = "x1"; then
   SUPPORT_FEATURES="$SUPPORT_FEATURES TLS-SRP"
 fi
 
-if test "x$USE_NGHTTP2" = "x1" -o "x$USE_HYPER" = "x1"; then
+if test "x$USE_NGHTTP2" = "x1"; then
   SUPPORT_FEATURES="$SUPPORT_FEATURES HTTP2"
 fi
 

diff --git a/docs/HYPER.md b/docs/HYPER.md
index 1c3b0dded40f3112f389b7cf6ec3634176048d93..9932c1bbf73de0c0755006c7dbe0fe3dc2115971 100644 (file)
--- a/docs/HYPER.md
+++ b/docs/HYPER.md
@@ -57,6 +57,10 @@ The hyper backend does not support
 - leading whitespace in first HTTP/1 response header
 - HTTP/0.9
 - HTTP/2 upgrade using HTTP:// URLs. Aka 'h2c'
+- HTTP/2 in general. Hyper has support for HTTP/2 but the curl side
+  needs changes so that a `hyper_clientconn` can last for the duration
+  of a connection. Probably this means turning the Hyper HTTP/2 backend
+  into a connection filter.
 
 ## Remaining issues
 

diff --git a/lib/c-hyper.c b/lib/c-hyper.c
index f34c44cc7376787852747fbfb6b2e42242f7acc4..e3f7d6d44474d32bdb9e4c6711f7e757e1b6f45e 100644 (file)
--- a/lib/c-hyper.c
+++ b/lib/c-hyper.c
@@ -22,6 +22,10 @@
  *
  ***************************************************************************/
 
+/* Curl's integration with Hyper. This replaces certain functions in http.c,
+ * based on configuration #defines. This implementation supports HTTP/1.1 but
+ * not HTTP/2.
+ */
 #include "curl_setup.h"
 
 #if !defined(CURL_DISABLE_HTTP) && defined(USE_HYPER)
@@ -539,11 +543,9 @@ CURLcode Curl_hyper_stream(struct Curl_easy *data,
 
 static CURLcode debug_request(struct Curl_easy *data,
                               const char *method,
-                              const char *path,
-                              bool h2)
+                              const char *path)
 {
-  char *req = aprintf("%s %s HTTP/%s\r\n", method, path,
-                      h2?"2":"1.1");
+  char *req = aprintf("%s %s HTTP/1.1\r\n", method, path);
   if(!req)
     return CURLE_OUT_OF_MEMORY;
   Curl_debug(data, CURLINFO_HEADER_OUT, req, strlen(req));
@@ -625,7 +627,6 @@ CURLcode Curl_hyper_header(struct Curl_easy *data, hyper_headers *headers,
 static CURLcode request_target(struct Curl_easy *data,
                                struct connectdata *conn,
                                const char *method,
-                               bool h2,
                                hyper_request *req)
 {
   CURLcode result;
@@ -637,26 +638,13 @@ static CURLcode request_target(struct Curl_easy *data,
   if(result)
     return result;
 
-  if(h2 && hyper_request_set_uri_parts(req,
-                                       /* scheme */
-                                       (uint8_t *)data->state.up.scheme,
-                                       strlen(data->state.up.scheme),
-                                       /* authority */
-                                       (uint8_t *)conn->host.name,
-                                       strlen(conn->host.name),
-                                       /* path_and_query */
-                                       (uint8_t *)Curl_dyn_uptr(&r),
-                                       Curl_dyn_len(&r))) {
-    failf(data, "error setting uri parts to hyper");
-    result = CURLE_OUT_OF_MEMORY;
-  }
-  else if(!h2 && hyper_request_set_uri(req, (uint8_t *)Curl_dyn_uptr(&r),
+  if(hyper_request_set_uri(req, (uint8_t *)Curl_dyn_uptr(&r),
                                        Curl_dyn_len(&r))) {
     failf(data, "error setting uri to hyper");
     result = CURLE_OUT_OF_MEMORY;
   }
   else
-    result = debug_request(data, method, Curl_dyn_ptr(&r), h2);
+    result = debug_request(data, method, Curl_dyn_ptr(&r));
 
   Curl_dyn_free(&r);
 
@@ -887,7 +875,6 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
   const char *p_accept; /* Accept: string */
   const char *method;
   Curl_HttpReq httpreq;
-  bool h2 = FALSE;
   const char *te = NULL; /* transfer-encoding */
   hyper_code rc;
 
@@ -960,8 +947,9 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
     goto error;
   }
   if(conn->alpn == CURL_HTTP_VERSION_2) {
-    hyper_clientconn_options_http2(options, 1);
-    h2 = TRUE;
+    failf(data, "ALPN protocol h2 not supported with Hyper");
+    result = CURLE_UNSUPPORTED_PROTOCOL;
+    goto error;
   }
   hyper_clientconn_options_set_preserve_header_case(options, 1);
   hyper_clientconn_options_set_preserve_header_order(options, 1);
@@ -1012,7 +1000,7 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
     }
   }
   else {
-    if(!h2 && !data->state.disableexpect) {
+    if(!data->state.disableexpect) {
       data->state.expect100header = TRUE;
     }
   }
@@ -1023,7 +1011,7 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
     goto error;
   }
 
-  result = request_target(data, conn, method, h2, req);
+  result = request_target(data, conn, method, req);
   if(result)
     goto error;
 
@@ -1044,19 +1032,10 @@ CURLcode Curl_http(struct Curl_easy *data, bool *done)
   if(result)
     goto error;
 
-  if(!h2) {
-    if(data->state.aptr.host) {
-      result = Curl_hyper_header(data, headers, data->state.aptr.host);
-      if(result)
-        goto error;
-    }
-  }
-  else {
-    /* For HTTP/2, we show the Host: header as if we sent it, to make it look
-       like for HTTP/1 but it isn't actually sent since :authority is then
-       used. */
-    Curl_debug(data, CURLINFO_HEADER_OUT, data->state.aptr.host,
-               strlen(data->state.aptr.host));
+  if(data->state.aptr.host) {
+    result = Curl_hyper_header(data, headers, data->state.aptr.host);
+    if(result)
+      goto error;
   }
 
   if(data->state.aptr.proxyuserpwd) {

diff --git a/lib/curl_setup.h b/lib/curl_setup.h
index 10bb0d7f2389f227c7aeb1d4805e126bd417bdf7..63320d73c17d3b3b2548620a9abff5bcf2bb7021 100644 (file)
--- a/lib/curl_setup.h
+++ b/lib/curl_setup.h
@@ -761,7 +761,8 @@ int getpwuid_r(uid_t uid, struct passwd *pwd, char *buf,
 #define UNITTEST static
 #endif
 
-#if defined(USE_NGHTTP2) || defined(USE_HYPER)
+/* Hyper supports HTTP2 also, but Curl's integration with Hyper does not */
+#if defined(USE_NGHTTP2)
 #define USE_HTTP2
 #endif
 

diff --git a/lib/version.c b/lib/version.c
index d1855313c203de74479f2cf671003f46c4d18fef..175550b9fe809d7d8bb3c7b3d511dbd1efc3c784 100644 (file)
--- a/lib/version.c
+++ b/lib/version.c
@@ -455,7 +455,7 @@ static const struct feat features_table[] = {
 #ifndef CURL_DISABLE_HSTS
   FEATURE("HSTS",        NULL,                CURL_VERSION_HSTS),
 #endif
-#if defined(USE_NGHTTP2) || defined(USE_HYPER)
+#if defined(USE_NGHTTP2)
   FEATURE("HTTP2",       NULL,                CURL_VERSION_HTTP2),
 #endif
 #if defined(ENABLE_QUIC)