You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2006/09/13 21:33:46 UTC
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=rev&rev=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=diff&rev=442758&r1=442757&r2=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
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Nick Kew wrote:
>
> 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.
At minimum report OK. The request worked, the client fell away. Admins go
crazy tracking down 500.
Maybe maybe an internal class of non-HTTP results such as 599 for
CLIENT_GONE_AWAY or something, but certainly not the conventional 500 codes
which represent broken activity on the server side.
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators: mod_cgi.c mod_cgid.c
Posted by Jeff Trawick <tr...@gmail.com>.
On 9/13/06, Nick Kew <ni...@webthing.com> wrote:
> On Wednesday 13 September 2006 22:31, Jeff Trawick wrote:
> > On 9/13/06, Nick Kew <ni...@webthing.com> 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
Posted by Nick Kew <ni...@webthing.com>.
On Wednesday 13 September 2006 22:31, Jeff Trawick wrote:
> On 9/13/06, Nick Kew <ni...@webthing.com> 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
Re: svn commit: r442758 - in /httpd/httpd/trunk/modules/generators:
mod_cgi.c mod_cgid.c
Posted by Ruediger Pluem <rp...@apache.org>.
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
Posted by Jeff Trawick <tr...@gmail.com>.
On 9/13/06, Nick Kew <ni...@webthing.com> 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
Posted by Nick Kew <ni...@webthing.com>.
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