Skip to content

Commit

Permalink
nginx: stop HTTP/3 steam read when skipping request data
Browse files Browse the repository at this point in the history
There's a few cases where nginx wants to skip over request payload data
but not immediately finalize the request. For HTTP/3, there is potential
for nginx to get into a tight loop when processing the client
connection. This is because quiche's `poll()` API always presents DATA
events for open streams wth buffered data. When the skip flag is set to
true, nginx does not drain quiche's buffer, resulting in repeated DATA
events for the same thing as long as the stream is active.

In most cases when the skip flag is set, the request is finalized
immediately. This change covers the remaining cases where the stream is
not finalized immediately. We now use `quiche_conn_stream_shutdown()` to
stop the read part of the stream, which effectively discards quiche's
buffering for the stream and prevents endless DATA events. Shutting only
the read side allows any pending response data to be sent but prevents
skippable request data from tying up the connection.

Signed-off-by: Diab Neiroukh <lazerl0rd@thezest.dev>
  • Loading branch information
LPardue authored and lzlrd committed Jan 24, 2021
1 parent cdd4b62 commit 551aeee
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/http/ngx_http_request_body.c
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,10 @@ ngx_http_discard_request_body(ngx_http_request_t *r)
#if (NGX_HTTP_V3)
if (r->qstream) {
r->qstream->skip_data = 1;

/* disable stream read to avoid pointless data events */
ngx_http_v3_stop_stream_read(r->qstream, 0);

return NGX_OK;
}
#endif
Expand Down
29 changes: 29 additions & 0 deletions src/http/v3/ngx_http_v3.c
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,9 @@ ngx_http_v3_read_request_body(ngx_http_request_t *r)

if (rb->buf == NULL) {
stream->skip_data = 1;

/* disable stream read to avoid pointless data events */
ngx_http_v3_stop_stream_read(stream, 0);
return NGX_HTTP_INTERNAL_SERVER_ERROR;
}

Expand Down Expand Up @@ -1575,6 +1578,9 @@ ngx_http_v3_read_unbuffered_request_body(ngx_http_request_t *r)
stream->skip_data = 1;
fc->timedout = 1;

/* disable stream read to avoid pointless data events */
ngx_http_v3_stop_stream_read(stream, 0);

return NGX_HTTP_REQUEST_TIME_OUT;
}

Expand All @@ -1587,6 +1593,10 @@ ngx_http_v3_read_unbuffered_request_body(ngx_http_request_t *r)

if (rc != NGX_OK) {
stream->skip_data = 1;

/* disable stream read to avoid pointless data events */
ngx_http_v3_stop_stream_read(stream, 0);

return rc;
}

Expand Down Expand Up @@ -2226,6 +2236,25 @@ ngx_http_v3_close_stream_handler(ngx_event_t *ev)
ngx_http_v3_close_stream(r->qstream, 0);
}

void
ngx_http_v3_stop_stream_read(ngx_http_v3_stream_t *stream, ngx_int_t rc)
{
ngx_http_v3_connection_t *h3c;

if (!stream) {
return;
}

h3c = stream->connection;

ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h3c->connection->log, 0,
"http3 stream shutdown read %ui", stream->id);

quiche_conn_stream_shutdown(h3c->connection->quic->conn,
stream->id,
QUICHE_SHUTDOWN_READ, rc);
}


static void
ngx_http_v3_finalize_connection(ngx_http_v3_connection_t *h3c,
Expand Down
1 change: 1 addition & 0 deletions src/http/v3/ngx_http_v3.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ ngx_int_t ngx_http_v3_read_unbuffered_request_body(ngx_http_request_t *r);
ngx_int_t ngx_http_v3_send_response(ngx_http_request_t *r);

void ngx_http_v3_close_stream(ngx_http_v3_stream_t *stream, ngx_int_t rc);
void ngx_http_v3_stop_stream_read(ngx_http_v3_stream_t *stream, ngx_int_t rc);


#endif /* _NGX_HTTP_V3_H_INCLUDED_ */
Empty file added timedout
Empty file.

0 comments on commit 551aeee

Please sign in to comment.