From e0363a47de4972c7d23d87b8f75a683cf0c4271b Mon Sep 17 00:00:00 2001 From: Javier Blazquez Date: Fri, 15 Nov 2019 23:57:45 -0800 Subject: ngtcp2: use overflow buffer for extra HTTP/3 data Fixes #4525 Closes #4603 --- lib/http.c | 1 + lib/http.h | 4 + lib/quic.h | 4 + lib/transfer.c | 7 +- lib/vquic/ngtcp2.c | 220 ++++++++++++++++++++++++++++++++++++++++++++--------- lib/vquic/quiche.c | 19 +++++ 6 files changed, 218 insertions(+), 37 deletions(-) (limited to 'lib') diff --git a/lib/http.c b/lib/http.c index 4631a7f36..e344663d0 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1618,6 +1618,7 @@ CURLcode Curl_http_done(struct connectdata *conn, } Curl_http2_done(conn, premature); + Curl_quic_done(data, premature); Curl_mime_cleanpart(&http->form); diff --git a/lib/http.h b/lib/http.h index 87e979ee2..70d5dccec 100644 --- a/lib/http.h +++ b/lib/http.h @@ -193,6 +193,7 @@ struct HTTP { #ifdef ENABLE_QUIC /*********** for HTTP/3 we store stream-local data here *************/ int64_t stream3_id; /* stream we are interested in */ + bool firstheader; /* FALSE until headers arrive */ bool firstbody; /* FALSE until body arrives */ bool h3req; /* FALSE until request is issued */ bool upload_done; @@ -200,6 +201,9 @@ struct HTTP { #ifdef USE_NGHTTP3 size_t unacked_window; struct h3out *h3out; /* per-stream buffers for upload */ + char *overflow_buf; /* excess data received during a single Curl_read */ + size_t overflow_buflen; /* amount of data currently in overflow_buf */ + size_t overflow_bufsize; /* size of the overflow_buf allocation */ #endif }; diff --git a/lib/quic.h b/lib/quic.h index 6c132a324..1eb23e976 100644 --- a/lib/quic.h +++ b/lib/quic.h @@ -45,9 +45,13 @@ CURLcode Curl_quic_is_connected(struct connectdata *conn, bool *connected); int Curl_quic_ver(char *p, size_t len); CURLcode Curl_quic_done_sending(struct connectdata *conn); +void Curl_quic_done(struct Curl_easy *data, bool premature); +bool Curl_quic_data_pending(const struct Curl_easy *data); #else /* ENABLE_QUIC */ #define Curl_quic_done_sending(x) +#define Curl_quic_done(x,y) +#define Curl_quic_data_pending(x) #endif /* !ENABLE_QUIC */ #endif /* HEADER_CURL_QUIC_H */ diff --git a/lib/transfer.c b/lib/transfer.c index d0d4aeb50..f16e29bdb 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -484,8 +484,9 @@ CURLcode Curl_readrewind(struct connectdata *conn) return CURLE_OK; } -static int data_pending(const struct connectdata *conn) +static int data_pending(const struct Curl_easy *data) { + struct connectdata *conn = data->conn; /* in the case of libssh2, we can never be really sure that we have emptied its internal buffers so we MUST always try until we get EAGAIN back */ return conn->handler->protocol&(CURLPROTO_SCP|CURLPROTO_SFTP) || @@ -499,6 +500,8 @@ static int data_pending(const struct connectdata *conn) be called and we cannot signal the HTTP/2 stream has closed. As a workaround, we return nonzero here to call http2_recv. */ ((conn->handler->protocol&PROTO_FAMILY_HTTP) && conn->httpversion >= 20); +#elif defined(ENABLE_QUIC) + Curl_ssl_data_pending(conn, FIRSTSOCKET) || Curl_quic_data_pending(data); #else Curl_ssl_data_pending(conn, FIRSTSOCKET); #endif @@ -918,7 +921,7 @@ static CURLcode readwrite_data(struct Curl_easy *data, break; } - } while(data_pending(conn) && maxloops--); + } while(data_pending(data) && maxloops--); if(maxloops <= 0) { /* we mark it as read-again-please */ diff --git a/lib/vquic/ngtcp2.c b/lib/vquic/ngtcp2.c index 36aa6c20f..071d45c02 100644 --- a/lib/vquic/ngtcp2.c +++ b/lib/vquic/ngtcp2.c @@ -754,41 +754,119 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t stream_id, stream->closed = TRUE; Curl_expire(data, 0, EXPIRE_QUIC); + /* make sure that ngh3_stream_recv is called again to complete the transfer + even if there are no more packets to be received from the server. */ + data->state.drain = 1; return 0; } -static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream_id, - const uint8_t *buf, size_t buflen, - void *user_data, void *stream_user_data) -{ - size_t ncopy; - struct Curl_easy *data = stream_user_data; - struct HTTP *stream = data->req.protop; - (void)conn; +/* Minimum size of the overflow buffer */ +#define OVERFLOWSIZE 1024 - /* TODO: this needs to be handled properly */ - if(buflen > stream->len) { - fprintf(stderr, "!! got %zd bytes, buffer has room for %zd bytes\n", - buflen, stream->len); - DEBUGASSERT(0); +/* + * allocate_overflow() ensures that there is room for incoming data in the + * overflow buffer, growing it to accommodate the new data if necessary. We + * may need to use the overflow buffer because we can't precisely limit the + * amount of HTTP/3 header data we receive using QUIC flow control mechanisms. + */ +static CURLcode allocate_overflow(struct Curl_easy *data, + struct HTTP *stream, + size_t length) +{ + size_t maxleft; + size_t newsize; + /* length can be arbitrarily large, so take care not to overflow newsize */ + maxleft = CURL_MAX_READ_SIZE - stream->overflow_buflen; + if(length > maxleft) { + /* The reason to have a max limit for this is to avoid the risk of a bad + server feeding libcurl with a highly compressed list of headers that + will cause our overflow buffer to grow too large */ + failf(data, "Rejected %zu bytes of overflow data (max is %d)!", + stream->overflow_buflen + length, CURL_MAX_READ_SIZE); + return CURLE_OUT_OF_MEMORY; } + newsize = stream->overflow_buflen + length; + if(newsize > stream->overflow_bufsize) { + /* We enlarge the overflow buffer as it is too small */ + char *newbuff; + newsize = CURLMAX(newsize * 3 / 2, stream->overflow_bufsize*2); + newsize = CURLMIN(CURLMAX(OVERFLOWSIZE, newsize), CURL_MAX_READ_SIZE); + newbuff = realloc(stream->overflow_buf, newsize); + if(!newbuff) { + failf(data, "Failed to alloc memory for overflow buffer!"); + return CURLE_OUT_OF_MEMORY; + } + stream->overflow_buf = newbuff; + stream->overflow_bufsize = newsize; + infof(data, "Grew HTTP/3 overflow buffer to %zu bytes\n", newsize); + } + return CURLE_OK; +} - ncopy = buflen; - memcpy(stream->mem, buf, ncopy); - stream->len -= ncopy; - stream->memlen += ncopy; +/* + * write_data() copies data to the stream's receive buffer. If not enough + * space is available in the receive buffer, it copies the rest to the + * stream's overflow buffer. + */ +static CURLcode write_data(struct Curl_easy *data, + struct HTTP *stream, + const void *mem, size_t memlen) +{ + CURLcode result = CURLE_OK; + const char *buf = mem; + size_t ncopy = memlen; + /* copy as much as possible to the receive buffer */ + if(stream->len) { + size_t len = CURLMIN(ncopy, stream->len); +#if 0 /* extra debugging of incoming h3 data */ + fprintf(stderr, "!! Copies %zd bytes to %p (total %zd)\n", + len, stream->mem, stream->memlen); +#endif + memcpy(stream->mem, buf, len); + stream->len -= len; + stream->memlen += len; + stream->mem += len; + buf += len; + ncopy -= len; + } + /* copy the rest to the overflow buffer */ + if(ncopy) { + result = allocate_overflow(data, stream, ncopy); + if(result) { + return result; + } +#if 0 /* extra debugging of incoming h3 data */ + fprintf(stderr, "!! Copies %zd overflow bytes to %p (total %zd)\n", + ncopy, stream->overflow_buf, stream->overflow_buflen); +#endif + memcpy(stream->overflow_buf + stream->overflow_buflen, buf, ncopy); + stream->overflow_buflen += ncopy; + } #if 0 /* extra debugging of incoming h3 data */ - fprintf(stderr, "!! Copies %zd bytes to %p (total %zd)\n", - ncopy, stream->mem, stream->memlen); { size_t i; - for(i = 0; i < ncopy; i++) { + for(i = 0; i < memlen; i++) { fprintf(stderr, "!! data[%d]: %02x '%c'\n", i, buf[i], buf[i]); } } #endif - stream->mem += ncopy; - stream->unacked_window += ncopy; + return result; +} + +static int cb_h3_recv_data(nghttp3_conn *conn, int64_t stream_id, + const uint8_t *buf, size_t buflen, + void *user_data, void *stream_user_data) +{ + struct Curl_easy *data = stream_user_data; + struct HTTP *stream = data->req.protop; + CURLcode result = CURLE_OK; + (void)conn; + + result = write_data(data, stream, buf, buflen); + if(result) { + return -1; + } + stream->unacked_window += buflen; (void)stream_id; (void)user_data; return 0; @@ -840,15 +918,17 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t stream_id, { struct Curl_easy *data = stream_user_data; struct HTTP *stream = data->req.protop; + CURLcode result = CURLE_OK; (void)conn; (void)stream_id; (void)user_data; - if(stream->memlen >= 2) { - memcpy(stream->mem, "\r\n", 2); - stream->len -= 2; - stream->memlen += 2; - stream->mem += 2; + /* add a CRLF only if we've received some headers */ + if(stream->firstheader) { + result = write_data(data, stream, "\r\n", 2); + if(result) { + return -1; + } } return 0; } @@ -862,7 +942,7 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id, nghttp3_vec h3val = nghttp3_rcbuf_get_buf(value); struct Curl_easy *data = stream_user_data; struct HTTP *stream = data->req.protop; - size_t ncopy; + CURLcode result = CURLE_OK; (void)conn; (void)stream_id; (void)token; @@ -871,20 +951,37 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id, if(h3name.len == sizeof(":status") - 1 && !memcmp(":status", h3name.base, h3name.len)) { + char line[14]; /* status line is always 13 characters long */ + size_t ncopy; int status = decode_status_code(h3val.base, h3val.len); DEBUGASSERT(status != -1); - msnprintf(stream->mem, stream->len, "HTTP/3 %03d \r\n", status); + ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n", status); + result = write_data(data, stream, line, ncopy); + if(result) { + return -1; + } } else { /* store as a HTTP1-style header */ - msnprintf(stream->mem, stream->len, "%.*s: %.*s\n", - h3name.len, h3name.base, h3val.len, h3val.base); + result = write_data(data, stream, h3name.base, h3name.len); + if(result) { + return -1; + } + result = write_data(data, stream, ": ", 2); + if(result) { + return -1; + } + result = write_data(data, stream, h3val.base, h3val.len); + if(result) { + return -1; + } + result = write_data(data, stream, "\r\n", 2); + if(result) { + return -1; + } } - ncopy = strlen(stream->mem); - stream->len -= ncopy; - stream->memlen += ncopy; - stream->mem += ncopy; + stream->firstheader = TRUE; return 0; } @@ -984,6 +1081,21 @@ static int init_ngh3_conn(struct quicsocket *qs) static Curl_recv ngh3_stream_recv; static Curl_send ngh3_stream_send; +static size_t drain_overflow_buffer(struct HTTP *stream) +{ + size_t ncopy = CURLMIN(stream->overflow_buflen, stream->len); + if(ncopy > 0) { + memcpy(stream->mem, stream->overflow_buf, ncopy); + stream->len -= ncopy; + stream->mem += ncopy; + stream->memlen += ncopy; + stream->overflow_buflen -= ncopy; + memmove(stream->overflow_buf, stream->overflow_buf + ncopy, + stream->overflow_buflen); + } + return ncopy; +} + /* incoming data frames on the h3 stream */ static ssize_t ngh3_stream_recv(struct connectdata *conn, int sockindex, @@ -1003,6 +1115,10 @@ static ssize_t ngh3_stream_recv(struct connectdata *conn, } /* else, there's data in the buffer already */ + /* if there's data in the overflow buffer from a previous call, copy as much + as possible to the receive buffer before receiving more */ + drain_overflow_buffer(stream); + if(ng_process_ingress(conn, sockfd, qs)) { *curlcode = CURLE_RECV_ERROR; return -1; @@ -1020,7 +1136,13 @@ static ssize_t ngh3_stream_recv(struct connectdata *conn, stream->memlen = 0; stream->mem = buf; stream->len = buffersize; + /* extend the stream window with the data we're consuming and send out + any additional packets to tell the server that we can receive more */ extend_stream_window(qs->qconn, stream); + if(ng_flush_egress(conn, sockfd, qs)) { + *curlcode = CURLE_SEND_ERROR; + return -1; + } return memlen; } @@ -1640,4 +1762,32 @@ CURLcode Curl_quic_done_sending(struct connectdata *conn) return CURLE_OK; } + +/* + * Called from http.c:Curl_http_done when a request completes. + */ +void Curl_quic_done(struct Curl_easy *data, bool premature) +{ + (void)premature; + if(data->conn->handler == &Curl_handler_http3) { + /* only for HTTP/3 transfers */ + struct HTTP *stream = data->req.protop; + Curl_safefree(stream->overflow_buf); + } +} + +/* + * Called from transfer.c:data_pending to know if we should keep looping + * to receive more data from the connection. + */ +bool Curl_quic_data_pending(const struct Curl_easy *data) +{ + /* We may have received more data than we're able to hold in the receive + buffer and allocated an overflow buffer. Since it's possible that + there's no more data coming on the socket, we need to keep reading + until the overflow buffer is empty. */ + const struct HTTP *stream = data->req.protop; + return stream->overflow_buflen > 0; +} + #endif diff --git a/lib/vquic/quiche.c b/lib/vquic/quiche.c index 6f9a72579..b2c98d255 100644 --- a/lib/vquic/quiche.c +++ b/lib/vquic/quiche.c @@ -785,4 +785,23 @@ CURLcode Curl_quic_done_sending(struct connectdata *conn) return CURLE_OK; } +/* + * Called from http.c:Curl_http_done when a request completes. + */ +void Curl_quic_done(struct Curl_easy *data, bool premature) +{ + (void)data; + (void)premature; +} + +/* + * Called from transfer.c:data_pending to know if we should keep looping + * to receive more data from the connection. + */ +bool Curl_quic_data_pending(const struct Curl_easy *data) +{ + (void)data; + return FALSE; +} + #endif -- cgit v1.2.3