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

diff --git a/lib/dynbuf.h b/lib/dynbuf.h
index b4932b5351c877f636110a5f158da9304f0ee5af..e21294115629aa4f7131df46c5dd1a18bed1cbaf 100644 (file)
--- 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 9ea3eb28306a747031b65b5cdc8dff5d39edf2e9..641bc0b93ab7c05790977c16e4b7adb7a33c47d1 100644 (file)
--- 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 2ca0fdeb2bf3eb593b4ba9799063a248bad67b5f..2f279f7cb27233097d493083a5e6d23d64fabd0e 100644 (file)
--- 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))