Klaus Demo nginx / 4e6a490
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. Valentin Bartenev 6 years ago
2 changed file(s) with 38 addition(s) and 26 deletion(s). Raw diff Collapse all Expand all
+35
-26
src/http/v2/ngx_http_v2.c less more
111111 u_char *pos, u_char *end);
112112 static u_char *ngx_http_v2_state_skip(ngx_http_v2_connection_t *h2c,
113113 u_char *pos, u_char *end);
114 static u_char *ngx_http_v2_state_skip_headers(ngx_http_v2_connection_t *h2c,
115 u_char *pos, u_char *end);
116114 static u_char *ngx_http_v2_state_save(ngx_http_v2_connection_t *h2c,
117115 u_char *pos, u_char *end, ngx_http_v2_handler_pt handler);
118116 static u_char *ngx_http_v2_connection_error(ngx_http_v2_connection_t *h2c,
11321130
11331131 h2c->last_sid = h2c->state.sid;
11341132
1133 h2c->state.pool = ngx_create_pool(1024, h2c->connection->log);
1134 if (h2c->state.pool == NULL) {
1135 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
1136 }
1137
11351138 if (depend == h2c->state.sid) {
11361139 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
11371140 "client sent HEADERS frame for stream %ui "
11451148 NGX_HTTP_V2_INTERNAL_ERROR);
11461149 }
11471150
1148 return ngx_http_v2_state_skip_headers(h2c, pos, end);
1151 return ngx_http_v2_state_header_block(h2c, pos, end);
11491152 }
11501153
11511154 h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
11651168 NGX_HTTP_V2_INTERNAL_ERROR);
11661169 }
11671170
1168 return ngx_http_v2_state_skip_headers(h2c, pos, end);
1171 return ngx_http_v2_state_header_block(h2c, pos, end);
11691172 }
11701173
11711174 node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 1);
11841187 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
11851188 }
11861189
1190 h2c->state.stream = stream;
1191
1192 stream->pool = h2c->state.pool;
1193 h2c->state.keep_pool = 1;
1194
11871195 stream->request->request_length = h2c->state.length;
11881196
11891197 stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG;
11901198 stream->node = node;
11911199
11921200 node->stream = stream;
1193
1194 h2c->state.stream = stream;
1195 h2c->state.pool = stream->request->pool;
11961201
11971202 if (priority || node->parent == NULL) {
11981203 node->weight = weight;
16851690 error:
16861691
16871692 h2c->state.stream = NULL;
1688 h2c->state.pool = NULL;
16891693
16901694 return ngx_http_v2_state_header_complete(h2c, pos, end);
16911695 }
16981702 ngx_http_v2_stream_t *stream;
16991703
17001704 if (h2c->state.length) {
1701 h2c->state.handler = h2c->state.pool ? ngx_http_v2_state_header_block
1702 : ngx_http_v2_state_skip_headers;
1705 h2c->state.handler = ngx_http_v2_state_header_block;
17031706 return pos;
17041707 }
17051708
17121715
17131716 if (stream) {
17141717 ngx_http_v2_run_request(stream->request);
1715
1716 } else if (h2c->state.pool) {
1718 }
1719
1720 if (!h2c->state.keep_pool) {
17171721 ngx_destroy_pool(h2c->state.pool);
17181722 }
17191723
17201724 h2c->state.pool = NULL;
1725 h2c->state.keep_pool = 0;
17211726
17221727 if (h2c->state.padding) {
17231728 return ngx_http_v2_state_skip_padded(h2c, pos, end);
23322337 "http2 frame skip %uz", h2c->state.length);
23332338
23342339 return ngx_http_v2_state_complete(h2c, pos + h2c->state.length, end);
2335 }
2336
2337
2338 static u_char *
2339 ngx_http_v2_state_skip_headers(ngx_http_v2_connection_t *h2c, u_char *pos,
2340 u_char *end)
2341 {
2342 h2c->state.pool = ngx_create_pool(1024, h2c->connection->log);
2343 if (h2c->state.pool == NULL) {
2344 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
2345 }
2346
2347 return ngx_http_v2_state_header_block(h2c, pos, end);
23482340 }
23492341
23502342
36323624 void
36333625 ngx_http_v2_close_stream(ngx_http_v2_stream_t *stream, ngx_int_t rc)
36343626 {
3627 ngx_pool_t *pool;
36353628 ngx_event_t *ev;
36363629 ngx_connection_t *fc;
36373630 ngx_http_v2_node_t *node;
36693662 ngx_queue_insert_tail(&h2c->closed, &node->reuse);
36703663 h2c->closed_nodes++;
36713664
3665 /*
3666 * This pool keeps decoded request headers which can be used by log phase
3667 * handlers in ngx_http_free_request().
3668 *
3669 * The pointer is stored into local variable because the stream object
3670 * will be destroyed after a call to ngx_http_free_request().
3671 */
3672 pool = stream->pool;
3673
36723674 ngx_http_free_request(stream->request, rc);
3675
3676 if (pool != h2c->state.pool) {
3677 ngx_destroy_pool(pool);
3678
3679 } else {
3680 /* pool will be destroyed when the complete header is parsed */
3681 h2c->state.keep_pool = 0;
3682 }
36733683
36743684 ev = fc->read;
36753685
38243834
38253835 if (h2c->state.stream) {
38263836 h2c->state.stream->out_closed = 1;
3827 h2c->state.pool = NULL;
38283837 ngx_http_v2_close_stream(h2c->state.stream, NGX_HTTP_BAD_REQUEST);
38293838 }
38303839
+3
-0
src/http/v2/ngx_http_v2.h less more
7272 unsigned flags:8;
7373
7474 unsigned incomplete:1;
75 unsigned keep_pool:1;
7576
7677 /* HPACK */
7778 unsigned parse_name:1;
185186
186187 size_t header_limit;
187188
189 ngx_pool_t *pool;
190
188191 unsigned handled:1;
189192 unsigned blocked:1;
190193 unsigned exhausted:1;