Hi Apache httpd committers,
I think I've found a bug which triggers under following conditions:
* Apache is configured to serve a local customized error page, e.g.
using something like "ErrorDocument 404 /var/www/errors/404.html"
* Apache is configured to log the original request's method, e.g.
using something like (please note, the "%<m" is the part that
matters):
CustomLog logs/mylog "%h %l %u %t \"%r\" %>s %b method=\"%<m\""
* HTTP request is a POST request (request body content doesn't matter)
* the request destination path results in a 404 Not Found error
If all these conditions are met, the request will get logged to
logs/mylog with a line ending with 'method="GET"', even though this was
a POST request.
For easier reproduction of this case, I've created a patch that extends
the Apache httpd test suite to cover this case. Please see the attached
file bz62186_httpd-test.patch.
An explanation of this behavior can be found in the code of ap_die_r(),
which explicitly sets r->method to "GET" and r->method_number to M_GET
before it is calling ap_internal_redirect(custom_response, r) to serve
the configured error document.
I've tried to fix this issue by taking a backup of the original
request's method and restoring it as soon as ap_internal_redirect()
returns (see attached patch bz62186_httpd_bugfix.patch). So far the
tests I've done are successful, i.e. the request is now correctly logged
as POST request.
I've filed this issue some days ago as
https://bz.apache.org/bugzilla/show_bug.cgi?id=62186 , but so far it
didn't get any comments yet. Could anybody please take a look?
Kind regards,
Micha
--- t/apache/errordoc_method_logging.t (nonexistent)
+++ t/apache/errordoc_method_logging.t (working copy)
@@ -0,0 +1,34 @@
+use strict;
+use warnings FATAL => 'all';
+
+use Data::Dumper;
+use Apache::Test;
+use Apache::TestRequest;
+use Apache::TestUtil qw/t_cmp
+ t_start_file_watch
+ t_finish_file_watch/;
+
+Apache::TestRequest::module('error_document');
+
+plan tests => 3, need_lwp;
+
+{
+ t_start_file_watch 'method_log';
+
+ my $response = POST '/method_logging', content => 'does not matter';
+ chomp(my $content = $response->content);
+
+ ok t_cmp($response->code,
+ 404,
+ 'POST /method_logging, code');
+
+ ok t_cmp($content,
+ 'Error 404 Test',
+ 'POST /method/logging, content');
+
+ my @loglines = t_finish_file_watch 'method_log';
+ chomp @loglines;
+ ok t_cmp($loglines[0],
+ qr/"POST \/method_logging HTTP\/1.1" .* method="POST"/,
+ 'POST /method/logging, log');
+}
--- t/conf/extra.conf.in (revision 1826815)
+++ t/conf/extra.conf.in (working copy)
@@ -742,7 +742,11 @@
##
<VirtualHost _default_:error_document>
ErrorDocument 404 "per-server 404
-
+ <IfModule mod_log_config.c>
+ CustomLog logs/method_log "%h %l %u %t \"%r\" %>s %b method=\"%<m\""
+ CustomLog logs/access_log "%h %l %u %t \"%r\" %>s %b"
+ </IfModule>
+
<Location /redefine>
ErrorDocument 404 "per-dir 404
</Location>
@@ -760,6 +764,10 @@
ErrorDocument 404 default
</Location>
+ <Location /method_logging>
+ ErrorDocument 404 /apache/errordoc/404.html
+ </Location>
+
<Directory @DocumentRoot@/apache>
ErrorDocument 404 "testing merge
</Directory>
--- t/htdocs/apache/errordoc/404.html (nonexistent)
+++ t/htdocs/apache/errordoc/404.html (working copy)
@@ -0,0 +1, @@
+Error 404 Test
--- modules/http/http_request.c (revision 1826989)
+++ modules/http/http_request.c (working copy)
@@ -187,7 +187,8 @@
apr_table_setn(r->headers_out, "Location", custom_response);
}
else if (custom_response[0] == '/') {
- const char *error_notes;
+ const char *error_notes, *original_method;
+ int original_method_number;
r->no_local_copy = 1; /* Do NOT send HTTP_NOT_MODIFIED for
* error documents! */
/*
@@ -205,9 +206,13 @@
"error-notes")) != NULL) {
apr_table_setn(r->subprocess_env, "ERROR_NOTES", error_notes);
}
+ original_method = r->method;
+ original_method_number = r->method_number;
r->method = "GET";
r->method_number = M_GET;
ap_internal_redirect(custom_response, r);
+ r->method = original_method;
+ r->method_number = original_method_number;
return;
}
else {
