From 7f187d897c000ea64d38aa29026a7837a88427df Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 3 Aug 2020 12:19:09 +0200 Subject: [PATCH] h2: repair trailer handling The previous h2 trailer fix in 54a2b63 was wrong and caused a regression: it cannot deal with trailers immediately when read since they may be read off the connection by the wrong 'data' owner. This change reverts the logic back to gathering all trailers into a single buffer, like before 54a2b63. Reported-by: Tadej Vengust Fixes #5663 Closes #5769 --- lib/dynbuf.h | 4 ++-- lib/http.h | 1 + lib/http2.c | 36 ++++++++++++++++++++++++++++-------- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/dynbuf.h b/lib/dynbuf.h index c80239e29..ecc995755 100644 --- a/lib/dynbuf.h +++ b/lib/dynbuf.h @@ -53,11 +53,11 @@ size_t Curl_dyn_len(const struct dynbuf *s); #define DYN_HAXPROXY 2048 #define DYN_HTTP_REQUEST (128*1024) #define DYN_H2_HEADERS (128*1024) -#define DYN_H2_TRAILER 4096 +#define DYN_H2_TRAILERS (128*1024) #define DYN_APRINTF 8000000 #define DYN_RTSP_REQ_HEADER (64*1024) #define DYN_TRAILERS (64*1024) #define DYN_PROXY_CONNECT_HEADERS 16384 #define DYN_QLOG_NAME 1024 -#define DYN_H1_TRAILER DYN_H2_TRAILER +#define DYN_H1_TRAILER 4096 #endif diff --git a/lib/http.h b/lib/http.h index 641bc0b93..9ea3eb283 100644 --- a/lib/http.h +++ b/lib/http.h @@ -148,6 +148,7 @@ struct HTTP { struct dynbuf header_recvbuf; size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into upper layer */ + struct dynbuf trailer_recvbuf; int status_code; /* HTTP status code */ const uint8_t *pausedata; /* pointer to data received in on_data_chunk */ size_t pauselen; /* the number of bytes left in data */ diff --git a/lib/http2.c b/lib/http2.c index e4fa9b0b3..d316da8b6 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -1015,18 +1015,11 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, if(stream->bodystarted) { /* This is a trailer */ - struct dynbuf trail; H2BUGF(infof(data_s, "h2 trailer: %.*s: %.*s\n", namelen, name, valuelen, value)); - Curl_dyn_init(&trail, DYN_H2_TRAILER); - result = Curl_dyn_addf(&trail, + result = Curl_dyn_addf(&stream->trailer_recvbuf, "%.*s: %.*s\r\n", namelen, name, valuelen, value); - if(!result) - result = Curl_client_write(conn, CLIENTWRITE_HEADER, - Curl_dyn_ptr(&trail), - Curl_dyn_len(&trail)); - Curl_dyn_free(&trail); if(result) return NGHTTP2_ERR_CALLBACK_FAILURE; @@ -1174,6 +1167,7 @@ void Curl_http2_done(struct Curl_easy *data, bool premature) /* there might be allocated resources done before this got the 'h2' pointer setup */ Curl_dyn_free(&http->header_recvbuf); + Curl_dyn_free(&http->trailer_recvbuf); if(http->push_headers) { /* if they weren't used and then freed before */ for(; http->push_headers_used > 0; --http->push_headers_used) { @@ -1487,6 +1481,31 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn, return -1; } + if(Curl_dyn_len(&stream->trailer_recvbuf)) { + char *trailp = Curl_dyn_ptr(&stream->trailer_recvbuf); + char *lf; + + do { + size_t len = 0; + CURLcode result; + /* each trailer line ends with a newline */ + lf = strchr(trailp, '\n'); + if(!lf) + break; + len = lf + 1 - trailp; + + if(data->set.verbose) + Curl_debug(data, CURLINFO_HEADER_IN, trailp, len); + /* pass the trailers one by one to the callback */ + result = Curl_client_write(conn, CLIENTWRITE_HEADER, trailp, len); + if(result) { + *err = result; + return -1; + } + trailp = ++lf; + } while(lf); + } + stream->close_handled = TRUE; H2BUGF(infof(data, "http2_recv returns 0, http2_handle_stream_close\n")); @@ -2171,6 +2190,7 @@ CURLcode Curl_http2_setup(struct connectdata *conn) stream->stream_id = -1; Curl_dyn_init(&stream->header_recvbuf, DYN_H2_HEADERS); + Curl_dyn_init(&stream->trailer_recvbuf, DYN_H2_TRAILERS); if((conn->handler == &Curl_handler_http2_ssl) || (conn->handler == &Curl_handler_http2))