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
This commit is contained in:
Daniel Stenberg 2020-08-03 12:19:09 +02:00
parent 8297978c21
commit 7f187d897c
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
3 changed files with 31 additions and 10 deletions

View File

@ -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

View File

@ -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 */

View File

@ -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))