[PATCH] HTTP/2: fix flow control with padded DATA frames

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516704 25200
#      Sun Mar 26 01:25:04 2017 -0700
# Node ID 899a53d2789b8c6bafdd5e40d78b4e92dd32dd10
# Parent  22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
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 <piotrsik...@google.com>

diff -r 22be63bf21ed -r 899a53d2789b src/http/v2/ngx_http_v2.c
--- 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)
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel