moparisthebest/curl
1
0
Fork 0
mirror of https://github.com/moparisthebest/curl synced 2025-01-11 05:58:01 -05:00
Code Issues Releases Wiki Activity

http2: simplify and clean up trailer handling

Browse Source 
Triggered by a crash detected by OSS-Fuzz after the dynbuf introduction in
ed35d6590e. 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
This commit is contained in:
Daniel Stenberg 2020-05-06 23:31:43 +02:00
parent 7ae8801901
commit 54a2b63c70
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
3 changed files with 12 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/dynbuf.h
View File

@ -53,7 +53,7 @@ size_t Curl_dyn_len(const struct dynbuf *s);
#define DYN_HAXPROXY 2048 #define DYN_HAXPROXY 2048
#define DYN_HTTP_REQUEST (128*1024) #define DYN_HTTP_REQUEST (128*1024)
#define DYN_H2_HEADERS (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_APRINTF 8000000
#define DYN_RTSP_REQ_HEADER (64*1024) #define DYN_RTSP_REQ_HEADER (64*1024)
#define DYN_TRAILERS (64*1024) #define DYN_TRAILERS (64*1024)

1
lib/http.h
View File

@ -148,7 +148,6 @@ struct HTTP {
struct dynbuf header_recvbuf; struct dynbuf header_recvbuf;
size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into size_t nread_header_recvbuf; /* number of bytes in header_recvbuf fed into
upper layer */ upper layer */
struct dynbuf trailer_recvbuf;
int status_code; /* HTTP status code */ int status_code; /* HTTP status code */
const uint8_t *pausedata; /* pointer to data received in on_data_chunk */ const uint8_t *pausedata; /* pointer to data received in on_data_chunk */
size_t pauselen; /* the number of bytes left in data */ size_t pauselen; /* the number of bytes left in data */

53
lib/http2.c
View File

@ -126,7 +126,6 @@ static void http2_stream_free(struct HTTP *http)
{ {
if(http) { if(http) {
Curl_dyn_free(&http->header_recvbuf); Curl_dyn_free(&http->header_recvbuf);
Curl_dyn_free(&http->trailer_recvbuf);
for(; http->push_headers_used > 0; --http->push_headers_used) { for(; http->push_headers_used > 0; --http->push_headers_used) {
free(http->push_headers[http->push_headers_used - 1]); 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) { if(stream->bodystarted) {
/* This is trailer fields. */ /* This is a trailer */
/* 4 is for ": " and "\r\n". */ struct dynbuf trail;
uint32_t n = (uint32_t)(namelen + valuelen + 4);
H2BUGF(infof(data_s, "h2 trailer: %.*s: %.*s\n", namelen, name, valuelen, H2BUGF(infof(data_s, "h2 trailer: %.*s: %.*s\n", namelen, name, valuelen,
value)); value));
Curl_dyn_init(&trail, DYN_H2_TRAILER);
result = Curl_dyn_addn(&stream->trailer_recvbuf, &n, sizeof(n)); result = Curl_dyn_addf(&trail,
if(result) "%.*s: %.*s\r\n", namelen, name,
return NGHTTP2_ERR_CALLBACK_FAILURE; valuelen, value);
result = Curl_dyn_addn(&stream->trailer_recvbuf, name, namelen); if(!result)
if(result) result = Curl_client_write(conn, CLIENTWRITE_HEADER,
return NGHTTP2_ERR_CALLBACK_FAILURE; Curl_dyn_ptr(&trail),
result = Curl_dyn_add(&stream->trailer_recvbuf, ": "); Curl_dyn_len(&trail));
if(result) Curl_dyn_free(&trail);
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");
if(result) if(result)
return NGHTTP2_ERR_CALLBACK_FAILURE; 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 /* there might be allocated resources done before this got the 'h2' pointer
setup */ setup */
Curl_dyn_free(&http->header_recvbuf); Curl_dyn_free(&http->header_recvbuf);
Curl_dyn_free(&http->trailer_recvbuf);
if(http->push_headers) { if(http->push_headers) {
/* if they weren't used and then freed before */ /* if they weren't used and then freed before */
for(; http->push_headers_used > 0; --http->push_headers_used) { 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 Curl_easy *data,
struct HTTP *stream, CURLcode *err) struct HTTP *stream, CURLcode *err)
{ {
char *trailer_pos, *trailer_end;
CURLcode result;
struct http_conn *httpc = &conn->proto.httpc; struct http_conn *httpc = &conn->proto.httpc;
if(httpc->pause_stream_id == stream->stream_id) { if(httpc->pause_stream_id == stream->stream_id) {
@ -1420,25 +1409,6 @@ static ssize_t http2_handle_stream_close(struct connectdata *conn,
return -1; 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; stream->close_handled = TRUE;
H2BUGF(infof(data, "http2_recv returns 0, http2_handle_stream_close\n")); 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; stream->stream_id = -1;
Curl_dyn_init(&stream->header_recvbuf, DYN_H2_HEADERS); 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) || if((conn->handler == &Curl_handler_http2_ssl) ||
(conn->handler == &Curl_handler_http2)) (conn->handler == &Curl_handler_http2))