[nginx] HTTP/2: fixed splitting of response headers on CONTINUAT...

Valentin Bartenev vbart at nginx.com
Mon Oct 26 14:46:04 UTC 2015


details:   http://hg.nginx.org/nginx/rev/b930e598a199
branches:  
changeset: 6277:b930e598a199
user:      Valentin Bartenev <vbart at nginx.com>
date:      Mon Sep 28 02:32:44 2015 +0300
description:
HTTP/2: fixed splitting of response headers on CONTINUATION frames.

Previous code has been based on assumption that the header block can only be
splitted at the borders of individual headers.  That wasn't the case and might
result in emitting frames bigger than the frame size limit.

The current approach is to split header blocks by the frame size limit.

diffstat:

 src/http/v2/ngx_http_v2_filter_module.c |  341 +++++++++++++++----------------
 1 files changed, 163 insertions(+), 178 deletions(-)

diffs (truncated from 470 to 300 lines):

diff -r 0efc16d55adb -r b930e598a199 src/http/v2/ngx_http_v2_filter_module.c
--- a/src/http/v2/ngx_http_v2_filter_module.c	Mon Oct 26 17:43:08 2015 +0300
+++ b/src/http/v2/ngx_http_v2_filter_module.c	Mon Sep 28 02:32:44 2015 +0300
@@ -43,10 +43,8 @@
 
 static u_char *ngx_http_v2_write_int(u_char *pos, ngx_uint_t prefix,
     ngx_uint_t value);
-static void ngx_http_v2_write_headers_head(u_char *pos, size_t length,
-    ngx_uint_t sid, ngx_uint_t end_headers, ngx_uint_t end_stream);
-static void ngx_http_v2_write_continuation_head(u_char *pos, size_t length,
-    ngx_uint_t sid, ngx_uint_t end_headers);
+static ngx_http_v2_out_frame_t *ngx_http_v2_create_headers_frame(
+    ngx_http_request_t *r, u_char *pos, u_char *end);
 
 static ngx_chain_t *ngx_http_v2_send_chain(ngx_connection_t *fc,
     ngx_chain_t *in, off_t limit);
@@ -116,17 +114,14 @@ static ngx_http_output_header_filter_pt 
 static ngx_int_t
 ngx_http_v2_header_filter(ngx_http_request_t *r)
 {
-    u_char                     status, *p, *head;
-    size_t                     len, rest, frame_size;
-    ngx_buf_t                 *b;
+    u_char                     status, *pos, *start, *p;
+    size_t                     len;
     ngx_str_t                  host, location;
-    ngx_uint_t                 i, port, continuation;
-    ngx_chain_t               *cl;
+    ngx_uint_t                 i, port;
     ngx_list_part_t           *part;
     ngx_table_elt_t           *header;
     ngx_connection_t          *fc;
     ngx_http_cleanup_t        *cln;
-    ngx_http_v2_stream_t      *stream;
     ngx_http_v2_out_frame_t   *frame;
     ngx_http_core_loc_conf_t  *clcf;
     ngx_http_core_srv_conf_t  *cscf;
@@ -388,134 +383,117 @@ ngx_http_v2_header_filter(ngx_http_reque
                  + NGX_HTTP_V2_INT_OCTETS + header[i].value.len;
     }
 
-    stream = r->stream;
-    frame_size = stream->connection->frame_size;
-
-    len += NGX_HTTP_V2_FRAME_HEADER_SIZE
-           * ((len + frame_size - 1) / frame_size);
-
-    b = ngx_create_temp_buf(r->pool, len);
-    if (b == NULL) {
+    pos = ngx_palloc(r->pool, len);
+    if (pos == NULL) {
         return NGX_ERROR;
     }
 
-    b->last_buf = r->header_only;
-
-    b->last += NGX_HTTP_V2_FRAME_HEADER_SIZE;
+    start = pos;
 
     if (status) {
-        *b->last++ = status;
+        *pos++ = status;
 
     } else {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_STATUS_INDEX);
-        *b->last++ = NGX_HTTP_V2_ENCODE_RAW | 3;
-        b->last = ngx_sprintf(b->last, "%03ui", r->headers_out.status);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_STATUS_INDEX);
+        *pos++ = NGX_HTTP_V2_ENCODE_RAW | 3;
+        pos = ngx_sprintf(pos, "%03ui", r->headers_out.status);
     }
 
     if (r->headers_out.server == NULL) {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_SERVER_INDEX);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_SERVER_INDEX);
 
         if (clcf->server_tokens) {
-            *b->last++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof(NGINX_VER) - 1);
-            b->last = ngx_cpymem(b->last, NGINX_VER, sizeof(NGINX_VER) - 1);
+            *pos++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof(NGINX_VER) - 1);
+            pos = ngx_cpymem(pos, NGINX_VER, sizeof(NGINX_VER) - 1);
 
         } else {
-            *b->last++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof("nginx") - 1);
-            b->last = ngx_cpymem(b->last, "nginx", sizeof("nginx") - 1);
+            *pos++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof("nginx") - 1);
+            pos = ngx_cpymem(pos, "nginx", sizeof("nginx") - 1);
         }
     }
 
     if (r->headers_out.date == NULL) {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_DATE_INDEX);
