On Mon, Jun 22, 2020 at 2:44 PM Eric Covener <cove...@gmail.com> wrote:
>
> > You need to set:
> > ProxyMappingDecoded off
> > in your vhost (or directory) for servlet mapping to be active, with a
>
> Does it work in directory context? pre_trans is before location_walk.
Argh no, didn't think of it :/
For this we have to add a third location walk in
ap_process_request_internal(), something like the attached.
To minimize impact, I save the original r->uri and don't re-walk if it
didn't change, that should address most common cases IMHO.
Would that work?
Index: server/request.c
===================================================================
--- server/request.c (revision 1879079)
+++ server/request.c (working copy)
@@ -160,6 +160,27 @@ AP_DECLARE(int) ap_some_authn_required(request_rec
return rv;
}
+static int walk_location_and_if(request_rec *r)
+{
+ int access_status;
+
+ if ((access_status = ap_location_walk(r))) {
+ return access_status;
+ }
+ if ((access_status = ap_if_walk(r))) {
+ return access_status;
+ }
+
+ /* Don't set per-dir loglevel if LogLevelOverride is set */
+ if (!r->connection->log) {
+ core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
+ if (d->log)
+ r->log = d->log;
+ }
+
+ return OK;
+}
+
/* This is the master logic for processing requests. Do NOT duplicate
* this logic elsewhere, or the security model will be broken by future
* API changes. Each phase must be individually optimized to pick up
@@ -168,13 +189,13 @@ AP_DECLARE(int) ap_some_authn_required(request_rec
AP_DECLARE(int) ap_process_request_internal(request_rec *r)
{
int file_req = (r->main && r->filename);
- int access_status;
- core_dir_config *d;
+ int access_status = DECLINED;
core_server_config *sconf =
ap_get_core_module_config(r->server->module_config);
+ const char *saved_uri = apr_pstrdup(r->pool, r->uri);
unsigned int normalize_flags = 0;
- if (r->main) {
+ if (file_req) {
/* Lookup subrequests can have a relative path. */
normalize_flags = AP_NORMALIZE_ALLOW_RELATIVE;
}
@@ -188,22 +209,40 @@ AP_DECLARE(int) ap_process_request_internal(reques
normalize_flags |
AP_NORMALIZE_DECODE_UNRESERVED)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO()
- "invalid URI path (%s)", r->uri);
+ "invalid URI path (%s)", saved_uri);
return HTTP_BAD_REQUEST;
}
}
- /* Let pre_translate_name hooks work with non-decoded URIs,
- * and eventually apply their own transformations (return OK).
+ /* All file subrequests are a huge pain... they cannot bubble through the
+ * next several steps. Only file subrequests are allowed an empty uri,
+ * otherwise let translate_name kill the request.
*/
- access_status = ap_run_pre_translate_name(r);
- if (access_status != OK && access_status != DECLINED) {
- return access_status;
+ if (!file_req) {
+ r->per_dir_config = r->server->lookup_defaults;
+
+ if ((access_status = walk_location_and_if(r))) {
+ return access_status;
+ }
+
+ /* Let pre_translate_name hooks work with non-decoded URIs,
+ * and eventually apply their own transformations (return OK).
+ */
+ access_status = ap_run_pre_translate_name(r);
+ if (access_status != OK && access_status != DECLINED) {
+ return access_status;
+ }
}
/* Ignore URL unescaping for translated URIs already */
if (access_status == DECLINED && r->parsed_uri.path) {
- d = ap_get_core_module_config(r->per_dir_config);
+ core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
+
+ /* May have changed since original */
+ if (strcmp(r->uri, saved_uri) != 0) {
+ saved_uri = apr_pstrdup(r->pool, r->uri);
+ }
+
if (d->allow_encoded_slashes) {
access_status = ap_unescape_url_keep2f(r->parsed_uri.path,
d->decode_encoded_slashes);
@@ -217,7 +256,7 @@ AP_DECLARE(int) ap_process_request_internal(reques
ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00026)
"found %%2f (encoded '/') in URI "
"(decoded='%s'), returning 404",
- r->uri);
+ saved_uri);
}
}
return access_status;
@@ -237,29 +276,17 @@ AP_DECLARE(int) ap_process_request_internal(reques
/* We still didn't merged slashes yet, do it now. */
ap_no2slash(r->parsed_uri.path);
}
- }
- /* All file subrequests are a huge pain... they cannot bubble through the
- * next several steps. Only file subrequests are allowed an empty uri,
- * otherwise let translate_name kill the request.
- */
- if (!file_req) {
- if ((access_status = ap_location_walk(r))) {
- return access_status;
- }
- if ((access_status = ap_if_walk(r))) {
- return access_status;
- }
+ if (!file_req && strcmp(r->uri, saved_uri) != 0) {
+ r->per_dir_config = r->server->lookup_defaults;
- /* Don't set per-dir loglevel if LogLevelOverride is set */
- if (!r->connection->log) {
- d = ap_get_core_module_config(r->per_dir_config);
- if (d->log)
- r->log = d->log;
- }
+ if ((access_status = walk_location_and_if(r))) {
+ return access_status;
+ }
- if ((access_status = ap_run_translate_name(r))) {
- return decl_die(access_status, "translate", r);
+ if ((access_status = ap_run_translate_name(r))) {
+ return decl_die(access_status, "translate", r);
+ }
}
}
@@ -274,20 +301,10 @@ AP_DECLARE(int) ap_process_request_internal(reques
/* Rerun the location walk, which overrides any map_to_storage config.
*/
- if ((access_status = ap_location_walk(r))) {
+ if ((access_status = walk_location_and_if(r))) {
return access_status;
}
- if ((access_status = ap_if_walk(r))) {
- return access_status;
- }
- /* Don't set per-dir loglevel if LogLevelOverride is set */
- if (!r->connection->log) {
- d = ap_get_core_module_config(r->per_dir_config);
- if (d->log)
- r->log = d->log;
- }
-
if ((access_status = ap_run_post_perdir_config(r))) {
return access_status;
}
