Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On 9/13/06, Nick Kew [EMAIL PROTECTED] wrote: On Wednesday 13 September 2006 22:31, Jeff Trawick wrote: On 9/13/06, Nick Kew [EMAIL PROTECTED] wrote: On Wednesday 13 September 2006 20:33, Ruediger Pluem wrote: Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that c-aborted is set, just like in the default handler? I'm not sure. Presumably if c-aborted is set, then we have no client to respond to, so this is just about housekeeping and what ends up in the logs. Do we want to log a successful POST or PUT when it wasn't? Here is my understanding: The connection status (%c) is what the admin should check to confirm that there were no network I/O issues (at least none that caused TCP to give us an error up through the point when the request was complete). In many cases, an HTTP status code has already been written to the client before the I/O problem occurs anyway so changing the status code doesn't make sense. A failure to read a request body would be prior to the point where we could write a status code, but I don't see why the log analysis heuristic should be different. So we should log an error, not a success. %c already handles this. 500 won't always be the ideal error, but I don't really see how we can do better within the current API. 500 implies that there could be an action to take to resolve a problem (e.g., screwy filters bungled the return codes; screwy configuration; out of memory; ???). It doesn't apply when somebody bored with an upload hit the Stop button. So are you supporting Rüdiger's proposition? I can accept that if it's the popular view. Yes, I am supporting Rüdiger's proposition. Don't make up some HTTP status code for the aborted-connection condition. We already have a way to record this issue (%c).
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
-Ursprüngliche Nachricht- Von: Jeff Trawick Yes, I am supporting Rüdiger's proposition. Don't make up some HTTP status code for the aborted-connection condition. We already have a way to record this issue (%c). I guess by %c you mean what is now %X in, mod_log_config, right? As said previously it seems to make sense to me to log a 408 when we know that a timeout caused this trouble. Honestly I currently do not know how we can notice this (maybe the return value from the filter can be examined). Regards Rüdiger
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On 9/14/06, Plüm, Rüdiger, VF EITO [EMAIL PROTECTED] wrote:
-Ursprüngliche Nachricht-
Von: Jeff Trawick
Yes, I am supporting Rüdiger's proposition. Don't make up some HTTP
status code for the aborted-connection condition. We already have a
way to record this issue (%c).
I guess by %c you mean what is now %X in, mod_log_config, right?
whoops, %c is the 1.3 format string for this; as the current docs for %X say:
(This directive was %...c in late versions of Apache 1.3, but this
conflicted with the historical ssl %...{var}c syntax.)
As said previously it seems to make sense to me to log a 408 when we know that
a timeout caused this trouble. Honestly I currently do not know how we can
notice
this (maybe the return value from the filter can be examined).
Okay, I buy that.
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On 09/13/2006 01:44 AM, wrote:
Author: niq
Date: Tue Sep 12 16:44:12 2006
New Revision: 442758
URL: http://svn.apache.org/viewvc?view=revrev=442758
Log:
PR 31759 (mutated) - reported by Jo Rhett
Don't return apr_status_t error value from input filter chain.
Modified:
httpd/httpd/trunk/modules/generators/mod_cgi.c
httpd/httpd/trunk/modules/generators/mod_cgid.c
Modified: httpd/httpd/trunk/modules/generators/mod_cgi.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/generators/mod_cgi.c?view=diffrev=442758r1=442757r2=442758
==
--- httpd/httpd/trunk/modules/generators/mod_cgi.c (original)
+++ httpd/httpd/trunk/modules/generators/mod_cgi.c Tue Sep 12 16:44:12 2006
@@ -837,7 +837,9 @@
APR_BLOCK_READ, HUGE_STRING_LEN);
if (rv != APR_SUCCESS) {
-return rv;
+ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r,
+ Error reading request entity data);
+return HTTP_INTERNAL_SERVER_ERROR;
}
Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that
c-aborted is set,
just like in the default handler?
Regards
Rüdiger
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On Wednesday 13 September 2006 20:33, Ruediger Pluem wrote: Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that c-aborted is set, just like in the default handler? I'm not sure. Presumably if c-aborted is set, then we have no client to respond to, so this is just about housekeeping and what ends up in the logs. Do we want to log a successful POST or PUT when it wasn't? -- Nick Kew
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On 09/13/2006 10:17 PM, Nick Kew wrote: On Wednesday 13 September 2006 20:33, Ruediger Pluem wrote: Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that c-aborted is set, just like in the default handler? I'm not sure. Presumably if c-aborted is set, then we have no client to respond to, so this is just about housekeeping and what ends up in the logs. Do we want to log a successful POST or PUT when it wasn't? I guess currently this is a problem, because no meaningful r-status value is set by the core network filters in the case that c-aborted comes true. In the case of a timeout this could be a 408. I am not sure about the correct code if the network connection is just closed by the client. Any RFC guru an idea? OTH I am not sure if returning an INTERNAL_SERVER_ERROR is correct either when the client aborted the connection. Regards Rüdiger
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On 9/13/06, Nick Kew [EMAIL PROTECTED] wrote: On Wednesday 13 September 2006 20:33, Ruediger Pluem wrote: Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that c-aborted is set, just like in the default handler? I'm not sure. Presumably if c-aborted is set, then we have no client to respond to, so this is just about housekeeping and what ends up in the logs. Do we want to log a successful POST or PUT when it wasn't? Here is my understanding: The connection status (%c) is what the admin should check to confirm that there were no network I/O issues (at least none that caused TCP to give us an error up through the point when the request was complete). In many cases, an HTTP status code has already been written to the client before the I/O problem occurs anyway so changing the status code doesn't make sense. A failure to read a request body would be prior to the point where we could write a status code, but I don't see why the log analysis heuristic should be different. 500 implies that there could be an action to take to resolve a problem (e.g., screwy filters bungled the return codes; screwy configuration; out of memory; ???). It doesn't apply when somebody bored with an upload hit the Stop button.
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
On Wednesday 13 September 2006 22:31, Jeff Trawick wrote: On 9/13/06, Nick Kew [EMAIL PROTECTED] wrote: On Wednesday 13 September 2006 20:33, Ruediger Pluem wrote: Wouldn't it make sense to return OK even if rv != APR_SUCCESS in the case that c-aborted is set, just like in the default handler? I'm not sure. Presumably if c-aborted is set, then we have no client to respond to, so this is just about housekeeping and what ends up in the logs. Do we want to log a successful POST or PUT when it wasn't? Here is my understanding: The connection status (%c) is what the admin should check to confirm that there were no network I/O issues (at least none that caused TCP to give us an error up through the point when the request was complete). In many cases, an HTTP status code has already been written to the client before the I/O problem occurs anyway so changing the status code doesn't make sense. A failure to read a request body would be prior to the point where we could write a status code, but I don't see why the log analysis heuristic should be different. So we should log an error, not a success. 500 won't always be the ideal error, but I don't really see how we can do better within the current API. 500 implies that there could be an action to take to resolve a problem (e.g., screwy filters bungled the return codes; screwy configuration; out of memory; ???). It doesn't apply when somebody bored with an upload hit the Stop button. So are you supporting Rüdiger's proposition? I can accept that if it's the popular view. -- Nick Kew Application Development with Apache - the Apache Modules Book http://www.prenhallprofessional.com/title/0132409674
