Bug 62186 - ErrorDocument causes POST request getting logged as GET request
Summary: ErrorDocument causes POST request getting logged as GET request
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: Core (show other bugs)
Version: 2.5-HEAD
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
Keywords: FixedInTrunk, PatchAvailable
: 46969 (view as bug list)
Depends on:
Reported: 2018-03-16 15:20 UTC by Micha Lenk
Modified: 2018-05-31 19:22 UTC (History)
2 users (show)

Preserve request method for non-GET requests with ErrorDocument (1.18 KB, patch)
2018-03-16 15:20 UTC, Micha Lenk
Details | Diff
Changes to httpd-test suite for easier reproduction (2.29 KB, patch)
2018-03-16 15:24 UTC, Micha Lenk
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Micha Lenk 2018-03-16 15:20:28 UTC
Created attachment 35777 [details]
Preserve request method for non-GET requests with ErrorDocument

If ErrorDocument is used with local files, and a LogFormat is used with "%<m" to log the original's request method, the logged request method is wrong for non-GET requests.

After some debugging and reading code I found the core function ap_die_r() to overwrite the original requests's r->method and r->method_number in the code path that calls ap_internal_redirect() for generating the response body from the local file configured with ErrorDocument. Any subsequent access on r->method will see a GET request, even if the original request was not a GET request. So, consequently a placeholder of "%<m" used in LogFormat will generate the string "GET" for all requests. (I assume the intent behind overwriting r->method and r->method_number is to force the internal redirect request for the local error document specified with ErrorDocument into a "GET" request.)

The attached patch fixes that as close as possible, that is right after calling ap_internal_redirect(), by restoring the original r->method and r->method_number. At least it fixes the issue for the use case described above, that is when logging the request method.

The patch is good enough for (ie. it fixes) my use case. Yet, it might still be slightly incorrect. Any processing that happens within the call of ap_internal_redirect (modules and so on) will see r->prev->method to be a "GET" request, even if it was not. So, a better fix might be to extend the ap_internal_redirect() function signature to contain an explicit request method, which would obsolete the need to overwrite r->method inside ap_die_r(). But that would mean an API change impacting anything that potentially calls ap_internal_redirect(). So, this probably can't be done on (i.e. backported to) the 2.4.x branch.
Comment 1 Micha Lenk 2018-03-16 15:24:17 UTC
Created attachment 35778 [details]
Changes to httpd-test suite for easier reproduction

I also prepared a change set for the httpd-test suite that can be used to reproduce the issue fairly easy. See the second attached patch.
Comment 2 Eric Covener 2018-04-11 18:52:13 UTC
Thanks Micha, committed to trunk in r1828920 and proposed for 2.4.x.
Comment 3 Christophe JAILLET 2018-04-28 08:27:48 UTC
Fixed in 2.4.x in r1830248.
Will be part of 2.4.34
Comment 4 Christophe JAILLET 2018-05-31 19:22:29 UTC
*** Bug 46969 has been marked as a duplicate of this bug. ***