-        *b->last++ = (u_char) ngx_cached_http_time.len;
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_DATE_INDEX);
+        *pos++ = (u_char) ngx_cached_http_time.len;
 
-        b->last = ngx_cpymem(b->last, ngx_cached_http_time.data,
-                             ngx_cached_http_time.len);
+        pos = ngx_cpymem(pos, ngx_cached_http_time.data,
+                         ngx_cached_http_time.len);
     }
 
     if (r->headers_out.content_type.len) {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_CONTENT_TYPE_INDEX);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_CONTENT_TYPE_INDEX);
 
         if (r->headers_out.content_type_len == r->headers_out.content_type.len
             && r->headers_out.charset.len)
         {
-            *b->last = NGX_HTTP_V2_ENCODE_RAW;
-            b->last = ngx_http_v2_write_int(b->last, ngx_http_v2_prefix(7),
-                                            r->headers_out.content_type.len
-                                            + sizeof("; charset=") - 1
-                                            + r->headers_out.charset.len);
+            *pos = NGX_HTTP_V2_ENCODE_RAW;
+            pos = ngx_http_v2_write_int(pos, ngx_http_v2_prefix(7),
+                                        r->headers_out.content_type.len
+                                        + sizeof("; charset=") - 1
+                                        + r->headers_out.charset.len);
 
-            p = b->last;
+            p = pos;
 
-            b->last = ngx_cpymem(p, r->headers_out.content_type.data,
-                                 r->headers_out.content_type.len);
+            pos = ngx_cpymem(pos, r->headers_out.content_type.data,
+                             r->headers_out.content_type.len);
 
-            b->last = ngx_cpymem(b->last, "; charset=",
-                                 sizeof("; charset=") - 1);
+            pos = ngx_cpymem(pos, "; charset=", sizeof("; charset=") - 1);
 
-            b->last = ngx_cpymem(b->last, r->headers_out.charset.data,
-                                 r->headers_out.charset.len);
+            pos = ngx_cpymem(pos, r->headers_out.charset.data,
+                             r->headers_out.charset.len);
 
             /* update r->headers_out.content_type for possible logging */
 
-            r->headers_out.content_type.len = b->last - p;
+            r->headers_out.content_type.len = pos - p;
             r->headers_out.content_type.data = p;
 
         } else {
-            *b->last = NGX_HTTP_V2_ENCODE_RAW;
-            b->last = ngx_http_v2_write_int(b->last, ngx_http_v2_prefix(7),
-                                            r->headers_out.content_type.len);
-            b->last = ngx_cpymem(b->last, r->headers_out.content_type.data,
-                                 r->headers_out.content_type.len);
+            *pos = NGX_HTTP_V2_ENCODE_RAW;
+            pos = ngx_http_v2_write_int(pos, ngx_http_v2_prefix(7),
+                                        r->headers_out.content_type.len);
+            pos = ngx_cpymem(pos, r->headers_out.content_type.data,
+                             r->headers_out.content_type.len);
         }
     }
 
     if (r->headers_out.content_length == NULL
         && r->headers_out.content_length_n >= 0)
     {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_CONTENT_LENGTH_INDEX);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_CONTENT_LENGTH_INDEX);
 
-        p = b->last;
-        b->last = ngx_sprintf(b->last + 1, "%O",
-                              r->headers_out.content_length_n);
-        *p = NGX_HTTP_V2_ENCODE_RAW | (u_char) (b->last - p - 1);
+        p = pos;
+        pos = ngx_sprintf(pos + 1, "%O", r->headers_out.content_length_n);
+        *p = NGX_HTTP_V2_ENCODE_RAW | (u_char) (pos - p - 1);
     }
 
     if (r->headers_out.last_modified == NULL
         && r->headers_out.last_modified_time != -1)
     {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_LAST_MODIFIED_INDEX);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_LAST_MODIFIED_INDEX);
 
-        *b->last++ = NGX_HTTP_V2_ENCODE_RAW
-                     | (sizeof("Wed, 31 Dec 1986 18:00:00 GMT") - 1);
-        b->last = ngx_http_time(b->last, r->headers_out.last_modified_time);
+        *pos++ = NGX_HTTP_V2_ENCODE_RAW
+                 | (sizeof("Wed, 31 Dec 1986 18:00:00 GMT") - 1);
+        pos = ngx_http_time(pos, r->headers_out.last_modified_time);
     }
 
     if (r->headers_out.location && r->headers_out.location->value.len) {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_LOCATION_INDEX);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_LOCATION_INDEX);
 
