Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi Yann, Am 09.12.2013 00:30, schrieb Yann Ylavic: Now, if trying to send the first bytes of the request immediately fails because the socket isn't connected anymore (e.g. EPIPE), you *know* that exactly *none* bits of your current request reached the server. For this reason it should be safe to retry by establishing a new connection to the server and to try to send the request again over the newly established connection. The send() will never fail at this point if the peer FIN-closed the connection (EPIPE or whatever on non-unixes), since it is only half-closed. You can't read anymore, but you still can write, and this is precisely why the error log is error *reading* status line and not error writing A second send() could fail however, though possibly with ECONNRESET, and you are no better here. The point is that mod_proxy already checks whether the connection is alive before trying to send a (successive) request on the kept alive connection, by using a recv(MSG_PEEK) (see ap_proxy_is_socket_connected). If that fails, a new connection is established and the request is sent on it, this is already the current behaviour. The race condition is when this check succeeds, but the peer closes the connection before the request reaches it (in the meantime). Okay, got that. If it really works like it should then I don't know how to improve the code. So the remaining question is, whether the checks whether the connection is alive are sufficient. Unfortunately I have no resources to investigate this any further. Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi all, Am 05.12.2013 22:03, schrieb Yann Ylavic: I'm not talking about retrying requests but retrying writing on the socket after trying to re-open a connection. When mod_proxy tries to forward the client request to the backends and encounters a closed connection due to the described race condition it will fail back to the client. Instead, I suggest trying to re-open the connection once and then either send the client request over that connection or go back to the client with an error. No double-sending requests anywhere. I must be missing something. When mod_proxy encounters a closed (reused) connection while forwarding the client's request, it has just sent the request once, right? That depends on the actual state. What Thomas is talking about (I know because I worked with him on the same product he is referring to here) is the state where you have everything in place to send out the next request headers on a connection that was established by a previous request and that was kept open using HTTP Keep-Alive. Let me stress the fact that in this state you haven't sent a single bit of the current request over the wire yet. Now, if trying to send the first bytes of the request immediately fails because the socket isn't connected anymore (e.g. EPIPE), you *know* that exactly *none* bits of your current request reached the server. For this reason it should be safe to retry by establishing a new connection to the server and to try to send the request again over the newly established connection. To avoid an endless retry loop, you certainly should do this retry approach only once per request. In the sense of getting the reverse proxy request correctly processed by the backend server, this approach *does* fix the keep-alive race condition issue where the server closes the backend connection because it sees no (new) client request for too long time -- IIRC this is what this discussion started with. I hope I could shed some more some light on this issue and on the idea how to possibly tackle it in Apache httpd. Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Sun, Dec 8, 2013 at 11:51 AM, Micha Lenk mi...@lenk.info wrote: Am 05.12.2013 22:03, schrieb Yann Ylavic: When mod_proxy encounters a closed (reused) connection while forwarding the client's request, it has just sent the request once, right? That depends on the actual state. What Thomas is talking about (I know because I worked with him on the same product he is referring to here) is the state where you have everything in place to send out the next request headers on a connection that was established by a previous request and that was kept open using HTTP Keep-Alive. Let me stress the fact that in this state you haven't sent a single bit of the current request over the wire yet. Yes, here we are. Now, if trying to send the first bytes of the request immediately fails because the socket isn't connected anymore (e.g. EPIPE), you *know* that exactly *none* bits of your current request reached the server. For this reason it should be safe to retry by establishing a new connection to the server and to try to send the request again over the newly established connection. The send() will never fail at this point if the peer FIN-closed the connection (EPIPE or whatever on non-unixes), since it is only half-closed. You can't read anymore, but you still can write, and this is precisely why the error log is error *reading* status line and not error writing A second send() could fail however, though possibly with ECONNRESET, and you are no better here. The point is that mod_proxy already checks whether the connection is alive before trying to send a (successive) request on the kept alive connection, by using a recv(MSG_PEEK) (see ap_proxy_is_socket_connected). If that fails, a new connection is established and the request is sent on it, this is already the current behaviour. The race condition is when this check succeeds, but the peer closes the connection before the request reaches it (in the meantime). In the sense of getting the reverse proxy request correctly processed by the backend server, this approach *does* fix the keep-alive race condition issue where the server closes the backend connection because it sees no (new) client request for too long time -- IIRC this is what this discussion started with. It does *not* fix the race, nothing can, and the proxy can't know whether its request as been processed by the peer based on the (second) send() error. IMHO, the best that can be done is HTTP ping for POST requests (using Expect: 100-continue, already implemented), retry idempotent requests (TODO), reduce the race time (my proposal/patch), and let client client decide for others (proxy-initial-not-pooled can help it). I hope I could shed some more some light on this issue and on the idea how to possibly tackle it in Apache httpd. Thanks, I really wish it could be, and Thomas prove me wrong! Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
I don't want to overdo this thread (apologies if it is already), last comment yet... On Thu, Dec 5, 2013 at 10:03 PM, Yann Ylavic ylavic@gmail.com wrote: Is sending a partial request first what you'd like to (be) implement(ed), and only if that succeeds send the remaining data? The issue would still be there for the remaining data since nothing guarantees the partial (initial) sending has reached the backend, which might still have closed the connection in the meantime (error reading status line from remote server is the proof, the failure occurs on the recv(), not the send()). Maybe one (other than me) facing the race condition can give my patch a chance (no more error reading status line... here with TTL KA-timeout, pretetch, and slow clients). No noticeable slow-down either, although statements like this are worthless without measure... I plan to do that soon (and keep you informed, for those who still care :) Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi, should this be fixed in trunk already? I see some commits in proxy code based on your ideas Yann, but I'm not sure if they address this particular problem too. Jan Kaluza On 10/17/2013 04:52 PM, Yann Ylavic wrote: On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com mailto:thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating the backend connection in an error prone way. By not ensuring the connection is still valid - e.g. as seen by the peer - we risk running into this problem no matter what we do elsewhere. So I really think the proper way to handle this is to reopen the connection if necessary - be this with a filter or integrated into the connection handling itself - and only fail after we tried at least once. IMHO this is hardly feasible for non-idempotent (and no body) requests (without disabling connection reuse, or using proxy-initial-not-pooled to let the client choose), even if mod_proxy takes care of not sending the full request in one time to be able to detect connection problems before it's too late (to retry), there is still a possibility that it won't know before sending the last chunk, and the issue remain. mod_proxy_http addresses retries for requests *with body* by the ping parameter, maybe it could also retry idempotent requests (like mod_proxy_ajp does), but this is still not an overall solution, is there one? Re-thinking the proposed flush mechanism, I don't think using the flushing will really solve the problem, albeit it probably narrowing down the window of opportunity for the problem significantly in most scenarios. I mention this because I'm getting the feeling two different discussions are being mixed up: Solving the keep-alive time out problem and introducing the flush-all-immediately option. While the latter might improve the situation with the first, it is no complete solution and there are a lot of scenarios where it does not apply due to other factors like filters (as discussed previously). See above, the flushall does not resolve the issue (filters...), but the early prefetch + flushall reduces it considerably, IMHO. The flush option itself sounds like a good idea, so I see no reason why not to put it in. I just don't want to loose focus on the actual problem ;-) I don't see either... Please consider the new (yet) patch attached, can you still encounter some proxy: error reading status line from
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi Jan, I don't think it is fixed in trunk, but I may have missed the commits. Which ones are you talking about? Regards, Yann. On Thu, Dec 5, 2013 at 1:51 PM, Jan Kaluža jkal...@redhat.com wrote: Hi, should this be fixed in trunk already? I see some commits in proxy code based on your ideas Yann, but I'm not sure if they address this particular problem too. Jan Kaluza On 10/17/2013 04:52 PM, Yann Ylavic wrote: On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com mailto:thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating the backend connection in an error prone way. By not ensuring the connection is still valid - e.g. as seen by the peer - we risk running into this problem no matter what we do elsewhere. So I really think the proper way to handle this is to reopen the connection if necessary - be this with a filter or integrated into the connection handling itself - and only fail after we tried at least once. IMHO this is hardly feasible for non-idempotent (and no body) requests (without disabling connection reuse, or using proxy-initial-not-pooled to let the client choose), even if mod_proxy takes care of not sending the full request in one time to be able to detect connection problems before it's too late (to retry), there is still a possibility that it won't know before sending the last chunk, and the issue remain. mod_proxy_http addresses retries for requests *with body* by the ping parameter, maybe it could also retry idempotent requests (like mod_proxy_ajp does), but this is still not an overall solution, is there one? Re-thinking the proposed flush mechanism, I don't think using the flushing will really solve the problem, albeit it probably narrowing down the window of opportunity for the problem significantly in most scenarios. I mention this because I'm getting the feeling two different discussions are being mixed up: Solving the keep-alive time out problem and introducing the flush-all-immediately option. While the latter might improve the situation with the first, it is no complete solution and there are a lot of scenarios where it does not apply due to other factors like filters (as discussed previously). See above, the flushall does not resolve the issue (filters...), but the early prefetch + flushall reduces it considerably, IMHO. The flush
Re: mod_proxy, oooled backend connections and the keep-alive race condition
There hardly seemed any consensus on the patch... It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. IMO, a reverse proxy should get out of the way as quickly as possible. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? Plus, as mentioned, this single patch includes 2 fixes which can be/should be independent. On Dec 5, 2013, at 8:06 AM, Yann Ylavic ylavic@gmail.com wrote: Hi Jan, I don't think it is fixed in trunk, but I may have missed the commits. Which ones are you talking about? Regards, Yann. On Thu, Dec 5, 2013 at 1:51 PM, Jan Kaluža jkal...@redhat.com wrote: Hi, should this be fixed in trunk already? I see some commits in proxy code based on your ideas Yann, but I'm not sure if they address this particular problem too. Jan Kaluza On 10/17/2013 04:52 PM, Yann Ylavic wrote: On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com mailto:thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating the backend connection in an error prone way. By not ensuring the connection is still valid - e.g. as seen by the peer - we risk running into this problem no matter what we do elsewhere. So I really think the proper way to handle this is to reopen the connection if necessary - be this with a filter or integrated into the connection handling itself - and only fail after we tried at least once. IMHO this is hardly feasible for non-idempotent (and no body) requests (without disabling connection reuse, or using proxy-initial-not-pooled to let the client choose), even if mod_proxy takes care of not sending the full request in one time to be able to detect connection problems before it's too late (to retry), there is still a possibility that it won't know before sending the last chunk, and the issue remain. mod_proxy_http addresses retries for requests *with body* by the ping parameter, maybe it could also retry idempotent requests (like mod_proxy_ajp does), but this is still not an overall solution, is there one? Re-thinking the proposed flush mechanism, I don't think using the flushing will really solve the problem, albeit it probably narrowing down the window of opportunity for the problem significantly in most scenarios. I mention this because I'm getting the feeling
Re: mod_proxy, oooled backend connections and the keep-alive race condition
It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. While it's true that the race condition itself cannot be avoided we can definitely work around the resulting problem situation, e.g. by trying to open the connection again once and then either send the data or fail back to the client as we do right now. The approach itself is not theoretical: I know that's exactly how certain forward proxies solve the problem. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? I go with my original suggestion: Have the above mentioned retry-once mechanism apply on connection level so that request modules don't even notice. Otherwise I would agree in having it added to the appropriate modules. I would like to come up with the appropriate patch but I doubt I'll have the time for it anytime soon :-( As it stands I think Yann's approach in flushing through the reverse proxy is better then doing nothing. Though I expect that to be useless for a lot of deployments where stuff like AV scanning or mod_security will need to buffer the request bodies - then you will have the original problem again. On Thu, Dec 5, 2013 at 4:05 PM, Jim Jagielski j...@jagunet.com wrote: There hardly seemed any consensus on the patch... It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. IMO, a reverse proxy should get out of the way as quickly as possible. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? Plus, as mentioned, this single patch includes 2 fixes which can be/should be independent. On Dec 5, 2013, at 8:06 AM, Yann Ylavic ylavic@gmail.com wrote: Hi Jan, I don't think it is fixed in trunk, but I may have missed the commits. Which ones are you talking about? Regards, Yann. On Thu, Dec 5, 2013 at 1:51 PM, Jan Kaluža jkal...@redhat.com wrote: Hi, should this be fixed in trunk already? I see some commits in proxy code based on your ideas Yann, but I'm not sure if they address this particular problem too. Jan Kaluza On 10/17/2013 04:52 PM, Yann Ylavic wrote: On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com mailto:thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating the
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Thu, Dec 5, 2013 at 4:05 PM, Jim Jagielski j...@jagunet.com wrote: There hardly seemed any consensus on the patch... It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. I don't think more cycles are added by this patch. What is done is that ap_proxy_http_request() is split in two functions, the prefetch part is now in ap_proxy_http_prefetch(), and ap_proxy_http_prefetch()+ap_proxy_http_request() has no more cycles than the previous ap_proxy_http_request(). Unless you consider dereferencing pointer arguments a cycles-overhead compared to using local variables... I agree the race condition can't really be removed, mod_proxy can (must?) strive for limiting it. With this patch and the reslist's TTL below the backend's KeepAliveTimeout, the race should not occur unless the backend closed before the timeout (which may indeed be the case when, say, mpm_event is busy). IMO, a reverse proxy should get out of the way as quickly as possible. I'm not sure to understand this but, IMHO, a reverse proxy should not reserve a backend connection (possibly) far before using it, and presume it will still be alive when it's time to use it. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? Agreed. Plus, as mentioned, this single patch includes 2 fixes which can be/should be independent. This was just to show that early prefetch and flushall can work together, but they are indeed 2 different things. I can split them into different patches, should there be an interest in one or/and the other. Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Thu, Dec 5, 2013 at 5:04 PM, Thomas Eckert thomas.r.w.eck...@gmail.comwrote: It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. While it's true that the race condition itself cannot be avoided we can definitely work around the resulting problem situation, e.g. by trying to open the connection again once and then either send the data or fail back to the client as we do right now. The approach itself is not theoretical: I know that's exactly how certain forward proxies solve the problem. You can't retry all the requests (particularly non idempotent ones), not even once. Suppose it is a charged $100 order, you wouldn't like any proxy to double that because of network problems... Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? I go with my original suggestion: Have the above mentioned retry-once mechanism apply on connection level so that request modules don't even notice. Otherwise I would agree in having it added to the appropriate modules. I would like to come up with the appropriate patch but I doubt I'll have the time for it anytime soon :-( Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. As it stands I think Yann's approach in flushing through the reverse proxy is better then doing nothing. Though I expect that to be useless for a lot of deployments where stuff like AV scanning or mod_security will need to buffer the request bodies - then you will have the original problem again. My first approach was to flush only, but the latest patch take also care about buffered request, the backend won't be connected/reused until the bytes are in mod_proxy.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Thu, Dec 5, 2013 at 5:45 PM, Yann Ylavic ylavic@gmail.com wrote: On Thu, Dec 5, 2013 at 5:07 PM, Yann Ylavic ylavic@gmail.com wrote: On Thu, Dec 5, 2013 at 4:05 PM, Jim Jagielski j...@jagunet.com wrote: There hardly seemed any consensus on the patch... It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. I don't think more cycles are added by this patch. What is done is that ap_proxy_http_request() is split in two functions, the prefetch part is now in ap_proxy_http_prefetch(), and ap_proxy_http_prefetch()+ap_proxy_http_request() has no more cycles than the previous ap_proxy_http_request(). Unless you consider dereferencing pointer arguments a cycles-overhead compared to using local variables... That's not totally true, maybe you refer to this cycles : +/* Preserve the header/input brigades since they may be retried. */ +input_bb = apr_brigade_create(p, backconn-bucket_alloc); +header_bb = apr_brigade_create(p, backconn-bucket_alloc); +proxy_buckets_lifetime_transform(r, input_brigade, input_bb); +proxy_buckets_lifetime_transform(r, header_brigade, header_bb); mod_proxy may need to setaside these buckets. Oups, I meant, since input_brigade has already been setaside during prefetch, reading its buckets in proxy_buckets_lifetime_transform() shouldn't take much time (though a few more cycles).
Re: mod_proxy, oooled backend connections and the keep-alive race condition
You can't retry all the requests (particularly non idempotent ones), not even once. Suppose it is a charged $100 order, you wouldn't like any proxy to double that because of network problems... I'm not talking about retrying requests but retrying writing on the socket after trying to re-open a connection. When mod_proxy tries to forward the client request to the backends and encounters a closed connection due to the described race condition it will fail back to the client. Instead, I suggest trying to re-open the connection once and then either send the client request over that connection or go back to the client with an error. No double-sending requests anywhere. Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. I doubt that statement is correct. As I said before, I've seen it work with another proxy so the basic approach is no problem. You might be correct in that it's not possible with apache but I cannot confirm or disconfirm this until I have tried adding it. Anyway, this is getting us nowhere until I (or anyone) come(s) up with a corresponding patch. I hope I'll get around for it sometime until January. Until then I'll just see this as declined. On Thu, Dec 5, 2013 at 5:26 PM, Yann Ylavic ylavic@gmail.com wrote: On Thu, Dec 5, 2013 at 5:04 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. While it's true that the race condition itself cannot be avoided we can definitely work around the resulting problem situation, e.g. by trying to open the connection again once and then either send the data or fail back to the client as we do right now. The approach itself is not theoretical: I know that's exactly how certain forward proxies solve the problem. You can't retry all the requests (particularly non idempotent ones), not even once. Suppose it is a charged $100 order, you wouldn't like any proxy to double that because of network problems... Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? I go with my original suggestion: Have the above mentioned retry-once mechanism apply on connection level so that request modules don't even notice. Otherwise I would agree in having it added to the appropriate modules. I would like to come up with the appropriate patch but I doubt I'll have the time for it anytime soon :-( Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. As it stands I think Yann's approach in flushing through the reverse proxy is better then doing nothing. Though I expect that to be useless for a lot of deployments where stuff like AV scanning or mod_security will need to buffer the request bodies - then you will have the original problem again. My first approach was to flush only, but the latest patch take also care about buffered request, the backend won't be connected/reused until the bytes are in mod_proxy.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Thu, Dec 5, 2013 at 7:03 PM, Thomas Eckert thomas.r.w.eck...@gmail.comwrote: I'm not talking about retrying requests but retrying writing on the socket after trying to re-open a connection. When mod_proxy tries to forward the client request to the backends and encounters a closed connection due to the described race condition it will fail back to the client. Instead, I suggest trying to re-open the connection once and then either send the client request over that connection or go back to the client with an error. No double-sending requests anywhere. I must be missing something. When mod_proxy encounters a closed (reused) connection while forwarding the client's request, it has just sent the request once, right? If, in that case, it opens a new connection and send that same request on it, this is the second time, still right? Unless it didn't send the complete request the first time, I mean deliberately by sending only part of the request (incomplete body if any, or incomplete header otherwise), how can the proxy be sure the first request has not been interpreted by the backend already? This is not because the send() failed on the proxy side the backend has not recv()ed all the bytes (there might be a problem with the ack of the last bytes for example), hence the request could be valid from the backend side. Is sending a partial request first what you'd like to (be) implement(ed), and only if that succeeds send the remaining data? Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. I doubt that statement is correct. As I said before, I've seen it work with another proxy so the basic approach is no problem. You might be correct in that it's not possible with apache but I cannot confirm or disconfirm this until I have tried adding it. A proxy that re-sends the same non-idempotent request twice is simply not HTTP compliant ( http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-25, 6.3.1. Retrying Requests). However, the RFC does not talk about incomplete requests. Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Sorry for the delayed reply. At the moment I don't have time to look at the
patch proposal in detail, sorry about that too. I'll get back to it soon, I
hope.
Pre-fetching 16K (or waiting for input filters to provide these) is not
always a fast operation, and the case where the backend closes its
connection in the meantime is easy to
reproduce (and even likely to happen with some applications).
I absolutely agree. That's why I proposed to solve this more systematically
then trial-and-error-like in the first place.
Hence, why cannot mod_proxy_http prefetch the client's body *before*
trying anything with the backend connection, and only if it's all good,
connect (or reuse a connection to) the
backend and then start forwarding the data immediatly (flushing the
prefetched ones) ?
Now I'm confused. Are we talking about flushing everything immediately from
client to backend (that's what I understood from your first patch proposal
in this thread) or are we talking about pre fetching the whole request body
from the client and then passing it on to the backend as a whole ?
The problem I mentioned in the first message is about treating the backend
connection in an error prone way. By not ensuring the connection is still
valid - e.g. as seen by the peer - we risk running into this problem no
matter what we do elsewhere. So I really think the proper way to handle
this is to reopen the connection if necessary - be this with a filter or
integrated into the connection handling itself - and only fail after we
tried at least once.
Re-thinking the proposed flush mechanism, I don't think using the flushing
will really solve the problem, albeit it probably narrowing down the window
of opportunity for the problem significantly in most scenarios. I mention
this because I'm getting the feeling two different discussions are being
mixed up: Solving the keep-alive time out problem and introducing the
flush-all-immediately option. While the latter might improve the
situation with the first, it is no complete solution and there are a lot of
scenarios where it does not apply due to other factors like filters (as
discussed previously).
The flush option itself sounds like a good idea, so I see no reason why not
to put it in. I just don't want to loose focus on the actual problem ;-)
Cheers,
Thomas
On Mon, Oct 7, 2013 at 6:49 PM, Yann Ylavic ylavic@gmail.com wrote:
Sorry to insist about this issue but I don't see how the current
mod_proxy_http's behaviour of connecting the backend (or checking
established connection reusability) before prefetching the client's body is
not a problem.
Pre-fetching 16K (or waiting for input filters to provide these) is not
always a fast operation, and the case where the backend closes its
connection in the meantime is easy to reproduce (and even likely to happen
with some applications).
Hence, why cannot mod_proxy_http prefetch the client's body *before*
trying anything with the backend connection, and only if it's all good,
connect (or reuse a connection to) the backend and then start forwarding
the data immediatly (flushing the prefetched ones) ?
By doing this, the time between the connect (or check) and the first bytes
sent to the backend is minimal.
It does not require to disable the prefetch since this one can really help
the decision about forwarding Content-Length vs chunked (which may not be
handled by the backend, IFAIK this is not a HTTP/1.1 requirement to handle
it for requests).
That was the purpose of the first patch I proposed ealier, but had no
feedback...
Maybe this message and the following patch could have more chance.
This patch is quite the same as the previous one (ap_proxy_http_request
split in 2, ap_proxy_http_prefetch to prefetch before connect and
ap_proxy_http_request to forward using the relevent
stream_reqbody_cl/chunked once prefetched and connected). It just fixes the
immediate flush of the prefetched bytes when it's time to forward, and the
handling of backend-close set by ap_proxy_create_hdrbrgd (or
ap_proxy_http_prefetch) before the connection even exist (or the previous
one to be reused).
Regards,
Yann.
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1529127)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -251,6 +251,7 @@ static int stream_reqbody_chunked(apr_pool_t *p,
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
char chunk_hdr[20]; /* must be here due to transient bucket. */
+int flush = 0;
/* If this brigade contains EOS, either stop or remove it. */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
@@ -299,13 +300,14 @@ static int stream_reqbody_chunked(apr_pool_t *p,
}
header_brigade = NULL;
+flush = 1;
}
else {
bb = input_brigade;
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating the backend connection in an error prone way. By not ensuring the connection is still valid - e.g. as seen by the peer - we risk running into this problem no matter what we do elsewhere. So I really think the proper way to handle this is to reopen the connection if necessary - be this with a filter or integrated into the connection handling itself - and only fail after we tried at least once. IMHO this is hardly feasible for non-idempotent (and no body) requests (without disabling connection reuse, or using proxy-initial-not-pooled to let the client choose), even if mod_proxy takes care of not sending the full request in one time to be able to detect connection problems before it's too late (to retry), there is still a possibility that it won't know before sending the last chunk, and the issue remain. mod_proxy_http addresses retries for requests *with body* by the ping parameter, maybe it could also retry idempotent requests (like mod_proxy_ajp does), but this is still not an overall solution, is there one? Re-thinking the proposed flush mechanism, I don't think using the flushing will really solve the problem, albeit it probably narrowing down the window of opportunity for the problem significantly in most scenarios. I mention this because I'm getting the feeling two different discussions are being mixed up: Solving the keep-alive time out problem and introducing the flush-all-immediately option. While the latter might improve the situation with the first, it is no complete solution and there are a lot of scenarios where it does not apply due to other factors like filters (as discussed previously). See above, the flushall does not resolve the issue (filters...), but the early prefetch + flushall reduces it considerably, IMHO. The flush option itself sounds like a good idea, so I see no reason why not to put it in. I just don't want to loose focus on the actual problem ;-) I don't see either... Please consider the new (yet) patch attached, can you still encounter some proxy: error reading status line from remote server with it? Even without proxy-flushall set, you shouldn't, whatever the time taken to read the request first 16K bytes... Regards, Yann. httpd-trunk-mod_proxy_http_prefetch+flushall.patch Description: Binary data
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi Yann, Am 03.10.2013 15:33, schrieb Yann Ylavic: On Thu, Oct 3, 2013 at 2:07 PM, Micha Lenk mi...@lenk.info wrote: Independent from how the HRS issue (CVE-2005-2088) was fixed at that time, I still believe that it is a bad idea in terms of security to flush the buffer and forward its content to the backend before the *whole* request body has been received. mod_proxy won't forward anything before the whole request *header* is received (it isn't even called before), with or without my patch(es), and that prevent HRS provided the httpd checks Content-Length/Transfer-Encoding against http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23#page-31 (which is the case, at least in trunk). mod_proxy will also prevent HRS by checking that the received data do not exceed the announced size, again my patch(es) don't change that. Well, assessing HRS alike issues, you need to not only look at the first HTTP request, but more importantly at a possible subsequent HTTP request sent on the same connection. So, if you flush the buffer and forward its content to the backend, you need to make sure to not accidentally forwarding the next request without parsing it. That is all I wanted to stress when talking about a potential HRS issue. It currently prefetches 16K of the body (if any) and forwards that and everything after to the backend from there, it won't retain the *whole* body unless one plays with the proxy-sendcl env. If this is the same behavior as with current unpatched httpd, I am fine with it. But then I guess this behavior is unrelated to your patch, right? Do you mean you always setenv proxy-sendcl to force mod_proxy in full-fetching mode because of security issues ? No. I'm not sure what changes you are talking about, though. Is it about the flushall or the prefetch before connect patch ? It is about the flushall patch. Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Sorry to insist about this issue but I don't see how the current
mod_proxy_http's behaviour of connecting the backend (or checking
established connection reusability) before prefetching the client's body is
not a problem.
Pre-fetching 16K (or waiting for input filters to provide these) is not
always a fast operation, and the case where the backend closes its
connection in the meantime is easy to reproduce (and even likely to happen
with some applications).
Hence, why cannot mod_proxy_http prefetch the client's body *before* trying
anything with the backend connection, and only if it's all good, connect
(or reuse a connection to) the backend and then start forwarding the data
immediatly (flushing the prefetched ones) ?
By doing this, the time between the connect (or check) and the first bytes
sent to the backend is minimal.
It does not require to disable the prefetch since this one can really help
the decision about forwarding Content-Length vs chunked (which may not be
handled by the backend, IFAIK this is not a HTTP/1.1 requirement to handle
it for requests).
That was the purpose of the first patch I proposed ealier, but had no
feedback...
Maybe this message and the following patch could have more chance.
This patch is quite the same as the previous one (ap_proxy_http_request
split in 2, ap_proxy_http_prefetch to prefetch before connect and
ap_proxy_http_request to forward using the relevent
stream_reqbody_cl/chunked once prefetched and connected). It just fixes the
immediate flush of the prefetched bytes when it's time to forward, and the
handling of backend-close set by ap_proxy_create_hdrbrgd (or
ap_proxy_http_prefetch) before the connection even exist (or the previous
one to be reused).
Regards,
Yann.
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1529127)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -251,6 +251,7 @@ static int stream_reqbody_chunked(apr_pool_t *p,
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
char chunk_hdr[20]; /* must be here due to transient bucket. */
+int flush = 0;
/* If this brigade contains EOS, either stop or remove it. */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
@@ -299,13 +300,14 @@ static int stream_reqbody_chunked(apr_pool_t *p,
}
header_brigade = NULL;
+flush = 1;
}
else {
bb = input_brigade;
}
/* The request is flushed below this loop with chunk EOS header */
-rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb, 0);
+rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
flush);
if (rv != OK) {
return rv;
}
@@ -389,12 +391,14 @@ static int stream_reqbody_cl(apr_pool_t *p,
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
+int flush = 0;
+
apr_brigade_length(input_brigade, 1, bytes);
bytes_streamed += bytes;
/* If this brigade contains EOS, either stop or remove it. */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
-seen_eos = 1;
+seen_eos = flush = 1;
/* We can't pass this EOS to the output_filters. */
e = APR_BRIGADE_LAST(input_brigade);
@@ -444,13 +448,14 @@ static int stream_reqbody_cl(apr_pool_t *p,
}
header_brigade = NULL;
+flush = 1;
}
else {
bb = input_brigade;
}
/* Once we hit EOS, we are ready to flush. */
-rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
seen_eos);
+rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
flush);
if (rv != OK) {
return rv ;
}
@@ -677,29 +682,33 @@ static apr_status_t proxy_buckets_lifetime_transfo
return rv;
}
+enum rb_methods {
+RB_INIT,
+RB_STREAM_CL,
+RB_STREAM_CHUNKED,
+RB_SPOOL_CL
+};
+
static
-int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
+int ap_proxy_http_prefetch(apr_pool_t *p, request_rec *r,
proxy_conn_rec *p_conn, proxy_worker
*worker,
proxy_server_conf *conf,
apr_uri_t *uri,
- char *url, char *server_portstr)
+ char *url, char *server_portstr,
+ apr_bucket_brigade *header_brigade,
+ apr_bucket_brigade *input_brigade,
+ char **old_cl_val, char **old_te_val,
+ enum rb_methods *rb_method)
{
conn_rec *c = r-connection;
apr_bucket_alloc_t *bucket_alloc = c-bucket_alloc;
-apr_bucket_brigade *header_brigade;
-
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Having a look at MPM event's doc ( http://httpd.apache.org/docs/current/mod/event.html#asyncrequestworkerfactor), I found this : [...] if all workers are busy, [mpm_event] will close connections in keep-alive state even if the keep-alive timeout has not expired . On Fri, Aug 2, 2013 at 2:33 PM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: The typical way to solve this today is to know the keepalive timeout of the backend and set ttl for this worker to a value a few seconds below. So it might not work if the backend is a busy mpm_event... Regards Rüdiger -Ursprüngliche Nachricht- Von: Jim Jagielski Gesendet: Freitag, 2. August 2013 14:29 An: dev@httpd.apache.org Betreff: Re: mod_proxy, oooled backend connections and the keep-alive race condition +1 for the theory, but I'm not sure if it's feasible or not. On Aug 2, 2013, at 5:28 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not- pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi Yann, Am 01.10.2013 17:08, schrieb Yann Ylavic: As far as I understand the issue, the main point of prefetch was to fix CVE-2005-2088, a HTTP Request Smuggling attack (see also http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-2088). This is discussed in PR40029 and is not related to HRS, the real fix regarding HRS was about both CL/TE sent by th client (https://issues.apache.org/bugzilla/show_bug.cgi?id=40029#c4). Independent from how the HRS issue (CVE-2005-2088) was fixed at that time, I still believe that it is a bad idea in terms of security to flush the buffer and forward its content to the backend before the *whole* request body has been received. At least I would recommend to carefully review this change to make sure you don't create a new security issue similar to the HRS issue. Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi Micha, On Thu, Oct 3, 2013 at 2:07 PM, Micha Lenk mi...@lenk.info wrote: Independent from how the HRS issue (CVE-2005-2088) was fixed at that time, I still believe that it is a bad idea in terms of security to flush the buffer and forward its content to the backend before the *whole* request body has been received. mod_proxy won't forward anything before the whole request *header* is received (it isn't even called before), with or without my patch(es), and that prevent HRS provided the httpd checks Content-Length/Transfer-Encoding against http://tools.ietf.org/html/draft-ietf-httpbis-p1-messaging-23#page-31(which is the case, at least in trunk). mod_proxy will also prevent HRS by checking that the received data do not exceed the announced size, again my patch(es) don't change that. It currently prefetches 16K of the body (if any) and forwards that and everything after to the backend from there, it won't retain the *whole* body unless one plays with the proxy-sendcl env. Do you mean you always setenv proxy-sendcl to force mod_proxy in full-fetching mode because of security issues ? At least I would recommend to carefully review this change to make sure you don't create a new security issue similar to the HRS issue. Sure, review and more testing is very welcome. I'm not sure what changes you are talking about, though. Is it about the flushall or the prefetch before connect patch ? For the first patch I don't see (as explained above) the changes about security, it is more about small network packet now vs big ones later. The latter case (the current default behaviour) also raises a backend timeout issue, when the client sends small but regular data fetched-but-not-flushed by the proxy (whether or not the bytes are the first ones) and the backend timeouts because it hasn't received anything since a while... For the second patch, do you mean pre-fetching (part of) the request body before connecting/reusing a backend could introduce a flaw ? You may be right but, IMHO, not for a HRS vulnerability (httpd/mod_proxy checks haven't been changed). Contrariwise, it avoids a DOS attempt on both the proxy and the backend (whose connections are requisitioned) should a client be slow (or Alice). Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
-sendchunked))) {
+ !apr_table_get(r-subprocess_env,
proxy-sendchunked)
+ !flushall)) {
rb_method = RB_SPOOL_CL;
}
else {
@@ -889,7 +908,8 @@ int ap_proxy_http_request(apr_pool_t *p, request_r
}
else if (!force10
(apr_table_get(r-subprocess_env, proxy-sendchunks)
- || apr_table_get(r-subprocess_env,
proxy-sendchunked))
+ || apr_table_get(r-subprocess_env,
proxy-sendchunked)
+ || flushall)
!apr_table_get(r-subprocess_env, proxy-sendcl)) {
rb_method = RB_STREAM_CHUNKED;
}
[/patch]
Regards,
Yann.
On Tue, Oct 1, 2013 at 2:36 PM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
-Ursprüngliche Nachricht-
Von: Joe Orton
Gesendet: Dienstag, 1. Oktober 2013 14:23
An: Plüm, Rüdiger, Vodafone Group
Cc: dev@httpd.apache.org
Betreff: Re: mod_proxy, oooled backend connections and the keep-alive
race condition
On Fri, Aug 02, 2013 at 12:33:58PM +, Plüm, Rüdiger, Vodafone Group
wrote:
The typical way to solve this today is to know the keepalive timeout
of the backend and set ttl for this worker to a value a few seconds
below.
I just looked at a case where the user is hitting this problem and does
already have the ttl keepalive-timeout configured like that; and a few
seconds difference is not necessarily enough. The problem is that the
prefetch of the 16K request body is making it a lot worse - the worst
case is (16K/packet size) * Timeout seconds to complete the prefetch.
True.
That's time when the proxy *thinks* the connection is valid but the
backend thinks the connection is idle. And in most reverse-proxy cases
that prefetch is adding basically no value AFAICT - the backend is a
known vintage and probably HTTP/1.1. So... could we make the prefetch
stuff configurable away?
IMHO no issue with this. Let's hear what others say.
I guess the main point of prefetch was to make better decisions whether
to use chunked
encoding when sending to the backend. Or provide a CL to the backend when
the real client does not.
Regards
Rüdiger
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Wed, Oct 2, 2013 at 9:40 AM, Thomas Eckert thomas.r.w.eck...@gmail.comwrote: Yann, although I do expect it to solve the issue discussed here, I don't think simply flushing everything instantly is the right way to go. For example, how do the proposed changes work with modules which scan the request body like mod_security ? A lot of scanning/parsing can only be done in a senseful way if the entity to be scanned/parsed is available in full. Indeed, this patch won't help if an input filter in the chain does spool the body. Maybe mod_proxy_http can partially prefetch the request body before the acquire/determine/connect backend logic so that the is_socket_connected() runs just before the first byte is sent (regardless of the time taken by the first input filters call) ? The flushall flag would still (optionally) be welcome :p Passing through everything immediately will probably cause the SlowHTTP to bybass the reverse proxy and harm the web server direclty. (I haven't had time to appl the changes to a test environment) SlowHTTP does currently eat Apache resources (the thread won't do anything else until all is forwarded to the backend, or even back to the client), though I don't know much about mpm_event still... IMHO that cannot be avoided without mod_reqtimeout (mod_security?). Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
A late (little) fix below...
On Thu, Oct 3, 2013 at 12:14 AM, Yann Ylavic ylavic@gmail.com wrote:
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c(revision 1528615)
+++ modules/proxy/proxy_util.c(working copy)
@@ -3092,7 +3092,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
buf = apr_pstrcat(p, r-method, , url, HTTP/1.1 CRLF, NULL);
}
if (apr_table_get(r-subprocess_env, proxy-nokeepalive)) {
-origin-keepalive = AP_CONN_CLOSE;
+if (origin) {
+origin-keepalive = AP_CONN_CLOSE;
+}
p_conn-close = 1;
}
ap_xlate_proto_to_ascii(buf, strlen(buf));
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1528615)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -677,30 +677,41 @@ static apr_status_t proxy_buckets_lifetime_transfo
return rv;
}
+enum rb_methods {
+RB_INIT,
+RB_STREAM_CL,
+RB_STREAM_CHUNKED,
+RB_SPOOL_CL
+};
+
static
-int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
+int ap_proxy_http_prefetch(apr_pool_t *p, request_rec *r,
proxy_conn_rec *p_conn, proxy_worker
*worker,
proxy_server_conf *conf,
apr_uri_t *uri,
- char *url, char *server_portstr)
+ char *url, char *server_portstr,
+ apr_bucket_brigade *header_brigade,
+ apr_bucket_brigade *input_brigade,
+ char **old_cl_val, char **old_te_val,
+ enum rb_methods *rb_method, int
*keepalive)
{
conn_rec *c = r-connection;
apr_bucket_alloc_t *bucket_alloc = c-bucket_alloc;
-apr_bucket_brigade *header_brigade;
-apr_bucket_brigade *input_brigade;
apr_bucket_brigade *temp_brigade;
apr_bucket *e;
char *buf;
apr_status_t status;
-enum rb_methods {RB_INIT, RB_STREAM_CL, RB_STREAM_CHUNKED,
RB_SPOOL_CL};
-enum rb_methods rb_method = RB_INIT;
-char *old_cl_val = NULL;
-char *old_te_val = NULL;
apr_off_t bytes_read = 0;
apr_off_t bytes;
int force10, rv;
-conn_rec *origin = p_conn-connection;
+/* XXX: This somehow should be fixed in the API.
+ * Assumes p_conn-close is 0 here since ap_proxy_create_hdrbrgd and
code
+ * below will use it to disable keep-alive and not for the connection
to
+ * be closed before reuse.
+ */
+AP_DEBUG_ASSERT(!p_conn-close);
+
if (apr_table_get(r-subprocess_env, force-proxy-request-1.0)) {
if (r-expecting_100) {
return HTTP_EXPECTATION_FAILED;
@@ -710,17 +721,13 @@ static
force10 = 0;
}
-header_brigade = apr_brigade_create(p, origin-bucket_alloc);
rv = ap_proxy_create_hdrbrgd(p, header_brigade, r, p_conn,
worker, conf, uri, url, server_portstr,
- old_cl_val, old_te_val);
+ old_cl_val, old_te_val);
if (rv != OK) {
return rv;
}
-/* We have headers, let's figure out our request body... */
-input_brigade = apr_brigade_create(p, bucket_alloc);
-
/* sub-requests never use keepalives, and mustn't pass request bodies.
* Because the new logic looks at input_brigade, we will
self-terminate
* input_brigade and jump past all of the request body logic...
@@ -733,15 +740,15 @@ static
if (!r-kept_body r-main) {
/* XXX: Why DON'T sub-requests use keepalives? */
p_conn-close = 1;
-if (old_cl_val) {
-old_cl_val = NULL;
+if (*old_cl_val) {
+*old_cl_val = NULL;
apr_table_unset(r-headers_in, Content-Length);
}
-if (old_te_val) {
-old_te_val = NULL;
+if (*old_te_val) {
+*old_te_val = NULL;
apr_table_unset(r-headers_in, Transfer-Encoding);
}
-rb_method = RB_STREAM_CL;
+*rb_method = RB_STREAM_CL;
e = apr_bucket_eos_create(input_brigade-bucket_alloc);
APR_BRIGADE_INSERT_TAIL(input_brigade, e);
goto skip_body;
@@ -755,20 +762,19 @@ static
* encoding has been done by the extensions' handler, and
* do not modify add_te_chunked's logic
*/
-if (old_te_val strcasecmp(old_te_val, chunked) != 0) {
+if (*old_te_val strcasecmp(*old_te_val, chunked) != 0) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01093)
%s Transfer-Encoding is not supported,
old_te_val);
return
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Is the Step Three-and-a-Half really required since
ap_proxy_connect_backend() (that does the same is_socket_connected() check)
is run almost before? May the ap_proxy_connection_create() function in
between take some time or is it a last chance to catch the race?
Regards
On Thu, Oct 3, 2013 at 12:38 AM, Yann Ylavic ylavic@gmail.com wrote:
A late (little) fix below...
On Thu, Oct 3, 2013 at 12:14 AM, Yann Ylavic ylavic@gmail.com wrote:
Index: modules/proxy/proxy_util.c
===
--- modules/proxy/proxy_util.c(revision 1528615)
+++ modules/proxy/proxy_util.c(working copy)
@@ -3092,7 +3092,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_poo
buf = apr_pstrcat(p, r-method, , url, HTTP/1.1 CRLF,
NULL);
}
if (apr_table_get(r-subprocess_env, proxy-nokeepalive)) {
-origin-keepalive = AP_CONN_CLOSE;
+if (origin) {
+origin-keepalive = AP_CONN_CLOSE;
+}
p_conn-close = 1;
}
ap_xlate_proto_to_ascii(buf, strlen(buf));
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1528615)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -677,30 +677,41 @@ static apr_status_t proxy_buckets_lifetime_transfo
return rv;
}
+enum rb_methods {
+RB_INIT,
+RB_STREAM_CL,
+RB_STREAM_CHUNKED,
+RB_SPOOL_CL
+};
+
static
-int ap_proxy_http_request(apr_pool_t *p, request_rec *r,
+int ap_proxy_http_prefetch(apr_pool_t *p, request_rec *r,
proxy_conn_rec *p_conn, proxy_worker
*worker,
proxy_server_conf *conf,
apr_uri_t *uri,
- char *url, char *server_portstr)
+ char *url, char *server_portstr,
+ apr_bucket_brigade *header_brigade,
+ apr_bucket_brigade *input_brigade,
+ char **old_cl_val, char **old_te_val,
+ enum rb_methods *rb_method, int
*keepalive)
{
conn_rec *c = r-connection;
apr_bucket_alloc_t *bucket_alloc = c-bucket_alloc;
-apr_bucket_brigade *header_brigade;
-apr_bucket_brigade *input_brigade;
apr_bucket_brigade *temp_brigade;
apr_bucket *e;
char *buf;
apr_status_t status;
-enum rb_methods {RB_INIT, RB_STREAM_CL, RB_STREAM_CHUNKED,
RB_SPOOL_CL};
-enum rb_methods rb_method = RB_INIT;
-char *old_cl_val = NULL;
-char *old_te_val = NULL;
apr_off_t bytes_read = 0;
apr_off_t bytes;
int force10, rv;
-conn_rec *origin = p_conn-connection;
+/* XXX: This somehow should be fixed in the API.
+ * Assumes p_conn-close is 0 here since ap_proxy_create_hdrbrgd and
code
+ * below will use it to disable keep-alive and not for the
connection to
+ * be closed before reuse.
+ */
+AP_DEBUG_ASSERT(!p_conn-close);
+
if (apr_table_get(r-subprocess_env, force-proxy-request-1.0)) {
if (r-expecting_100) {
return HTTP_EXPECTATION_FAILED;
@@ -710,17 +721,13 @@ static
force10 = 0;
}
-header_brigade = apr_brigade_create(p, origin-bucket_alloc);
rv = ap_proxy_create_hdrbrgd(p, header_brigade, r, p_conn,
worker, conf, uri, url, server_portstr,
- old_cl_val, old_te_val);
+ old_cl_val, old_te_val);
if (rv != OK) {
return rv;
}
-/* We have headers, let's figure out our request body... */
-input_brigade = apr_brigade_create(p, bucket_alloc);
-
/* sub-requests never use keepalives, and mustn't pass request
bodies.
* Because the new logic looks at input_brigade, we will
self-terminate
* input_brigade and jump past all of the request body logic...
@@ -733,15 +740,15 @@ static
if (!r-kept_body r-main) {
/* XXX: Why DON'T sub-requests use keepalives? */
p_conn-close = 1;
-if (old_cl_val) {
-old_cl_val = NULL;
+if (*old_cl_val) {
+*old_cl_val = NULL;
apr_table_unset(r-headers_in, Content-Length);
}
-if (old_te_val) {
-old_te_val = NULL;
+if (*old_te_val) {
+*old_te_val = NULL;
apr_table_unset(r-headers_in, Transfer-Encoding);
}
-rb_method = RB_STREAM_CL;
+*rb_method = RB_STREAM_CL;
e = apr_bucket_eos_create(input_brigade-bucket_alloc);
APR_BRIGADE_INSERT_TAIL(input_brigade, e);
goto skip_body;
@@ -755,20 +762,19 @@ static
* encoding has been done by the extensions' handler, and
* do
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Fri, Aug 02, 2013 at 12:33:58PM +, Plüm, Rüdiger, Vodafone Group wrote: The typical way to solve this today is to know the keepalive timeout of the backend and set ttl for this worker to a value a few seconds below. I just looked at a case where the user is hitting this problem and does already have the ttl keepalive-timeout configured like that; and a few seconds difference is not necessarily enough. The problem is that the prefetch of the 16K request body is making it a lot worse - the worst case is (16K/packet size) * Timeout seconds to complete the prefetch. That's time when the proxy *thinks* the connection is valid but the backend thinks the connection is idle. And in most reverse-proxy cases that prefetch is adding basically no value AFAICT - the backend is a known vintage and probably HTTP/1.1. So... could we make the prefetch stuff configurable away? Regards, Joe
Re: mod_proxy, oooled backend connections and the keep-alive race condition
, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: -Ursprüngliche Nachricht- Von: Joe Orton Gesendet: Dienstag, 1. Oktober 2013 14:23 An: Plüm, Rüdiger, Vodafone Group Cc: dev@httpd.apache.org Betreff: Re: mod_proxy, oooled backend connections and the keep-alive race condition On Fri, Aug 02, 2013 at 12:33:58PM +, Plüm, Rüdiger, Vodafone Group wrote: The typical way to solve this today is to know the keepalive timeout of the backend and set ttl for this worker to a value a few seconds below. I just looked at a case where the user is hitting this problem and does already have the ttl keepalive-timeout configured like that; and a few seconds difference is not necessarily enough. The problem is that the prefetch of the 16K request body is making it a lot worse - the worst case is (16K/packet size) * Timeout seconds to complete the prefetch. True. That's time when the proxy *thinks* the connection is valid but the backend thinks the connection is idle. And in most reverse-proxy cases that prefetch is adding basically no value AFAICT - the backend is a known vintage and probably HTTP/1.1. So... could we make the prefetch stuff configurable away? IMHO no issue with this. Let's hear what others say. I guess the main point of prefetch was to make better decisions whether to use chunked encoding when sending to the backend. Or provide a CL to the backend when the real client does not. Regards Rüdiger httpd-trunk-mod_proxy_http_flushall.patch Description: Binary data
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Tue, Oct 1, 2013 at 3:39 PM, Yann Ylavic ylavic@gmail.com wrote: It should address this issue, and in the same time allow protocols like MS RPC-over-HTTP to work with : SetEnvIf Request_Method ^RPC_IN_DATA proxy-flushall See PR40029 for RPC-over-HTTP debate, it was finally RESOLVED INVALID, which is not obvious... Sorry if that's off topic though, just to point another side-effect caused by the prefetch. Regards, Yann.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Oct 1, 2013, at 8:36 AM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: -Ursprüngliche Nachricht- Von: Joe Orton Gesendet: Dienstag, 1. Oktober 2013 14:23 An: Plüm, Rüdiger, Vodafone Group Cc: dev@httpd.apache.org Betreff: Re: mod_proxy, oooled backend connections and the keep-alive race condition That's time when the proxy *thinks* the connection is valid but the backend thinks the connection is idle. And in most reverse-proxy cases that prefetch is adding basically no value AFAICT - the backend is a known vintage and probably HTTP/1.1. So... could we make the prefetch stuff configurable away? IMHO no issue with this. Let's hear what others say. +1... fine w/ me I guess the main point of prefetch was to make better decisions whether to use chunked encoding when sending to the backend. Or provide a CL to the backend when the real client does not.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Hi all, Am 01.10.2013 14:36, schrieb Plüm, Rüdiger, Vodafone Group: That's time when the proxy *thinks* the connection is valid but the backend thinks the connection is idle. And in most reverse-proxy cases that prefetch is adding basically no value AFAICT - the backend is a known vintage and probably HTTP/1.1. So... could we make the prefetch stuff configurable away? IMHO no issue with this. Let's hear what others say. I guess the main point of prefetch was to make better decisions whether to use chunked encoding when sending to the backend. Or provide a CL to the backend when the real client does not. As far as I understand the issue, the main point of prefetch was to fix CVE-2005-2088, a HTTP Request Smuggling attack (see also http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-2088). This is not an argument against making the prefetch stuff configurable, but if this ever gets implemented, this CVE should definitely needs to be mentioned in the documentation so that users are aware of it. Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Tue, Oct 1, 2013 at 4:53 PM, Micha Lenk mi...@lenk.info wrote: As far as I understand the issue, the main point of prefetch was to fix CVE-2005-2088, a HTTP Request Smuggling attack (see also http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2005-2088). This is discussed in PR40029 and is not related to HRS, the real fix regarding HRS was about both CL/TE sent by th client ( https://issues.apache.org/bugzilla/show_bug.cgi?id=40029#c4). Regards, Micha
Re: mod_proxy, oooled backend connections and the keep-alive race condition
One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. I don't see why an OPTIONS request should not encounter problems where a GET request will. After all, the problem is on the transport layer, not on the application layer. Am I missing something ? Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though. This is pretty much what I stated earlier. So I assume (re)opening a connection and having the whole process of sending the request transition to that (re)new(ed) connection is possible for an input filter. I was not sure about it. Going to look into it once I have time.. On Sat, Aug 3, 2013 at 7:26 PM, Stefan Fritsch s...@sfritsch.de wrote: Am Freitag, 2. August 2013, 11:21:56 schrieb Eric Covener: I think this does not work for GET requests or request without a request body. Just re-read spec, you are right -- we are abusing this in a module as a sort of extended handshake even w/ no body, but not against heterogenous backends. One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Mon, Aug 5, 2013 at 2:49 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. I don't see why an OPTIONS request should not encounter problems where a GET request will. After all, the problem is on the transport layer, not on the application layer. Am I missing something ? Could have problems at either level (e.g. MaxKeepalives)
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On 05.08.2013 13:18, Eric Covener wrote: On Mon, Aug 5, 2013 at 2:49 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. I don't see why an OPTIONS request should not encounter problems where a GET request will. After all, the problem is on the transport layer, not on the application layer. Am I missing something ? Could have problems at either level (e.g. MaxKeepalives) I think what people are trying to say: another request in front of each request might increase the relative frequency (per real request) with which the problem occurs. Regards, Rainer
Re: mod_proxy, oooled backend connections and the keep-alive race condition
On Aug 5, 2013, at 10:13 AM, Rainer Jung rainer.j...@kippdata.de wrote: On 05.08.2013 13:18, Eric Covener wrote: On Mon, Aug 5, 2013 at 2:49 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. I don't see why an OPTIONS request should not encounter problems where a GET request will. After all, the problem is on the transport layer, not on the application layer. Am I missing something ? Could have problems at either level (e.g. MaxKeepalives) I think what people are trying to say: another request in front of each request might increase the relative frequency (per real request) with which the problem occurs. +1
Re: mod_proxy, oooled backend connections and the keep-alive race condition
I've tried looking into that, and I found it more trouble than it was worth... (I'm sure that the list archives have posts about the 'http ping' tests). The problem is that the OPTIONS request could be that request that kicks the backend from being a keptalive connection to closing it. :/ On Aug 3, 2013, at 1:26 PM, Stefan Fritsch s...@sfritsch.de wrote: Am Freitag, 2. August 2013, 11:21:56 schrieb Eric Covener: I think this does not work for GET requests or request without a request body. Just re-read spec, you are right -- we are abusing this in a module as a sort of extended handshake even w/ no body, but not against heterogenous backends. One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Am Freitag, 2. August 2013, 11:21:56 schrieb Eric Covener: I think this does not work for GET requests or request without a request body. Just re-read spec, you are right -- we are abusing this in a module as a sort of extended handshake even w/ no body, but not against heterogenous backends. One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
+1 for the theory, but I'm not sure if it's feasible or not. On Aug 2, 2013, at 5:28 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not-pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Yes, the theory thing ... I wish I could have added an experimental patch for such an input filter but I'm afraid that might take a long time for me to finish. I'll try though I hope someone more knowledgeable will pick this up. On Fri, Aug 2, 2013 at 2:28 PM, Jim Jagielski j...@jagunet.com wrote: +1 for the theory, but I'm not sure if it's feasible or not. On Aug 2, 2013, at 5:28 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not-pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
True enough, but that's inelegant ;) On Aug 2, 2013, at 8:33 AM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: The typical way to solve this today is to know the keepalive timeout of the backend and set ttl for this worker to a value a few seconds below. Regards Rüdiger -Ursprüngliche Nachricht- Von: Jim Jagielski Gesendet: Freitag, 2. August 2013 14:29 An: dev@httpd.apache.org Betreff: Re: mod_proxy, oooled backend connections and the keep-alive race condition +1 for the theory, but I'm not sure if it's feasible or not. On Aug 2, 2013, at 5:28 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not- pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
+1 for the theory, but I'm not sure if it's feasible or not. Some other ideas: * Does something like a select/poll/getpeername() ever tell us of an error when a write() doesn't? * Could we give people an Expect: 100-continue option on the initial client request?
Re: mod_proxy, oooled backend connections and the keep-alive race condition
I think this does not work for GET requests or request without a request body. Just re-read spec, you are right -- we are abusing this in a module as a sort of extended handshake even w/ no body, but not against heterogenous backends.
