HTTP/2: implemented preread buffer for request body (closes #959).

Previously, the stream's window was kept zero in order to prevent a client
from sending the request body before it was requested (see 887cca40ba6a for
details).  Until such initial window was acknowledged all requests with
data were rejected (see 0aa07850922f for details).

That approach revealed a number of problems:

 1. Some clients (notably MS IE/Edge, Safari, iOS applications) show an error
    or even crash if a stream is rejected;

 2. This requires at least one RTT for every request with body before the
    client receives window update and able to send data.

To overcome these problems the new directive "http2_body_preread_size" is
introduced.  It sets the initial window and configures a special per stream
preread buffer that is used to save all incoming data before the body is
requested and processed.

If the directive's value is lower than the default initial window (65535),
as previously, all streams with data will be rejected until the new window
is acknowledged.  Otherwise, no special processing is used and all requests
with data are welcome right from the connection start.

The default value is chosen to be 64k, which is bigger than the default
initial window.  Setting it to zero is fully complaint to the previous
behavior.

diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c
index 278c9abf68fd1fb19e382852b40b0a4671ac19e0..cd0243cfce9ee30c363f9a01db1954990bc82986 100644 (file)
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -48,11 +48,6 @@
 
 #define NGX_HTTP_V2_DEFAULT_FRAME_SIZE           (1 << 14)
 
-#define NGX_HTTP_V2_MAX_WINDOW                   ((1U << 31) - 1)
-#define NGX_HTTP_V2_DEFAULT_WINDOW               65535
-
-#define NGX_HTTP_V2_INITIAL_WINDOW               0
-
 #define NGX_HTTP_V2_ROOT                         (void *) -1
 
 
@@ -879,8 +874,6 @@ ngx_http_v2_state_data(ngx_http_v2_connection_t *h2c, u_char *pos, u_char *end)
         return ngx_http_v2_state_skip_padded(h2c, pos, end);
     }
 
-    stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG;
-
     h2c->state.stream = stream;
 
     return ngx_http_v2_state_read_data(h2c, pos, end);
