aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Stenberg <daniel@haxx.se>2020-05-06 23:31:43 +0200
committerDaniel Stenberg <daniel@haxx.se>2020-05-07 09:49:51 +0200
commit54a2b63c704cd963dd17101477c62afd30d1b319 (patch)
treef9d033beb66bf57e02c83c025e9bcac15421296c
parent7ae880190126bb5c5a4698f1b290457ea54d78c0 (diff)
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
-rw-r--r--lib/dynbuf.h2
-rw-r--r--lib/http.h1
-rw-r--r--lib/http2.c53
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))