On Fri, Jun 26, 2020 at 8:57 AM Ruediger Pluem <[email protected]> wrote:
>
> On 6/25/20 2:16 PM, Yann Ylavic wrote:
> > On Thu, Jun 25, 2020 at 1:27 PM Ruediger Pluem <[email protected]> wrote:
> >>
> >> On 6/25/20 12:13 PM, Yann Ylavic wrote:
> >>> Index: modules/proxy/mod_proxy.c
> >>> ===================================================================
> >>> --- modules/proxy/mod_proxy.c (revision 1879145)
> >>> +++ modules/proxy/mod_proxy.c (working copy)
> >>
> >>> @@ -987,10 +991,10 @@ PROXY_DECLARE(int) ap_proxy_trans_match(request_re
> >>>                        "URI path '%s' matches proxy handler '%s'", r->uri,
> >>>                        found);
> >>>
> >>> -        return OK;
> >>> +        return servlet ? DONE : OK;
> >>
> >> Why setting it to DONE in the servlet case? Wouldn't that cause 
> >> ap_process_request_internal to be left early?
> >
> > No, ap_process_request_internal() would just skip r->uri decoding (we
> > are in pre_trans hook here, since mapping=servlet only happens there).
> > Anyway, it's an orthogonal change sorry, maybe we still want uri
> > decoding for directory/location walk in the servlet case, but since
> > this patch actually modifies r->uri (while other proxy mappings do
> > not), I thought it could be the final transformation (that's DONE
> > returned from pre_trans).
>
> Sorry, but I am still struggling: This is from server/request.c starting line 
> 233
>
> 233        access_status = ap_run_pre_translate_name(r);
> 234        if (access_status != OK && access_status != DECLINED) {
> 235            return access_status;
> 236        }

Ah yes sorry, I missed you were referring to this initial commit.
It was later changed (http://svn.apache.org/r1879137) like this:

--- httpd/httpd/trunk/include/http_request.h (original)
+++ httpd/httpd/trunk/include/http_request.h Wed Jun 24 07:47:58 2020
@@ -366,7 +366,10 @@ AP_DECLARE_HOOK(int,create_request,(requ
  * This hook allow modules an opportunity to translate the URI into an
  * actual filename, before URL decoding happens.
  * @param r The current request
- * @return OK, DECLINED, or HTTP_...
+ * @return DECLINED to let other modules handle the pre-translation,
+ *         OK if it was handled and no other module should process it,
+ *         DONE if no further transformation should happen on the URI,
+ *         HTTP_... in case of error.
  * @ingroup hooks
  */
AP_DECLARE_HOOK(int,pre_translate_name,(request_rec *r))

--- httpd/httpd/trunk/server/request.c (original)
+++ httpd/httpd/trunk/server/request.c Wed Jun 24 07:47:58 2020
@@ -227,11 +227,11 @@ AP_DECLARE(int) ap_process_request_inter
             return access_status;
         }

-        /* Let pre_translate_name hooks work with non-decoded URIs,
-         * and eventually apply their own transformations (return OK).
+        /* Let pre_translate_name hooks work with non-decoded URIs, and
+         * eventually prevent further URI transformations (return DONE).
          */
         access_status = ap_run_pre_translate_name(r);
-        if (access_status != OK && access_status != DECLINED) {
+        if (ap_is_HTTP_ERROR(access_status)) {
             return access_status;
         }

@@ -240,7 +240,7 @@ AP_DECLARE(int) ap_process_request_inter
     }

     /* Ignore URL unescaping for translated URIs already */
-    if (access_status == DECLINED && r->parsed_uri.path) {
+    if (access_status != DONE && r->parsed_uri.path) {
         core_dir_config *d = ap_get_core_module_config(r->per_dir_config);
...

>
> In case of a successful match of a proxy mapping with servlet mapping enabled 
> access_status would be DONE and we leave
> ap_process_request_internal via the the return in line 235

So we should be good with latest version of ap_process_request_internal().


Regards;
Yann.

Reply via email to