-        *b->last = NGX_HTTP_V2_ENCODE_RAW;
-        b->last = ngx_http_v2_write_int(b->last, ngx_http_v2_prefix(7),
-                                        r->headers_out.location->value.len);
-        b->last = ngx_cpymem(b->last, r->headers_out.location->value.data,
-                                      r->headers_out.location->value.len);
+        *pos = NGX_HTTP_V2_ENCODE_RAW;
+        pos = ngx_http_v2_write_int(pos, ngx_http_v2_prefix(7),
+                                    r->headers_out.location->value.len);
+        pos = ngx_cpymem(pos, r->headers_out.location->value.data,
+                              r->headers_out.location->value.len);
     }
 
 #if (NGX_HTTP_GZIP)
     if (r->gzip_vary) {
-        *b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_VARY_INDEX);
-        *b->last++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof("Accept-Encoding") - 1);
-        b->last = ngx_cpymem(b->last, "Accept-Encoding",
-                             sizeof("Accept-Encoding") - 1);
+        *pos++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_VARY_INDEX);
+        *pos++ = NGX_HTTP_V2_ENCODE_RAW | (sizeof("Accept-Encoding") - 1);
+        pos = ngx_cpymem(pos, "Accept-Encoding", sizeof("Accept-Encoding") - 1);
     }
 #endif
 
-    continuation = 0;
-    head = b->pos;
-
-    len = b->last - head - NGX_HTTP_V2_FRAME_HEADER_SIZE;
-    rest = frame_size - len;
-
     part = &r->headers_out.headers.part;
     header = part->elts;
 
@@ -535,82 +513,26 @@ ngx_http_v2_header_filter(ngx_http_reque
             continue;
         }
 
-        len = 1 + NGX_HTTP_V2_INT_OCTETS * 2
-              + header[i].key.len
-              + header[i].value.len;
+        *pos++ = 0;
 
-        if (len > rest) {
-            len = b->last - head - NGX_HTTP_V2_FRAME_HEADER_SIZE;
+        *pos = NGX_HTTP_V2_ENCODE_RAW;
+        pos = ngx_http_v2_write_int(pos, ngx_http_v2_prefix(7),
+                                    header[i].key.len);
+        ngx_strlow(pos, header[i].key.data, header[i].key.len);
+        pos += header[i].key.len;
 
-            if (continuation) {
-                ngx_http_v2_write_continuation_head(head, len,
-                                                    stream->node->id, 0);
-            } else {
-                continuation = 1;
-                ngx_http_v2_write_headers_head(head, len, stream->node->id, 0,
-                                               r->header_only);
-            }
-
-            rest = frame_size;
-            head = b->last;
-
-            b->last += NGX_HTTP_V2_FRAME_HEADER_SIZE;
-        }
-
-        p = b->last;
-
-        *p++ = 0;
-
-        *p = NGX_HTTP_V2_ENCODE_RAW;
-        p = ngx_http_v2_write_int(p, ngx_http_v2_prefix(7), header[i].key.len);
-        ngx_strlow(p, header[i].key.data, header[i].key.len);
-        p += header[i].key.len;
-
-        *p = NGX_HTTP_V2_ENCODE_RAW;
-        p = ngx_http_v2_write_int(p, ngx_http_v2_prefix(7),
-                                  header[i].value.len);
-        p = ngx_cpymem(p, header[i].value.data, header[i].value.len);
-
-        rest -= p - b->last;
-        b->last = p;
+        *pos = NGX_HTTP_V2_ENCODE_RAW;
+        pos = ngx_http_v2_write_int(pos, ngx_http_v2_prefix(7),
+                                    header[i].value.len);
+        pos = ngx_cpymem(pos, header[i].value.data, header[i].value.len);
     }
 
-    len = b->last - head - NGX_HTTP_V2_FRAME_HEADER_SIZE;
-
-    if (continuation) {
-        ngx_http_v2_write_continuation_head(head, len, stream->node->id, 1);
-
-    } else {
-        ngx_http_v2_write_headers_head(head, len, stream->node->id, 1,
-                                       r->header_only);
-    }
-
-    cl = ngx_alloc_chain_link(r->pool);
-    if (cl == NULL) {
-        return NGX_ERROR;
-    }
-
-    cl->buf = b;
-    cl->next = NULL;
-
-    frame = ngx_palloc(r->pool, sizeof(ngx_http_v2_out_frame_t));
+    frame = ngx_http_v2_create_headers_frame(r, start, pos);
     if (frame == NULL) {
         return NGX_ERROR;
     }



More information about the nginx-devel mailing list