You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@serf.apache.org by Evgeny Kotkov <ev...@visualsvn.com> on 2017/03/17 11:53:29 UTC
[PATCH] HTTP/2: Fix improper handling of SETTINGS_INITIAL_WINDOW_SIZE
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
Re: [PATCH] HTTP/2: Fix improper handling of
SETTINGS_INITIAL_WINDOW_SIZE
Posted by Branko Čibej <br...@apache.org>.
On 23.03.2017 10:27, Evgeny Kotkov wrote:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
>> That or r1788145 broke buildbot:
>>
>> https://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/484
>> https://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/498
> These two changes aren't triggered by "scons check" on the buildbots,
> so it is very unlikely that they have caused the failures.
>
> I see that the buildbots have been failing once in a while, with similar
> output:
>
> https://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/466/
> https://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/451/
> https://ci.apache.org/builders/serf-x64-macosx/builds/501/
>
> (Perhaps, this is caused by a test suite bug or indicates an existing
> issue in the code.)
I've seen such failures that cleared up after another test run. There
seems to be a heisenbug somewhere, but I don't know if it's limited to
OSX or not.
-- Brane
Re: [PATCH] HTTP/2: Fix improper handling of
SETTINGS_INITIAL_WINDOW_SIZE
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Bert Huijben wrote on Thu, Mar 23, 2017 at 13:44:06 +0100:
> So unless you want to do the heavy debugging to get down to the real
> issue, feel free to ignore this problem :-)
Sorry for the false alarm Evgeny, I hadn't been aware of the bots' flakiness.
RE: [PATCH] HTTP/2: Fix improper handling of SETTINGS_INITIAL_WINDOW_SIZE
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Evgeny Kotkov [mailto:evgeny.kotkov@visualsvn.com]
> Sent: donderdag 23 maart 2017 10:27
> To: Daniel Shahaf <d....@daniel.shahaf.name>
> Cc: dev@serf.apache.org; Bert Huijben <be...@qqmail.nl>
> Subject: Re: [PATCH] HTTP/2: Fix improper handling of
> SETTINGS_INITIAL_WINDOW_SIZE
>
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
>
> > That or r1788145 broke buildbot:
> >
> > https://ci.apache.org/builders/serf-x64-macosx-default-
> openssl/builds/484
> > https://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/498
>
> These two changes aren't triggered by "scons check" on the buildbots,
> so it is very unlikely that they have caused the failures.
>
> I see that the buildbots have been failing once in a while, with similar
> output:
>
> https://ci.apache.org/builders/serf-x64-macosx-default-
> openssl/builds/466/
> https://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/451/
> https://ci.apache.org/builders/serf-x64-macosx/builds/501/
>
> (Perhaps, this is caused by a test suite bug or indicates an existing
> issue in the code.)
Just forcing the builds one at a time fixes the problem...
So unless you want to do the heavy debugging to get down to the real issue, feel free to ignore this problem :-)
Bert
Re: [PATCH] HTTP/2: Fix improper handling of SETTINGS_INITIAL_WINDOW_SIZE
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
> That or r1788145 broke buildbot:
>
> https://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/484
> https://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/498
These two changes aren't triggered by "scons check" on the buildbots,
so it is very unlikely that they have caused the failures.
I see that the buildbots have been failing once in a while, with similar
output:
https://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/466/
https://ci.apache.org/builders/serf-x64-macosx-apr2.0-dev/builds/451/
https://ci.apache.org/builders/serf-x64-macosx/builds/501/
(Perhaps, this is caused by a test suite bug or indicates an existing
issue in the code.)
Regards,
Evgeny Kotkov
Re: [PATCH] HTTP/2: Fix improper handling of
SETTINGS_INITIAL_WINDOW_SIZE
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Evgeny Kotkov wrote on Wed, Mar 22, 2017 at 21:10:07 +0300:
> Evgeny Kotkov <ev...@visualsvn.com> writes:
>
> > The attached patch contains a fix for this issue. The log message is
> > included in the beginning of the patch file.
>
> Committed in https://svn.apache.org/r1788146
That or r1788145 broke buildbot:
https://ci.apache.org/builders/serf-x64-macosx-default-openssl/builds/484
https://ci.apache.org/builders/serf-x64-macosx-apr1.5/builds/498
Re: [PATCH] HTTP/2: Fix improper handling of SETTINGS_INITIAL_WINDOW_SIZE
Posted by Evgeny Kotkov <ev...@visualsvn.com>.
Evgeny Kotkov <ev...@visualsvn.com> writes:
> The attached patch contains a fix for this issue. The log message is
> included in the beginning of the patch file.
Committed in https://svn.apache.org/r1788146
Regards,
Evgeny Kotkov