You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "stefan@eissing.org" <st...@eissing.org> on 2019/02/20 14:14:30 UTC
Re: svn commit: r1853939 - /httpd/httpd/branches/2.4.x/STATUS
> Am 20.02.2019 um 10:53 schrieb ylavic@apache.org:
>
> Author: ylavic
> Date: Wed Feb 20 09:53:30 2019
> New Revision: 1853939
>
> URL: http://svn.apache.org/viewvc?rev=1853939&view=rev
> Log:
> Propose.
>
> Modified:
> httpd/httpd/branches/2.4.x/STATUS
>
> Modified: httpd/httpd/branches/2.4.x/STATUS
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1853939&r1=1853938&r2=1853939&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Wed Feb 20 09:53:30 2019
> @@ -246,6 +246,23 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-forward_100_continue-v3.patch
> +1: ylavic
>
> + *) mod_reqtimeout: Allow to configure (TLS-)handshake timeouts. PR 61310.
> + trunk patch: http://svn.apache.org/r1853901
> + http://svn.apache.org/r1853906
> + http://svn.apache.org/r1853908
> + http://svn.apache.org/r1853929
> + http://svn.apache.org/r1853935
> + 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-reqtimeout_handshake.patch
> + +1: ylavic
This one is giving me headaches. I added test_600_01 to master at https://github.com/icing/mod_h2 that triggers it.
My suspicion is that mod_reqtimeout and mod_http2 need some more coordination. While the handshake timeout is very useful also for h2 connections, waiting for GETLINE or such is less useful.
There is more than one path how a connection can go from h1 to h2 (alpn, direct 24 bytes detection, h1 upgrade:). Direct h2 use by curl/nghttp works, but mod_proxy_http2 does not.
We have a hook for protocol switching: ap_hook_protocol_switch(...) and probably mod_reqtimeout needs to listen to that and changes its (at least part of the) input/output filtering accordingly.
I will try to understand how things go sideways with the change and report back here.
-Stefan
Re: svn commit: r1853939 - /httpd/httpd/branches/2.4.x/STATUS
Posted by Yann Ylavic <yl...@gmail.com>.
Included, thanks Stefan!
On Wed, Feb 20, 2019 at 4:57 PM stefan@eissing.org <st...@eissing.org> wrote:
>
> Ok, got a fix in r1853967 and propose to include in mod_reqtimeout backport.
>
> Cheers, Stefan
>
> > Am 20.02.2019 um 16:09 schrieb stefan@eissing.org:
> >
> > Ok, I think I understood:
> >
> > - On a h2 slave connection, slave->keepalives is always > 0, to keep certain parts of our server from thinking "Oh, this is the first request!"
> > - mod_reqtimeout interprets this as "we are inbetween requests", ccfg->in_keep_alive != 0
> > - mod_reqtimeout.c:184+ input filter does a speculative read and expects to see either an error or some data
> > - if not, it return the spec read status
> > - Here, in the failure case, the base slave_in filter return APR_EAGAIN
> > * this was a fix to prevent ap_check_pipeline() from closing the connection when seeing APR_SUCCESS with 0 bytes
> > - The read itself is, in the test case, done by mod_cgid which on != APR_SUCCESS fails with a 400 response
> >
> > The patch did not really change this behaviour, but it changed the initialization order.
> >
> > Why does this only trigger in this new test case?
> > - Sending requests via mod_proxy_http2 changes the timings of EOF detection. When the EOF is not signalled on the header frame, the request body is indeterminate until a DATA frame with EOF arrives. That may happen after mod_reqtimeout checks.
> > - the slave input filter gets called with a SPEC read of 1 byte, has no data and also has not seen an EOF. It returns ARP_EAGAIN.
> >
> > What to do?
> > - I think the mod_reqtimeout patch is mostly correct and it should be be active on h2 slave connections as well
> > - But maybe mod_reqtimeout needs slightly different behaviour on a slave connection. maybe the keepalives spec reads need to be abandoned here.
> > - Changing the return code of the h2 slave input filter to return APR_SUCCESS on the speculative read will not really help. mod_reqtimeout will return this to the caller as there was no data. This means that mod_cgid will just call it again in a infinite loop until either data or an EOF arrives. That does not seem good either.
> >
> > Any other ideas?
> >
> > -Stefan
> >
> >> Am 20.02.2019 um 15:14 schrieb stefan@eissing.org:
> >>
> >>
> >>
> >>> Am 20.02.2019 um 10:53 schrieb ylavic@apache.org:
> >>>
> >>> Author: ylavic
> >>> Date: Wed Feb 20 09:53:30 2019
> >>> New Revision: 1853939
> >>>
> >>> URL: http://svn.apache.org/viewvc?rev=1853939&view=rev
> >>> Log:
> >>> Propose.
> >>>
> >>> Modified:
> >>> httpd/httpd/branches/2.4.x/STATUS
> >>>
> >>> Modified: httpd/httpd/branches/2.4.x/STATUS
> >>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1853939&r1=1853938&r2=1853939&view=diff
> >>> ==============================================================================
> >>> --- httpd/httpd/branches/2.4.x/STATUS (original)
> >>> +++ httpd/httpd/branches/2.4.x/STATUS Wed Feb 20 09:53:30 2019
> >>> @@ -246,6 +246,23 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> >>> 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-forward_100_continue-v3.patch
> >>> +1: ylavic
> >>>
> >>> + *) mod_reqtimeout: Allow to configure (TLS-)handshake timeouts. PR 61310.
> >>> + trunk patch: http://svn.apache.org/r1853901
> >>> + http://svn.apache.org/r1853906
> >>> + http://svn.apache.org/r1853908
> >>> + http://svn.apache.org/r1853929
> >>> + http://svn.apache.org/r1853935
> >>> + 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-reqtimeout_handshake.patch
> >>> + +1: ylavic
> >>
> >> This one is giving me headaches. I added test_600_01 to master at https://github.com/icing/mod_h2 that triggers it.
> >>
> >> My suspicion is that mod_reqtimeout and mod_http2 need some more coordination. While the handshake timeout is very useful also for h2 connections, waiting for GETLINE or such is less useful.
> >>
> >> There is more than one path how a connection can go from h1 to h2 (alpn, direct 24 bytes detection, h1 upgrade:). Direct h2 use by curl/nghttp works, but mod_proxy_http2 does not.
> >>
> >> We have a hook for protocol switching: ap_hook_protocol_switch(...) and probably mod_reqtimeout needs to listen to that and changes its (at least part of the) input/output filtering accordingly.
> >>
> >> I will try to understand how things go sideways with the change and report back here.
> >>
> >> -Stefan
> >
>
Re: svn commit: r1853939 - /httpd/httpd/branches/2.4.x/STATUS
Posted by "stefan@eissing.org" <st...@eissing.org>.
Ok, got a fix in r1853967 and propose to include in mod_reqtimeout backport.
Cheers, Stefan
> Am 20.02.2019 um 16:09 schrieb stefan@eissing.org:
>
> Ok, I think I understood:
>
> - On a h2 slave connection, slave->keepalives is always > 0, to keep certain parts of our server from thinking "Oh, this is the first request!"
> - mod_reqtimeout interprets this as "we are inbetween requests", ccfg->in_keep_alive != 0
> - mod_reqtimeout.c:184+ input filter does a speculative read and expects to see either an error or some data
> - if not, it return the spec read status
> - Here, in the failure case, the base slave_in filter return APR_EAGAIN
> * this was a fix to prevent ap_check_pipeline() from closing the connection when seeing APR_SUCCESS with 0 bytes
> - The read itself is, in the test case, done by mod_cgid which on != APR_SUCCESS fails with a 400 response
>
> The patch did not really change this behaviour, but it changed the initialization order.
>
> Why does this only trigger in this new test case?
> - Sending requests via mod_proxy_http2 changes the timings of EOF detection. When the EOF is not signalled on the header frame, the request body is indeterminate until a DATA frame with EOF arrives. That may happen after mod_reqtimeout checks.
> - the slave input filter gets called with a SPEC read of 1 byte, has no data and also has not seen an EOF. It returns ARP_EAGAIN.
>
> What to do?
> - I think the mod_reqtimeout patch is mostly correct and it should be be active on h2 slave connections as well
> - But maybe mod_reqtimeout needs slightly different behaviour on a slave connection. maybe the keepalives spec reads need to be abandoned here.
> - Changing the return code of the h2 slave input filter to return APR_SUCCESS on the speculative read will not really help. mod_reqtimeout will return this to the caller as there was no data. This means that mod_cgid will just call it again in a infinite loop until either data or an EOF arrives. That does not seem good either.
>
> Any other ideas?
>
> -Stefan
>
>> Am 20.02.2019 um 15:14 schrieb stefan@eissing.org:
>>
>>
>>
>>> Am 20.02.2019 um 10:53 schrieb ylavic@apache.org:
>>>
>>> Author: ylavic
>>> Date: Wed Feb 20 09:53:30 2019
>>> New Revision: 1853939
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1853939&view=rev
>>> Log:
>>> Propose.
>>>
>>> Modified:
>>> httpd/httpd/branches/2.4.x/STATUS
>>>
>>> Modified: httpd/httpd/branches/2.4.x/STATUS
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1853939&r1=1853938&r2=1853939&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>>> +++ httpd/httpd/branches/2.4.x/STATUS Wed Feb 20 09:53:30 2019
>>> @@ -246,6 +246,23 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>>> 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-forward_100_continue-v3.patch
>>> +1: ylavic
>>>
>>> + *) mod_reqtimeout: Allow to configure (TLS-)handshake timeouts. PR 61310.
>>> + trunk patch: http://svn.apache.org/r1853901
>>> + http://svn.apache.org/r1853906
>>> + http://svn.apache.org/r1853908
>>> + http://svn.apache.org/r1853929
>>> + http://svn.apache.org/r1853935
>>> + 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-reqtimeout_handshake.patch
>>> + +1: ylavic
>>
>> This one is giving me headaches. I added test_600_01 to master at https://github.com/icing/mod_h2 that triggers it.
>>
>> My suspicion is that mod_reqtimeout and mod_http2 need some more coordination. While the handshake timeout is very useful also for h2 connections, waiting for GETLINE or such is less useful.
>>
>> There is more than one path how a connection can go from h1 to h2 (alpn, direct 24 bytes detection, h1 upgrade:). Direct h2 use by curl/nghttp works, but mod_proxy_http2 does not.
>>
>> We have a hook for protocol switching: ap_hook_protocol_switch(...) and probably mod_reqtimeout needs to listen to that and changes its (at least part of the) input/output filtering accordingly.
>>
>> I will try to understand how things go sideways with the change and report back here.
>>
>> -Stefan
>
Re: svn commit: r1853939 - /httpd/httpd/branches/2.4.x/STATUS
Posted by "stefan@eissing.org" <st...@eissing.org>.
Ok, I think I understood:
- On a h2 slave connection, slave->keepalives is always > 0, to keep certain parts of our server from thinking "Oh, this is the first request!"
- mod_reqtimeout interprets this as "we are inbetween requests", ccfg->in_keep_alive != 0
- mod_reqtimeout.c:184+ input filter does a speculative read and expects to see either an error or some data
- if not, it return the spec read status
- Here, in the failure case, the base slave_in filter return APR_EAGAIN
* this was a fix to prevent ap_check_pipeline() from closing the connection when seeing APR_SUCCESS with 0 bytes
- The read itself is, in the test case, done by mod_cgid which on != APR_SUCCESS fails with a 400 response
The patch did not really change this behaviour, but it changed the initialization order.
Why does this only trigger in this new test case?
- Sending requests via mod_proxy_http2 changes the timings of EOF detection. When the EOF is not signalled on the header frame, the request body is indeterminate until a DATA frame with EOF arrives. That may happen after mod_reqtimeout checks.
- the slave input filter gets called with a SPEC read of 1 byte, has no data and also has not seen an EOF. It returns ARP_EAGAIN.
What to do?
- I think the mod_reqtimeout patch is mostly correct and it should be be active on h2 slave connections as well
- But maybe mod_reqtimeout needs slightly different behaviour on a slave connection. maybe the keepalives spec reads need to be abandoned here.
- Changing the return code of the h2 slave input filter to return APR_SUCCESS on the speculative read will not really help. mod_reqtimeout will return this to the caller as there was no data. This means that mod_cgid will just call it again in a infinite loop until either data or an EOF arrives. That does not seem good either.
Any other ideas?
-Stefan
> Am 20.02.2019 um 15:14 schrieb stefan@eissing.org:
>
>
>
>> Am 20.02.2019 um 10:53 schrieb ylavic@apache.org:
>>
>> Author: ylavic
>> Date: Wed Feb 20 09:53:30 2019
>> New Revision: 1853939
>>
>> URL: http://svn.apache.org/viewvc?rev=1853939&view=rev
>> Log:
>> Propose.
>>
>> Modified:
>> httpd/httpd/branches/2.4.x/STATUS
>>
>> Modified: httpd/httpd/branches/2.4.x/STATUS
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1853939&r1=1853938&r2=1853939&view=diff
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/STATUS (original)
>> +++ httpd/httpd/branches/2.4.x/STATUS Wed Feb 20 09:53:30 2019
>> @@ -246,6 +246,23 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>> 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-forward_100_continue-v3.patch
>> +1: ylavic
>>
>> + *) mod_reqtimeout: Allow to configure (TLS-)handshake timeouts. PR 61310.
>> + trunk patch: http://svn.apache.org/r1853901
>> + http://svn.apache.org/r1853906
>> + http://svn.apache.org/r1853908
>> + http://svn.apache.org/r1853929
>> + http://svn.apache.org/r1853935
>> + 2.4.x patch: http://people.apache.org/~ylavic/patches/httpd-2.4.x-reqtimeout_handshake.patch
>> + +1: ylavic
>
> This one is giving me headaches. I added test_600_01 to master at https://github.com/icing/mod_h2 that triggers it.
>
> My suspicion is that mod_reqtimeout and mod_http2 need some more coordination. While the handshake timeout is very useful also for h2 connections, waiting for GETLINE or such is less useful.
>
> There is more than one path how a connection can go from h1 to h2 (alpn, direct 24 bytes detection, h1 upgrade:). Direct h2 use by curl/nghttp works, but mod_proxy_http2 does not.
>
> We have a hook for protocol switching: ap_hook_protocol_switch(...) and probably mod_reqtimeout needs to listen to that and changes its (at least part of the) input/output filtering accordingly.
>
> I will try to understand how things go sideways with the change and report back here.
>
> -Stefan