[nginx] HTTP/2: always use temporary pool for processing headers.
Valentin Bartenev
vbart at nginx.com
Wed Feb 24 13:08:44 UTC 2016
details: http://hg.nginx.org/nginx/rev/8ec349bb60b2
branches:
changeset: 6411:8ec349bb60b2
user: Valentin Bartenev <vbart at nginx.com>
date: Wed Feb 24 16:05:47 2016 +0300
description:
HTTP/2: always use temporary pool for processing headers.
This is required for implementing per request timeouts.
Previously, the temporary pool was used only during skipping of
headers and the request pool was used otherwise. That required
switching of pools if the request was closed while parsing.
It wasn't a problem since the request could be closed only after
the validation of the fully parsed header. With the per request
timeouts, the request can be closed at any moment, and switching
of pools in the middle of parsing header name or value becomes a
problem.
To overcome this, the temporary pool is now always created and
used. Special checks are added to keep it when either the stream
is being processed or until header block is fully parsed.
diffstat:
src/http/v2/ngx_http_v2.c | 61 ++++++++++++++++++++++++++--------------------
src/http/v2/ngx_http_v2.h | 3 ++
2 files changed, 38 insertions(+), 26 deletions(-)
diffs (181 lines):
diff -r c6ccc1ea9450 -r 8ec349bb60b2 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c Wed Feb 24 16:05:46 2016 +0300
+++ b/src/http/v2/ngx_http_v2.c Wed Feb 24 16:05:47 2016 +0300
@@ -112,8 +112,6 @@ static u_char *ngx_http_v2_state_skip_pa
u_char *pos, u_char *end);
static u_char *ngx_http_v2_state_skip(ngx_http_v2_connection_t *h2c,
u_char *pos, u_char *end);
-static u_char *ngx_http_v2_state_skip_headers(ngx_http_v2_connection_t *h2c,
- u_char *pos, u_char *end);
static u_char *ngx_http_v2_state_save(ngx_http_v2_connection_t *h2c,
u_char *pos, u_char *end, ngx_http_v2_handler_pt handler);
static u_char *ngx_http_v2_connection_error(ngx_http_v2_connection_t *h2c,
@@ -1133,6 +1131,11 @@ ngx_http_v2_state_headers(ngx_http_v2_co
h2c->last_sid = h2c->state.sid;
+ h2c->state.pool = ngx_create_pool(1024, h2c->connection->log);
+ if (h2c->state.pool == NULL) {
+ return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
+ }
+
if (depend == h2c->state.sid) {
ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
"client sent HEADERS frame for stream %ui "
@@ -1146,7 +1149,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
NGX_HTTP_V2_INTERNAL_ERROR);
}
- return ngx_http_v2_state_skip_headers(h2c, pos, end);
+ return ngx_http_v2_state_header_block(h2c, pos, end);
}
h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
@@ -1166,7 +1169,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
NGX_HTTP_V2_INTERNAL_ERROR);
}
- return ngx_http_v2_state_skip_headers(h2c, pos, end);
+ return ngx_http_v2_state_header_block(h2c, pos, end);
}
node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 1);
@@ -1185,6 +1188,11 @@ ngx_http_v2_state_headers(ngx_http_v2_co
return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
}
+ h2c->state.stream = stream;
+
+ stream->pool = h2c->state.pool;
+ h2c->state.keep_pool = 1;
+
stream->request->request_length = h2c->state.length;
stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG;
@@ -1192,9 +1200,6 @@ ngx_http_v2_state_headers(ngx_http_v2_co
node->stream = stream;
- h2c->state.stream = stream;
- h2c->state.pool = stream->request->pool;
-
if (priority || node->parent == NULL) {
node->weight = weight;
ngx_http_v2_set_dependency(h2c, node, depend, excl);
@@ -1686,7 +1691,6 @@ ngx_http_v2_state_process_header(ngx_htt
error:
h2c->state.stream = NULL;
- h2c->state.pool = NULL;
return ngx_http_v2_state_header_complete(h2c, pos, end);
}
@@ -1699,8 +1703,7 @@ ngx_http_v2_state_header_complete(ngx_ht
ngx_http_v2_stream_t *stream;
if (h2c->state.length) {
- h2c->state.handler = h2c->state.pool ? ngx_http_v2_state_header_block
- : ngx_http_v2_state_skip_headers;
+ h2c->state.handler = ngx_http_v2_state_header_block;
return pos;
}
@@ -1713,12 +1716,14 @@ ngx_http_v2_state_header_complete(ngx_ht
if (stream) {
ngx_http_v2_run_request(stream->request);
-
- } else if (h2c->state.pool) {
+ }
+
+ if (!h2c->state.keep_pool) {
ngx_destroy_pool(h2c->state.pool);
}
h2c->state.pool = NULL;
+ h2c->state.keep_pool = 0;
if (h2c->state.padding) {
return ngx_http_v2_state_skip_padded(h2c, pos, end);
@@ -2337,19 +2342,6 @@ ngx_http_v2_state_skip(ngx_http_v2_conne
static u_char *
-ngx_http_v2_state_skip_headers(ngx_http_v2_connection_t *h2c, u_char *pos,
- u_char *end)
-{
- h2c->state.pool = ngx_create_pool(1024, h2c->connection->log);
- if (h2c->state.pool == NULL) {
- return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
- }
-
- return ngx_http_v2_state_header_block(h2c, pos, end);
-}
-
-
-static u_char *
ngx_http_v2_state_save(ngx_http_v2_connection_t *h2c, u_char *pos, u_char *end,
ngx_http_v2_handler_pt handler)
{
@@ -3633,6 +3625,7 @@ ngx_http_v2_terminate_stream(ngx_http_v2
void
ngx_http_v2_close_stream(ngx_http_v2_stream_t *stream, ngx_int_t rc)
{
+ ngx_pool_t *pool;
ngx_event_t *ev;
ngx_connection_t *fc;
ngx_http_v2_node_t *node;
@@ -3670,8 +3663,25 @@ ngx_http_v2_close_stream(ngx_http_v2_str
ngx_queue_insert_tail(&h2c->closed, &node->reuse);
h2c->closed_nodes++;
+ /*
+ * This pool keeps decoded request headers which can be used by log phase
+ * handlers in ngx_http_free_request().
+ *
+ * The pointer is stored into local variable because the stream object
+ * will be destroyed after a call to ngx_http_free_request().
+ */
+ pool = stream->pool;
+
ngx_http_free_request(stream->request, rc);
+ if (pool != h2c->state.pool) {
+ ngx_destroy_pool(pool);
+
+ } else {
+ /* pool will be destroyed when the complete header is parsed */
+ h2c->state.keep_pool = 0;
+ }
+
ev = fc->read;
if (ev->active || ev->disabled) {
@@ -3825,7 +3835,6 @@ ngx_http_v2_finalize_connection(ngx_http
if (h2c->state.stream) {
h2c->state.stream->out_closed = 1;
- h2c->state.pool = NULL;
ngx_http_v2_close_stream(h2c->state.stream, NGX_HTTP_BAD_REQUEST);
}
diff -r c6ccc1ea9450 -r 8ec349bb60b2 src/http/v2/ngx_http_v2.h
--- a/src/http/v2/ngx_http_v2.h Wed Feb 24 16:05:46 2016 +0300
+++ b/src/http/v2/ngx_http_v2.h Wed Feb 24 16:05:47 2016 +0300
@@ -73,6 +73,7 @@ typedef struct {
unsigned flags:8;
unsigned incomplete:1;
+ unsigned keep_pool:1;
/* HPACK */
unsigned parse_name:1;
@@ -186,6 +187,8 @@ struct ngx_http_v2_stream_s {
size_t header_limit;
+ ngx_pool_t *pool;
+
unsigned handled:1;
unsigned blocked:1;
unsigned exhausted:1;
More information about the nginx-devel
mailing list