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; }