Skip to content

Commit

Permalink
ngtcp2: fix stall or busy loop on STOP_SENDING with upload data
Browse files Browse the repository at this point in the history
Fixes #9122
Closes #9123
  • Loading branch information
tatsuhiro-t authored and bagder committed Jul 10, 2022
1 parent d123f0e commit 4989cd0
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 6 deletions.
5 changes: 3 additions & 2 deletions lib/http.h
Expand Up @@ -227,13 +227,11 @@ struct HTTP {
/*********** for HTTP/2 we store stream-local data here *************/
int32_t stream_id; /* stream we are interested in */

bool bodystarted;
/* We store non-final and final response headers here, per-stream */
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 */
bool close_handled; /* TRUE if stream closure is handled by libcurl */
Expand All @@ -244,6 +242,8 @@ struct HTTP {
uint32_t error; /* HTTP/2 stream error code */
#endif
#if defined(USE_NGHTTP2) || defined(USE_NGHTTP3)
bool bodystarted;
int status_code; /* HTTP status code */
bool closed; /* TRUE on HTTP2 stream close */
char *mem; /* points to a buffer in memory to store received data */
size_t len; /* size of the buffer 'mem' points to */
Expand All @@ -260,6 +260,7 @@ struct HTTP {
#ifndef USE_MSH3
/*********** for HTTP/3 we store stream-local data here *************/
int64_t stream3_id; /* stream we are interested in */
uint64_t error3; /* HTTP/3 stream error code */
bool firstheader; /* FALSE until headers arrive */
bool firstbody; /* FALSE until body arrives */
bool h3req; /* FALSE until request is issued */
Expand Down
19 changes: 18 additions & 1 deletion lib/transfer.c
Expand Up @@ -539,6 +539,13 @@ static CURLcode readwrite_data(struct Curl_easy *data,
bool is_http2 = ((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
(conn->httpversion == 20));
#endif
bool is_http3 =
#ifdef ENABLE_QUIC
((conn->handler->protocol & PROTO_FAMILY_HTTP) &&
(conn->httpversion == 30));
#else
FALSE;
#endif

if(
#ifdef USE_NGHTTP2
Expand All @@ -549,6 +556,7 @@ static CURLcode readwrite_data(struct Curl_easy *data,
for a particular stream. */
!is_http2 &&
#endif
!is_http3 && /* Same reason mentioned above. */
k->size != -1 && !k->header) {
/* make sure we don't read too much */
curl_off_t totalleft = k->size - k->bytecount;
Expand Down Expand Up @@ -596,6 +604,9 @@ static CURLcode readwrite_data(struct Curl_easy *data,
DEBUGF(infof(data, "nread == 0, stream closed, bailing"));
else
#endif
if(is_http3 && !nread)
DEBUGF(infof(data, "nread == 0, stream closed, bailing"));
else
DEBUGF(infof(data, "nread <= 0, server closed connection, bailing"));
k->keepon &= ~KEEP_RECV;
break;
Expand Down Expand Up @@ -753,7 +764,13 @@ static CURLcode readwrite_data(struct Curl_easy *data,
if(nread < 0) /* this should be unusual */
nread = 0;

k->keepon &= ~KEEP_RECV; /* we're done reading */
/* HTTP/3 over QUIC should keep reading until QUIC connection
is closed. In contrast to HTTP/2 which can stop reading
from TCP connection, HTTP/3 over QUIC needs ACK from server
to ensure stream closure. It should keep reading. */
if(!is_http3) {
k->keepon &= ~KEEP_RECV; /* we're done reading */
}
}

k->bytecount += nread;
Expand Down
35 changes: 32 additions & 3 deletions lib/vquic/ngtcp2.c
Expand Up @@ -899,6 +899,7 @@ static int cb_h3_stream_close(nghttp3_conn *conn, int64_t stream_id,
H3BUGF(infof(data, "cb_h3_stream_close CALLED"));

stream->closed = TRUE;
stream->error3 = app_error_code;
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. */
Expand Down Expand Up @@ -1010,6 +1011,10 @@ static int cb_h3_end_headers(nghttp3_conn *conn, int64_t stream_id,
return -1;
}
}

if(stream->status_code / 100 != 1) {
stream->bodystarted = TRUE;
}
return 0;
}

Expand All @@ -1032,9 +1037,10 @@ static int cb_h3_recv_header(nghttp3_conn *conn, int64_t stream_id,
if(token == NGHTTP3_QPACK_TOKEN__STATUS) {
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);
ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n", status);
stream->status_code = decode_status_code(h3val.base, h3val.len);
DEBUGASSERT(stream->status_code != -1);
ncopy = msnprintf(line, sizeof(line), "HTTP/3 %03d \r\n",
stream->status_code);
result = write_data(stream, line, ncopy);
if(result) {
return -1;
Expand Down Expand Up @@ -1226,6 +1232,24 @@ static ssize_t ngh3_stream_recv(struct Curl_easy *data,
}

if(stream->closed) {
if(stream->error3 != NGHTTP3_H3_NO_ERROR) {
failf(data,
"HTTP/3 stream %" PRId64 " was not closed cleanly: (err %" PRIu64
")",
stream->stream3_id, stream->error3);
*curlcode = CURLE_HTTP3;
return -1;
}

if(!stream->bodystarted) {
failf(data,
"HTTP/3 stream %" PRId64 " was closed cleanly, but before getting"
" all response header fields, treated as error",
stream->stream3_id);
*curlcode = CURLE_HTTP3;
return -1;
}

*curlcode = CURLE_OK;
return 0;
}
Expand Down Expand Up @@ -1444,6 +1468,11 @@ static ssize_t ngh3_stream_send(struct Curl_easy *data,
curl_socket_t sockfd = conn->sock[sockindex];
struct HTTP *stream = data->req.p.http;

if(stream->closed) {
*curlcode = CURLE_HTTP3;
return -1;
}

if(!stream->h3req) {
CURLcode result = http_request(data, mem, len);
if(result) {
Expand Down

0 comments on commit 4989cd0

Please sign in to comment.