Ok, got a fix in r1853967 and propose to include in mod_reqtimeout backport.
Cheers, Stefan > Am 20.02.2019 um 16:09 schrieb [email protected]: > > 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 [email protected]: >> >> >> >>> Am 20.02.2019 um 10:53 schrieb [email protected]: >>> >>> 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 >
