From 54a2b63c704cd963dd17101477c62afd30d1b319 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 6 May 2020 23:31:43 +0200 Subject: http2: simplify and clean up trailer handling Triggered by a crash detected by OSS-Fuzz after the dynbuf introduction in ed35d6590e72. This should make the trailer handling more straight forward and hopefully less error-prone. Deliver the trailer header to the callback already at receive-time. No longer caches the trailers to get delivered at end of stream. Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22030 Closes #5348 --- lib/dynbuf.h | 2 +- lib/http.h | 1 - lib/http2.c | 53 +++++++++++------------------------------------------ 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/lib/dynbuf.h b/lib/dynbuf.h index b4932b535..e21294115 100644 --- a/lib/dynbuf.h +++ b/lib/dynbuf.h @@ -53,7 +53,7 @@ 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_TRAILERS (128*1024) +#define DYN_H2_TRAILER 4096 #define DYN_APRINTF 8000000 #define DYN_RTSP_REQ_HEADER (64*1024) #define DYN_TRAILERS (64*1024) diff --git a/lib/http.h b/lib/http.h index 9ea3eb283..641bc0b93 100644 --- a/lib/http.h +++ b/lib/http.h @@ -148,7 +148,6 @@ 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 2ca0fdeb2..2f279f7cb 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -126,7 +126,6 @@ static void http2_stream_free(struct HTTP *http) { if(http) { Curl_dyn_free(&http->header_recvbuf); - Curl_dyn_free(&http->trailer_recvbuf); for(; http->push_headers_used > 0; --http->push_headers_used) { free(http->push_headers[http->push_headers_used - 1]); } @@ -963,26 +962,19 @@ static int on_header(nghttp2_session *session, const nghttp2_frame *frame, } if(stream->bodystarted) { - /* This is trailer fields. */ - /* 4 is for ": " and "\r\n". */ - uint32_t n = (uint32_t)(namelen + valuelen + 4); - + /* This is a trailer */ + struct dynbuf trail; H2BUGF(infof(data_s, "h2 trailer: %.*s: %.*s\n", namelen, name, valuelen, value)); - - result = Curl_dyn_addn(&stream->trailer_recvbuf, &n, sizeof(n)); - if(result) - return NGHTTP2_ERR_CALLBACK_FAILURE; - result = Curl_dyn_addn(&stream->trailer_recvbuf, name, namelen); - if(result) - return NGHTTP2_ERR_CALLBACK_FAILURE; - result = Curl_dyn_add(&stream->trailer_recvbuf, ": "); - if(result) - return NGHTTP2_ERR_CALLBACK_FAILURE; - result = Curl_dyn_addn(&stream->trailer_recvbuf, value, valuelen); - if(result) - return NGHTTP2_ERR_CALLBACK_FAILURE; - result = Curl_dyn_add(&stream->trailer_recvbuf, "\r\n\0"); + Curl_dyn_init(&trail, DYN_H2_TRAILER); + result = Curl_dyn_addf(&trail, + "%.*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; @@ -1130,7 +1122,6 @@ 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) { @@ -1376,8 +1367,6 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn, struct Curl_easy *data, struct HTTP *stream, CURLcode *err) { - char *trailer_pos, *trailer_end; - CURLcode result; struct http_conn *httpc = &conn->proto.httpc; if(httpc->pause_stream_id == stream->stream_id) { @@ -1420,25 +1409,6 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn, return -1; } - if(Curl_dyn_len(&stream->trailer_recvbuf)) { - trailer_pos = Curl_dyn_ptr(&stream->trailer_recvbuf); - trailer_end = trailer_pos + Curl_dyn_len(&stream->trailer_recvbuf); - - for(; trailer_pos < trailer_end;) { - uint32_t n; - memcpy(&n, trailer_pos, sizeof(n)); - trailer_pos += sizeof(n); - - result = Curl_client_write(conn, CLIENTWRITE_HEADER, trailer_pos, n); - if(result) { - *err = result; - return -1; - } - - trailer_pos += n + 1; - } - } - stream->close_handled = TRUE; H2BUGF(infof(data, "http2_recv returns 0, http2_handle_stream_close\n")); @@ -2118,7 +2088,6 @@ 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)) -- cgit v1.2.1