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.