Hi,

I noticed that Serf currently errors out during certain HTTP/2 requests,
for example:

    > serf_get --http2 https://www.cloudflare.com
    Error running context: (120153) HTTP2 flow control limits exceeded

See the http2_handle_settings() function at http2_protocol.c:827, which
is responsible for processing SETTINGS_INITIAL_WINDOW_SIZE value
in a SETTINGS frame.

Serf uses the incoming value to update the size of the connection
flow-control window.  This violates RFC 7540, 6.9.2 [1], which states that
the SETTINGS_INITIAL_WINDOW_SIZE value in the SETTINGS frame
*cannot* alter the connection flow-control window size, and that
it only affects the initial window size for new streams.

Here is what happens in the example above (I think, this is how most of
the nginx servers currently behave):

    (Initial window size is 65535)

    <-  SETTINGS
    SETTINGS_INITIAL_WINDOW_SIZE: 65536

    (Serf incorrectly increases connection flow-control window by 1 byte,
     from 65535 to 65536, although this value should only be used for
     new streams.)

    <-  WINDOW_UPDATE
    Window Size Increment: 2147418112

    (The server is not interested in flow-control capabilities, and it
     advertises a flow-control window of the maximum size (2^31-1).
     While 65535 + 2147418112 is 2^31-1, Serf incorrectly thinks that
     it's current flow-control window is 65536, and 65536 + 2147418112
     is more than the allowed maximum of 2^31-1.  This triggers an error.)


The attached patch contains a fix for this issue.  The log message is
included in the beginning of the patch file.

[1] https://tools.ietf.org/html/rfc7540#section-6.9.2


Regards,
Evgeny Kotkov
HTTP/2: Fix improper handling of SETTINGS_INITIAL_WINDOW_SIZE that
could result in the "HTTP2 flow control limits exceeded" error when
talking to nginx servers:

    > serf_get --http2 https://www.cloudflare.com
    Error running context: (120153) HTTP2 flow control limits exceeded

Serf used the incoming value to update the size of the connection
flow-control window.  Doing so violates RFC 7540, 6.9.2 [1], which
states that the SETTINGS_INITIAL_WINDOW_SIZE value in the SETTINGS
frame *cannot* alter the connection flow-control window size, and
that it only affects the initial window size for new streams.

[1] https://tools.ietf.org/html/rfc7540#section-6.9.2

* protocols/http2_protocol.c
  (http2_handle_settings): Don't change the connection flow-control
   window size.  Include the relevant part of RFC 7540 in the comment.

Patch by: Evgeny Kotkov <evgeny.kotkov{_AT_}visualsvn.com>

Index: protocols/http2_protocol.c
===================================================================
--- protocols/http2_protocol.c  (revision 1787257)
+++ protocols/http2_protocol.c  (working copy)
@@ -824,7 +824,14 @@ http2_handle_settings(void *baton,
               /* Sanitize? */
                 serf__log(LOGLVL_INFO, SERF_LOGHTTP2, h2->config,
                           "Setting Initial Window Size %u\n", value);
-                h2->lr_window += (value - h2->lr_default_window);
+                /* This only affects the default window size for new streams
+                   (the connection window size is left unchanged):
+
+                   Both endpoints can adjust the initial window size for new
+                   streams by including a value for 
SETTINGS_INITIAL_WINDOW_SIZE
+                   in the SETTINGS frame that forms part of the connection
+                   preface.  The connection flow-control window can only be
+                   changed using WINDOW_UPDATE frames. */
                 h2->lr_default_window = value;
                 break;
             case HTTP2_SETTING_MAX_FRAME_SIZE:

Reply via email to