@@ -891,10 +884,12 @@ static u_char *
 ngx_http_v2_state_read_data(ngx_http_v2_connection_t *h2c, u_char *pos,
     u_char *end)
 {
-    size_t                 size;
-    ngx_int_t              rc;
-    ngx_uint_t             last;
-    ngx_http_v2_stream_t  *stream;
+    size_t                   size;
+    ngx_buf_t               *buf;
+    ngx_int_t                rc;
+    ngx_http_request_t      *r;
+    ngx_http_v2_stream_t    *stream;
+    ngx_http_v2_srv_conf_t  *h2scf;
 
     stream = h2c->state.stream;
 
@@ -913,17 +908,42 @@ ngx_http_v2_state_read_data(ngx_http_v2_connection_t *h2c, u_char *pos,
 
     if (size >= h2c->state.length) {
         size = h2c->state.length;
-        last = stream->in_closed;
-
-    } else {
-        last = 0;
+        stream->in_closed  = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG;
     }
 
-    rc = ngx_http_v2_process_request_body(stream->request, pos, size, last);
+    r = stream->request;
 
-    if (rc != NGX_OK) {
-        stream->skip_data = 1;
-        ngx_http_finalize_request(stream->request, rc);
+    if (r->request_body) {
+        rc = ngx_http_v2_process_request_body(r, pos, size, stream->in_closed);
+
+        if (rc != NGX_OK) {
+            stream->skip_data = 1;
+            ngx_http_finalize_request(r, rc);
+        }
+
+    } else if (size) {
+        buf = stream->preread;
+
+        if (buf == NULL) {
+            h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
+
+            buf = ngx_create_temp_buf(r->pool, h2scf->preread_size);
+            if (buf == NULL) {
+                return ngx_http_v2_connection_error(h2c,
+                                                    NGX_HTTP_V2_INTERNAL_ERROR);
+            }
+
+            stream->preread = buf;
+        }
+
+        if (size > (size_t) (buf->end - buf->last)) {
+            ngx_log_error(NGX_LOG_ALERT, h2c->connection->log, 0,
+                          "http2 preread buffer overflow");
+            return ngx_http_v2_connection_error(h2c,
+                                                NGX_HTTP_V2_INTERNAL_ERROR);
+        }
+
+        buf->last = ngx_cpymem(buf->last, pos, size);
     }
 
     pos += size;
@@ -1058,7 +1078,9 @@ ngx_http_v2_state_headers(ngx_http_v2_connection_t *h2c, u_char *pos,
         goto rst_stream;
     }
 
-    if (!h2c->settings_ack && !(h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG))
+    if (!h2c->settings_ack
+        && !(h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG)
+        && h2scf->preread_size < NGX_HTTP_V2_DEFAULT_WINDOW)
     {
         ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
                       "client sent stream with data "
@@ -2434,8 +2456,7 @@ ngx_http_v2_send_settings(ngx_http_v2_connection_t *h2c, ngx_uint_t ack)
 
         buf->last = ngx_http_v2_write_uint16(buf->last,
                                          NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING);
-        buf->last = ngx_http_v2_write_uint32(buf->last,
-                                             NGX_HTTP_V2_INITIAL_WINDOW);
+        buf->last = ngx_http_v2_write_uint32(buf->last, h2scf->preread_size);
 
         buf->last = ngx_http_v2_write_uint16(buf->last,
                                            NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING);
@@ -2643,6 +2664,7 @@ ngx_http_v2_create_stream(ngx_http_v2_connection_t *h2c)
     ngx_http_log_ctx_t        *ctx;
     ngx_http_request_t        *r;
     ngx_http_v2_stream_t      *stream;
+    ngx_http_v2_srv_conf_t    *h2scf;
     ngx_http_core_srv_conf_t  *cscf;
 
     fc = h2c->free_fake_connections;
@@ -2756,8 +2778,10 @@ ngx_http_v2_create_stream(ngx_http_v2_connection_t *h2c)
     stream->request = r;
     stream->connection = h2c;
 
+    h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
+
     stream->send_window = h2c->init_window;
-    stream->recv_window = NGX_HTTP_V2_INITIAL_WINDOW;
+    stream->recv_window = h2scf->preread_size;
 
     h2c->processing++;
 
@@ -3411,7 +3435,11 @@ ngx_http_v2_read_request_body(ngx_http_request_t *r,
     ngx_http_client_body_handler_pt post_handler)
 {
     off_t                      len;
+    size_t                     size;
+    ngx_buf_t                 *buf;
+    ngx_int_t                  rc;
     ngx_http_v2_stream_t      *stream;
+    ngx_http_v2_srv_conf_t    *h2scf;
     ngx_http_request_body_t   *rb;
     ngx_http_core_loc_conf_t  *clcf;
     ngx_http_v2_connection_t  *h2c;
@@ -3444,24 +3472,34 @@ ngx_http_v2_read_request_body(ngx_http_request_t *r,
 
     r->request_body = rb;
 
+    h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
     clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
 
     len = r->headers_in.content_length_n;
 
     if (r->request_body_no_buffering && !stream->in_closed) {
-        r->request_body_in_file_only = 0;
 
         if (len < 0 || len > (off_t) clcf->client_body_buffer_size) {
             len = clcf->client_body_buffer_size;
         }
 
+        /*
+         * We need a room to store data up to the stream's initial window size,
+         * at least until this window will be exhausted.
+         */
+
+        if (len < (off_t) h2scf->preread_size) {
+            len = h2scf->preread_size;
+        }
+
         if (len > NGX_HTTP_V2_MAX_WINDOW) {
             len = NGX_HTTP_V2_MAX_WINDOW;
         }
-    }
 
-    if (len >= 0 && len <= (off_t) clcf->client_body_buffer_size
-        && !r->request_body_in_file_only)
+        rb->buf = ngx_create_temp_buf(r->pool, (size_t) len);
+
+    } else if (len >= 0 && len <= (off_t) clcf->client_body_buffer_size
+               && !r->request_body_in_file_only)
     {
         rb->buf = ngx_create_temp_buf(r->pool, (size_t) len);
 
@@ -3478,22 +3516,44 @@ ngx_http_v2_read_request_body(ngx_http_request_t *r,
         return NGX_HTTP_INTERNAL_SERVER_ERROR;
     }
 
+    buf = stream->preread;
+
     if (stream->in_closed) {
         r->request_body_no_buffering = 0;
+
+        if (buf) {
+            rc = ngx_http_v2_process_request_body(r, buf->pos,
+                                                  buf->last - buf->pos, 1);
+            ngx_pfree(r->pool, buf->start);
+            return rc;
+        }
+
         return ngx_http_v2_process_request_body(r, NULL, 0, 1);
     }
 
-    if (len) {
-        if (r->request_body_no_buffering) {
-            stream->recv_window = (size_t) len;
+    if (buf) {
+        rc = ngx_http_v2_process_request_body(r, buf->pos,
+                                              buf->last - buf->pos, 0);
 
-        } else {
-            stream->no_flow_control = 1;
-            stream->recv_window = NGX_HTTP_V2_MAX_WINDOW;
+        ngx_pfree(r->pool, buf->start);
+
+        if (rc != NGX_OK) {
+            stream->skip_data = 1;
+            return rc;
         }
+    }
 
-        if (ngx_http_v2_send_window_update(stream->connection, stream->node->id,
-                                           stream->recv_window)
+    if (r->request_body_no_buffering) {
+        size = len - h2scf->preread_size;
+
+    } else {
+        stream->no_flow_control = 1;
+        size = NGX_HTTP_V2_MAX_WINDOW - stream->recv_window;
+    }
+
+    if (size) {
+        if (ngx_http_v2_send_window_update(stream->connection,
+                                           stream->node->id, size)
             == NGX_ERROR)
         {
             stream->skip_data = 1;
@@ -3508,9 +3568,13 @@ ngx_http_v2_read_request_body(ngx_http_request_t *r,
                 return NGX_HTTP_INTERNAL_SERVER_ERROR;
             }
         }
+
+        stream->recv_window += size;
     }
 
-    ngx_add_timer(r->connection->read, clcf->client_body_timeout);
+    if (!buf) {
+        ngx_add_timer(r->connection->read, clcf->client_body_timeout);
+    }
 
     r->read_event_handler = ngx_http_v2_read_client_request_body_handler;
     r->write_event_handler = ngx_http_request_empty_handler;
@@ -3529,13 +3593,8 @@ ngx_http_v2_process_request_body(ngx_http_request_t *r, u_char *pos,
     ngx_http_request_body_t   *rb;
     ngx_http_core_loc_conf_t  *clcf;
 
-    rb = r->request_body;
-
-    if (rb == NULL) {
-        return NGX_OK;
-    }
-
     fc = r->connection;
+    rb = r->request_body;
     buf = rb->buf;
 
     if (size) {
@@ -3789,7 +3848,14 @@ ngx_http_v2_read_unbuffered_request_body(ngx_http_request_t *r)
         window -= h2c->state.length;
     }
 
-    if (window == stream->recv_window) {
+    if (window <= stream->recv_window) {
+        if (window < stream->recv_window) {
+            ngx_log_error(NGX_LOG_ALERT, r->connection->log, 0,
+                          "http2 negative window update");
+            stream->skip_data = 1;
+            return NGX_HTTP_INTERNAL_SERVER_ERROR;
+        }
+
         return NGX_AGAIN;
     }
 

diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
index 1adf8deb92f3c0d310b1e1b6a08a033bc12fce96..9e738aa55749f8b6a73ce1b73fa28a38384232e5 100644 (file)
--- a/src/http/v2/ngx_http_v2.h
+++ b/src/http/v2/ngx_http_v2.h
@@ -46,6 +46,9 @@
 #define NGX_HTTP_V2_PADDED_FLAG          0x08
 #define NGX_HTTP_V2_PRIORITY_FLAG        0x20
 
+#define NGX_HTTP_V2_MAX_WINDOW           ((1U << 31) - 1)
+#define NGX_HTTP_V2_DEFAULT_WINDOW       65535
+
 
 typedef struct ngx_http_v2_connection_s   ngx_http_v2_connection_t;
 typedef struct ngx_http_v2_node_s         ngx_http_v2_node_t;
@@ -174,6 +177,8 @@ struct ngx_http_v2_stream_s {
     ssize_t                          send_window;
     size_t                           recv_window;
 
+    ngx_buf_t                       *preread;
+
     ngx_http_v2_out_frame_t         *free_frames;
     ngx_chain_t                     *free_frame_headers;
     ngx_chain_t                     *free_bufs;

diff --git a/src/http/v2/ngx_http_v2_module.c b/src/http/v2/ngx_http_v2_module.c
index 5a4561c2b0d09cf04aab5e0b1ad426428026f7aa..b7d99e055229de25c4a4c783e462ccde87d5e340 100644 (file)
--- a/src/http/v2/ngx_http_v2_module.c
+++ b/src/http/v2/ngx_http_v2_module.c
@@ -30,6 +30,7 @@ static char *ngx_http_v2_merge_loc_conf(ngx_conf_t *cf, void *parent,
 static char *ngx_http_v2_recv_buffer_size(ngx_conf_t *cf, void *post,
     void *data);
 static char *ngx_http_v2_pool_size(ngx_conf_t *cf, void *post, void *data);
+static char *ngx_http_v2_preread_size(ngx_conf_t *cf, void *post, void *data);
 static char *ngx_http_v2_streams_index_mask(ngx_conf_t *cf, void *post,
     void *data);
 static char *ngx_http_v2_chunk_size(ngx_conf_t *cf, void *post, void *data);
@@ -41,6 +42,8 @@ static ngx_conf_post_t  ngx_http_v2_recv_buffer_size_post =
     { ngx_http_v2_recv_buffer_size };
 static ngx_conf_post_t  ngx_http_v2_pool_size_post =
     { ngx_http_v2_pool_size };
+static ngx_conf_post_t  ngx_http_v2_preread_size_post =
+    { ngx_http_v2_preread_size };
 static ngx_conf_post_t  ngx_http_v2_streams_index_mask_post =
     { ngx_http_v2_streams_index_mask };
 static ngx_conf_post_t  ngx_http_v2_chunk_size_post =
@@ -84,6 +87,13 @@ static ngx_command_t  ngx_http_v2_commands[] = {
       offsetof(ngx_http_v2_srv_conf_t, max_header_size),
       NULL },
 
+    { ngx_string("http2_body_preread_size"),
+      NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
+      ngx_conf_set_size_slot,
+      NGX_HTTP_SRV_CONF_OFFSET,
+      offsetof(ngx_http_v2_srv_conf_t, preread_size),
+      &ngx_http_v2_preread_size_post },
+
     { ngx_string("http2_streams_index_size"),
       NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_TAKE1,
       ngx_conf_set_num_slot,
@@ -316,6 +326,8 @@ ngx_http_v2_create_srv_conf(ngx_conf_t *cf)
     h2scf->max_field_size = NGX_CONF_UNSET_SIZE;
     h2scf->max_header_size = NGX_CONF_UNSET_SIZE;
 
+    h2scf->preread_size = NGX_CONF_UNSET_SIZE;
+
     h2scf->streams_index_mask = NGX_CONF_UNSET_UINT;
 
     h2scf->recv_timeout = NGX_CONF_UNSET_MSEC;
@@ -341,6 +353,8 @@ ngx_http_v2_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child)
     ngx_conf_merge_size_value(conf->max_header_size, prev->max_header_size,
                               16384);
 
+    ngx_conf_merge_size_value(conf->preread_size, prev->preread_size, 65536);
+
     ngx_conf_merge_uint_value(conf->streams_index_mask,
                               prev->streams_index_mask, 32 - 1);
 
@@ -419,6 +433,23 @@ ngx_http_v2_pool_size(ngx_conf_t *cf, void *post, void *data)
 }
 
 
+static char *
+ngx_http_v2_preread_size(ngx_conf_t *cf, void *post, void *data)
+{
+    size_t *sp = data;
+
+    if (*sp > NGX_HTTP_V2_MAX_WINDOW) {
+        ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+                           "the maximum body preread buffer size is %uz",
+                           NGX_HTTP_V2_MAX_WINDOW);
+
+        return NGX_CONF_ERROR;
+    }
+
+    return NGX_CONF_OK;
+}
+
+
 static char *
 ngx_http_v2_streams_index_mask(ngx_conf_t *cf, void *post, void *data)
 {

diff --git a/src/http/v2/ngx_http_v2_module.h b/src/http/v2/ngx_http_v2_module.h
index 95cc7d86fcee57dcb854626f0e781621e75795b8..91f97c2532d073e70b813ea28820bdd61aa6eada 100644 (file)
--- a/src/http/v2/ngx_http_v2_module.h
+++ b/src/http/v2/ngx_http_v2_module.h
@@ -25,6 +25,7 @@ typedef struct {
     ngx_uint_t                      concurrent_streams;
     size_t                          max_field_size;
     size_t                          max_header_size;
+    size_t                          preread_size;
     ngx_uint_t                      streams_index_mask;
     ngx_msec_t                      recv_timeout;
     ngx_msec_t                      idle_timeout;