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