Included, thanks Stefan!
On Wed, Feb 20, 2019 at 4:57 PM ste...@eissing.org <ste...@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 ste...@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 ste...@eissing.org: > >> > >> > >> > >>> Am 20.02.2019 um 10:53 schrieb yla...@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 > > >
