Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
minf...@apache.org wrote: Author: minfrin Date: Tue May 7 20:27:37 2013 New Revision: 1480058 URL: http://svn.apache.org/r1480058 Log: mod_proxy: Ensure network errors detected by the proxy are returned as 504 Gateway Timout as opposed to 502 Bad Gateway, in order to be compliant with RFC2616 14.9.4 Cache Revalidation and Reload Controls. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/modules/proxy/mod_proxy_ftp.c httpd/httpd/trunk/modules/proxy/mod_proxy_http.c httpd/httpd/trunk/modules/proxy/proxy_util.c I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate. If this is not the case IMHO 10.5.3 and 10.5.5 apply. And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules as our proxy modules. So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures. 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request. Regards Rüdiger
Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
On 08 May 2013, at 9:47 AM, Ruediger Pluem rpl...@apache.org wrote: I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate. If this is not the case IMHO 10.5.3 and 10.5.5 apply. And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules as our proxy modules. So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures. 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request. Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client. The way I interpret the RFC is that received an invalid response described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a timely response which means at the appropriate time, meaning a network error of some kind. The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp. Ultimately Roy is the authority on this? Regards, Graham -- smime.p7s Description: S/MIME cryptographic signature
Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
Graham Leggett wrote: On 08 May 2013, at 9:47 AM, Ruediger Pluem rpl...@apache.org wrote: I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate. If this is not the case IMHO 10.5.3 and 10.5.5 apply. And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules as our proxy modules. So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures. 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request. Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client. The way I interpret the RFC is that received an invalid response described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a timely response which means at the appropriate time, meaning a network error of some kind. The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp. Ultimately Roy is the authority on this? Good idea. He also knows possible clarifications on that that are work in progress and that we could already apply. Roy can you please help us here? Regards Rüdiger
Re: Improve mod_proxy's error marking of workers
I think you need to screen out 4xx at least to prevent client errors from marking down your backends. Seems like those responses never reach that code. Neither 400 nor 401 or 403 had an effect on the worker's error state, it always remained in state 'OK'. I'd like to say it's not an issue but until I know why it behaves like this I would agree with you. Need to screen them out unless we can be 100% sure not to encounter those status codes at that point. Agreed... An all or nothing setting will likely create more trouble than not. In my opinion this should not even be configurable, at least not when other error related response codes lead to having workers set to state 'ERROR'. I only added this so it could be applied to existing setups without causing havoc. Ideally this should be part of the code just like the other worker state changes (some lines above my suggested change). I really don't see why some errors should cause the worker to display as such while others don't. This of course includes Eric's reply regarding 4xx codes which should not have the worker's state be changed. This patch addresses corner cases (e.g. DNS resolution failure) but in those cases mod_proxy should not just be silent about it. If such errors occur let the system know about so it can be fixed / worked around. It really does not matter whether this is done as directive via the suggested patch, via failonstatus or some other way as long as it's there. On Wed, May 8, 2013 at 1:18 AM, Daniel Ruggeri drugg...@primary.net wrote: On 5/7/2013 2:00 PM, Jim Jagielski wrote: Agreed... An all or nothing setting will likely create more trouble than not. On May 7, 2013, at 8:08 AM, Eric Covener cove...@gmail.com wrote: On Tue, May 7, 2013 at 6:24 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Attached patch contains a directive to improve the error marking of workers. Basically, some errors will cause a worker to be marked as in error while others don't. I can't see a reason for this so I added a directive to have all errors mark the error correctly - especially useful for automated systems using mod_proxy to check if the system is healthy. I think you need to screen out 4xx at least to prevent client errors from marking down your backends. There is also failonstatus which, granted, could probably be enhanced to accept ranges instead of just a list to accommodate this need. -- Daniel Ruggeri
Re: help with fixing a mod_auth_form bug regarding kept_body
On 05/03/2013 05:28 PM, David Mansfield wrote: I'm hitting this bug (I think): https://issues.apache.org/bugzilla/show_bug.cgi?id=53692 This bug makes the mode described in the docs. for mod_auth_form called Inline with Body Preservation impossible, because it's impossible to access the POSTed data from inside the login handler. (I've tried using a cgi like /cgi-bin/login.cgi, and via mod_include using smth like /login.shtml and KeptBodySize). The magic, or lack thereof, occurs at around line 979 of mod_auth_form.c. The code sets up a subrequest and uses that to ap_parse_form_data(). The kept_body_filter is not invoked until after this happens, which could be the crux of the problem. Unfortunately I don't know much about apache internals and perhaps a few minutes of expert guidance could point me in the right direction for how to fix this. It doesn't seem that ap_parse_form_data has any provision for saving the brigade anywhere so I imagine this will have to be done via a kept_body, but I'm really lost at this point. Any ideas? I've tried extensively to debug this - but it seems the kept_body is not passed properly to the request in internal_internal_redirect and that also the f-ctx (filter context variable) is not passed on either, and without these two things it is hopeless. Maybe this is intentional for ErrorDocument handling, but it makes this particular (documented) feature completely broken AFACT. If there's someone on the list that can provide any feedback whatsoever it would be really helpful. Can anyone confirm that inline login with body preservation has ever worked in any situation for them?
Re: disable pid file writing?
On Mon, 6 May 2013 23:42:27 +0100 Tom Jones t...@inpher.com wrote: We use process supervision and don't have a use for pid files. We are running multiple httpd instances, and have the config management to create a writable configured place for each instance to put its pid file. Surely you have the very same problem with the various fcntl/mutex and disk-backed shared mem segments, as well as log files? This seems fairly incidental to the big picture. This config management has some ongoing cost to maintain, and we would find it nicer if we could just disable pid files in our apache httpd configurations. Of course apachectl requires this, but it's pretty clear you aren't using apachectl in the first place... Before going further, I just wanted to get a feel for what others think of the idea of geing able to disable pid files. If added, should it be done with a special value given to the PidFile directive (such as the empty string), or should a new directive be introduced such as DisablePidFile? No, another directive simply confused the issue. Wouldn't /dev/null be the appropriate PidFile value, with special case handling of that specific string?
Re: disable pid file writing?
On 08.05.2013 20:06, William A. Rowe Jr. wrote: On Mon, 6 May 2013 23:42:27 +0100 Tom Jones t...@inpher.com wrote: We use process supervision and don't have a use for pid files. We are running multiple httpd instances, and have the config management to create a writable configured place for each instance to put its pid file. Surely you have the very same problem with the various fcntl/mutex and disk-backed shared mem segments, as well as log files? This seems fairly incidental to the big picture. This config management has some ongoing cost to maintain, and we would find it nicer if we could just disable pid files in our apache httpd configurations. Of course apachectl requires this, but it's pretty clear you aren't using apachectl in the first place... Before going further, I just wanted to get a feel for what others think of the idea of geing able to disable pid files. If added, should it be done with a special value given to the PidFile directive (such as the empty string), or should a new directive be introduced such as DisablePidFile? No, another directive simply confused the issue. Wouldn't /dev/null be the appropriate PidFile value, with special case handling of that specific string? Careful: I didn't test it but we delete the pid file during web server shutdown. That might remove /dev/null then. On a quick look through the code I had the impression you can not easily get rid of the pid file. Regards, Rainer
Re: svn commit: r1480058 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy_ftp.c modules/proxy/mod_proxy_http.c modules/proxy/proxy_util.c
On May 8, 2013, at 1:11 AM, Ruediger Pluem wrote: Graham Leggett wrote: On 08 May 2013, at 9:47 AM, Ruediger Pluem rpl...@apache.org wrote: I don't agree with this. The case you mention is only true if the client sends Cache-Control: must-revalidate. If this is not the case IMHO 10.5.3 and 10.5.5 apply. And only a cache is required to respond with 504 in this case, not a gateway or a proxy. So the cache should change a 502 to a 504 in case the revalidation fails. Imagine the case where you have other backend modules as our proxy modules. So while changing the response to 504 for failed DNS lookups is always correct it is not for other failures. 10.5.3: The server, while acting as a gateway or proxy, received an invalid response from the upstream server it accessed in attempting to fulfill the request. Arbitrarily changing a 502 to a 504 on the fly will confuse people, as this server isn't the only server that could generate a 502, and 502 might be the intended error to be sent to the client. The way I interpret the RFC is that received an invalid response described by 10.5.3 occurs when the upstream has said something that the proxy doesn't like, while the unfortunately named 10.5.5 504 Gateway Timed Out describes a timely response which means at the appropriate time, meaning a network error of some kind. The change above isolates network errors specifically and returns 504 in those cases only, while leaving 502 for genuine protocol errors, mostly affecting ftp. Ultimately Roy is the authority on this? Good idea. He also knows possible clarifications on that that are work in progress and that we could already apply. Roy can you please help us here? Unfortunately, I am at the tail end of a long standards meeting and haven't slept much for three days. Have you checked to see if the descriptions changed in HTTPbis p6? RFC2616 hasn't been relevant for a while now. http://svn.tools.ietf.org/svn/wg/httpbis/draft-ietf-httpbis/latest/p6-cache.html I'll look at it tomorrow when I have slept a bit. Roy
Re: disable pid file writing?
On 5/8/2013 3:29 PM, Rainer Jung wrote: Careful: I didn't test it but we delete the pid file during web server shutdown. That might remove /dev/null then. On a quick look through the code I had the impression you can not easily get rid of the pid file. Agreed - setting to /dev/null under the current code also fails startup anyway with the following error: (20014)Internal error: Error retrieving pid file /dev/null Remove it before continuing if it is corrupted. I haven't looked into it any further than that, though. -- Daniel Ruggeri
