Bug 36090 - CGI Status: header triggers recursive ErrorDocument handling
Summary: CGI Status: header triggers recursive ErrorDocument handling
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.2.0
Hardware: All All
: P3 minor (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2005-08-09 03:17 UTC by Chris Darroch
Modified: 2005-12-09 04:00 UTC (History)
1 user (show)



Attachments
suggested patch to ap_process_request() (300 bytes, patch)
2005-08-09 03:19 UTC, Chris Darroch
Details | Diff
for 2.2.0 (296 bytes, patch)
2005-12-02 16:01 UTC, Chris Darroch
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Darroch 2005-08-09 03:17:35 UTC
If a CGI script outputs a Status: header with a value other than 200, and
then generates malformed headers, the message that is returned to the
client includes the bogus statement that "Additionally, a 500
Internal Server Error error was encountered while trying to use
an ErrorDocument to handle the request" even if no ErrorDocument directives
are present in the server configuration files.

For example, with no ErrorDocuments defined, the following Perl CGI script:

#!/usr/bin/perl
print "Status: 404\n";
print "FOO\n\n";

produces this HTML output:

Internal Server Error
The server encountered an internal error or misconfiguration and was
unable to complete your request.
...
Additionally, a 500 Internal Server Error error was encountered while
trying to use an ErrorDocument to handle the request.

This bug occurs with both Apache 1.3 and 2.x (it was tested using 1.3.33 and
2.1.6-alpha).

The source of the problem appears to be in ap_process_request() in
modules/http/http_request.c.  I will follow up this bug report with
a proposed patch which addresses the issue described below.

The ap_process_request() function is used by the main "process_connection"
hook functions in modules/http/http_core.c.  The ap_process_request()
function calls ap_invoke_handler(), which calls ap_run_handler(), which
invokes the correct "handler" hook function for a given request.  In the
case of a request that maps to a CGI script, this is either cgi_handler()
or cgid_handler() in modules/generators/mod_cgi.c or mod_cgid.c,
respectively.

Those "handler" hook functions both use ap_scan_script_header_err_brigade()
to parse the HTTP headers output by a CGI script.  That function is
a wrapper around ap_scan_script_header_err_core() in server/util_script.c,
which does the actual parsing.  If ap_scan_script_header_err_core()
sees a valid Status: header line in the script's output, it sets
r->status accordingly.  However, it may then encounter an error, for
example, if it finds a malformed HTTP header.  In the case of the
sample CGI script shown above, the "FOO" header line causes cgi_handler()
or cgid_handler() to receive back an HTTP_INTERNAL_SERVER_ERROR return
code from ap_scan_script_header_err_brigade().  They pass that back
through their local log_script() function (in mod_cgi.c or mod_cgid.c)
to ap_run_handler(), which passes it back through ap_invoke_handler()
to ap_process_request().

When ap_process_request() does not receive OK or DONE fro
ap_invoke_handler() it calls ap_die(), also in modules/http/http_request.c.
The ap_die() function is called from various places, mostly in the
modules/http/* and modules/proxy/* code, in order to terminate the
current request.  It determines whether an ErrorDocument directive
exists for a given error code; if one does and it maps to a local
URL (beginning with a leading / character), then it performs an
internal redirection using ap_internal_redirect() in the same file.
If the sub-request that is generated causes another error, then ap_die()
will be called recursively.

When this occurs, ap_internal_redirect() needs to back out of the
situation, use the default error message for the original error code,
and amend it with a note about the secondary error code from the
internal redirection.  To do this, it examines r->status for a value
other than HTTP_OK; if another value is found, then ap_die() assumes
that it has been called from within a failing internal redirection
for an ErrorDocument directive with a local URL.  It then recovers
the initial request data, prevents any handling of customized error
messages with ErrorDocument directives, and calls ap_send_error_response()
in modules/http/http_protocol.c with its recursive_error parameter set.
This function is responsible for sending back the appropriate
error message to the client, along with the additional information about
the recursive_error parameter.

The problem described in this bug report occurs because if r->status
has been set to a value other than HTTP_OK by
ap_scan_script_header_err_core() -- or any other function invoked
during request handling -- then ap_die() treats that as a signal
that a recursive error is in progress, although this is not true.

The proposed patch amends the ap_process_request() function by resetting
r->status to HTTP_OK just before calling ap_die().

No changes are made to ap_internal_redirect() or
ap_internal_redirect_handler() in the same modules/http/http_request.c
file, although they have similar blocks of code.  This is because these
functions are used in various places to perform internal redirections,
copying most of the original request's data, including its r->status
value.  This includes ap_die() itself, which uses ap_internal_redirect()
to handle local URLs in ErrorDocument directives.

The assumption is therefore made that any caller of these two internal
redirection functions knows that the r->status value of the initial
request has an appropriate value (i.e., HTTP_OK) before invoking them.
If this is not true, then other similar bugs may exist elsewhere in
the code; however, a quick scan suggests that some users of these
functions do this.  For example, asis_handler() in
modules/generators/mod_asis.c resets r->status to HTTP_OK before
calling ap_internal_redirect_handler().  The mod_cgi and mod_cgid
modules seem to perform explicit checks for r->status == HTTP_OK
before using these functions.

The modules/arch/win32/mod_isapi.c file along with mod_rewrite.c,
mod_actions.c, and mod_negotiation.c in modules/mappers might all
be examined to determine if r->status could be a value other than
HTTP_OK prior to their use of these functions, since if these
functions detect an error and call ap_die(), then ap_die() will
think it is handling a recursive error, as is the case with this
particular bug.
Comment 1 Chris Darroch 2005-08-09 03:19:02 UTC
Created attachment 15973 [details]
suggested patch to ap_process_request()
Comment 2 Chris Darroch 2005-12-02 16:01:53 UTC
Created attachment 17122 [details]
for 2.2.0

updated for 2.2.0
Comment 3 Nick Kew 2005-12-09 13:00:20 UTC
Fixed for 2.2.1 in Revision 355454.