Mercurial > hg > nginx

changeset 6954:052305810ca4

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
HTTP/2: fix flow control with padded DATA frames. Previously, flow control didn't account for padding in DATA frames, which meant that its view of the world could drift from peer's view by up to 256 bytes per received padded DATA frame, which could lead to a deadlock. Signed-off-by: Piotr Sikora <piotrsikora@google.com>
author Piotr Sikora <piotrsikora@google.com>
date Sun, 26 Mar 2017 01:25:04 -0700
parents 663e6a48bfcb
children d38161da62cd
files src/http/v2/ngx_http_v2.c
diffstat 1 files changed, 13 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -783,9 +783,12 @@ ngx_http_v2_state_head(ngx_http_v2_conne
 static u_char *
 ngx_http_v2_state_data(ngx_http_v2_connection_t *h2c, u_char *pos, u_char *end)
 {
+    size_t                 size;
     ngx_http_v2_node_t    *node;
     ngx_http_v2_stream_t  *stream;
 
+    size = h2c->state.length;
+
     if (h2c->state.flags & NGX_HTTP_V2_PADDED_FLAG) {
 
         if (h2c->state.length == 0) {
@@ -802,33 +805,32 @@ ngx_http_v2_state_data(ngx_http_v2_conne
         }
 
         h2c->state.padding = *pos++;
-        h2c->state.length--;
-
-        if (h2c->state.padding > h2c->state.length) {
+
+        if (h2c->state.padding >= size) {
             ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                           "client sent padded DATA frame "
                           "with incorrect length: %uz, padding: %uz",
-                          h2c->state.length, h2c->state.padding);
+                          size, h2c->state.padding);
 
             return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
         }
 
-        h2c->state.length -= h2c->state.padding;
+        h2c->state.length -= 1 + h2c->state.padding;
     }
 
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
                    "http2 DATA frame");
 
-    if (h2c->state.length > h2c->recv_window) {
+    if (size > h2c->recv_window) {
         ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                       "client violated connection flow control: "
                       "received DATA frame length %uz, available window %uz",
-                      h2c->state.length, h2c->recv_window);
+                      size, h2c->recv_window);
 
         return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_FLOW_CTRL_ERROR);
     }
 
-    h2c->recv_window -= h2c->state.length;
+    h2c->recv_window -= size;
 
     if (h2c->recv_window < NGX_HTTP_V2_MAX_WINDOW / 4) {
 
@@ -854,11 +856,11 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
     stream = node->stream;
 
-    if (h2c->state.length > stream->recv_window) {
+    if (size > stream->recv_window) {
         ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                       "client violated flow control for stream %ui: "
                       "received DATA frame length %uz, available window %uz",
-                      node->id, h2c->state.length, stream->recv_window);
+                      node->id, size, stream->recv_window);
 
         if (ngx_http_v2_terminate_stream(h2c, stream,
                                          NGX_HTTP_V2_FLOW_CTRL_ERROR)
@@ -871,7 +873,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
         return ngx_http_v2_state_skip_padded(h2c, pos, end);
     }
 
-    stream->recv_window -= h2c->state.length;
+    stream->recv_window -= size;
 
     if (stream->no_flow_control
         && stream->recv_window < NGX_HTTP_V2_MAX_WINDOW / 4)