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
This commit is contained in:
Jacob Hoffman-Andrews 2023-10-24 07:51:05 -07:00 committed by Daniel Stenberg
parent 9ac6023d36
commit b9b50f3193
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
5 changed files with 26 additions and 43 deletions

View File

@ -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

View File

@ -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

View File

@ -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) {

View File

@ -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

View File

@ -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)