On 8/19/24 2:14 PM, Eric Covener wrote:
>>
>> Maybe reverting r757427
>> (https://svn.apache.org/viewvc?view=revision&revision=757427) fixes this.
>> My new patch sofar:
>>
>> Index: modules/mappers/mod_rewrite.c
>> ===================================================================
>> --- modules/mappers/mod_rewrite.c (revision 1920017)
>> +++ modules/mappers/mod_rewrite.c (working copy)
>> @@ -4527,20 +4527,6 @@
>> * ourself).
>> */
>> if (p->flags & RULEFLAG_PROXY) {
>> - /* For rules evaluated in server context, the mod_proxy fixup
>> - * hook can be relied upon to escape the URI as and when
>> - * necessary, since it occurs later. If in directory context,
>> - * the ordering of the fixup hooks is forced such that
>> - * mod_proxy comes first, so the URI must be escaped here
>> - * instead. See PR 39746, 46428, and other headaches. */
>> - if (ctx->perdir && (p->flags & RULEFLAG_NOESCAPE) == 0) {
>> - char *old_filename = r->filename;
>> -
>> - r->filename = ap_escape_uri(r->pool, r->filename);
>> - rewritelog(r, 2, ctx->perdir, "escaped URI in per-dir context "
>> - "for proxy, %s -> %s", old_filename, r->filename);
>> - }
>> -
>> fully_qualify_uri(r);
>>
>> rewritelog(r, 2, ctx->perdir, "forcing proxy-throughput with %s",
>> @@ -5392,12 +5378,17 @@
>> return HTTP_FORBIDDEN;
>> }
>>
>> - /* make sure the QUERY_STRING and
>> - * PATH_INFO parts get incorporated
>> + if (rulestatus == ACTION_NOESCAPE) {
>> + apr_table_setn(r->notes, "proxy-nocanon", "1");
>> + }
>
> This seems to preserve 2.4.58 behavior of [P,NE] in per-dir.
> We should re-document NE in this context though
I am not sure what you mean here.
>
>> +
>> + /* make sure the QUERY_STRING gets incorporated, but only
>> + * if we do not escape the target. Otherwise the mod_proxy
>> + * canon handler will take care to do this.
>
> nit: I don't think we will ever escape the target later though [in
> mod_rewrite].
What the comment wants to say is that we only need to add the QUERY_STRING if
we do no escape the target. In case we escape the
target the QUERY_STRING gets added by the canon handler.
>
>
>> * (r->path_info was already appended by the
>> * rewriting engine because of the per-dir context!)
>> */
>> - if (r->args != NULL) {
>> + if ((r->args != NULL) && (rulestatus == ACTION_NOESCAPE)) {
>> /* see proxy_http:proxy_http_canon() */
>> r->filename = apr_pstrcat(r->pool, r->filename,
>> "?", r->args, NULL);
>
> nit: It might be more clear here to check (or memoize) proxy-nocanon
> directly, for the same confusion as above.
>
You mean replace (rulestatus == ACTION_NOESCAPE) with apr_table_get(r->notes,
"proxy-nocanon") ?
Regards
Rüdiger
