You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2023/06/28 10:38:27 UTC

svn commit: r1910650 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c

Author: covener
Date: Wed Jun 28 10:38:27 2023
New Revision: 1910650

URL: http://svn.apache.org/viewvc?rev=1910650&view=rev
Log:
act more like pre-r1908097 with rewrite

- be more conservative with setting QSNONE
- allow the query to be split as historically, but then drop r->args if QSNONE


Modified:
    httpd/httpd/trunk/modules/mappers/mod_rewrite.c

Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910650&r1=1910649&r2=1910650&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
+++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 10:38:27 2023
@@ -806,12 +806,6 @@ static void splitout_queryargs(request_r
     int qsdiscard = flags & RULEFLAG_QSDISCARD;
     int qslast = flags & RULEFLAG_QSLAST;
 
-    if (flags & RULEFLAG_QSNONE) {
-        rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
-        r->args = NULL;
-        return;
-    }
-
     /* don't touch, unless it's a scheme for which a query string makes sense.
      * See RFC 1738 and RFC 2368.
      */
@@ -853,6 +847,10 @@ static void splitout_queryargs(request_r
                r->args[len-1] = '\0';
            }
         }
+        if (flags & RULEFLAG_QSNONE) {
+            rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
+            r->args = NULL;
+        }
 
         rewritelog(r, 3, NULL, "split uri=%s -> uri=%s, args=%s", olduri,
                    r->filename, r->args ? r->args : "<none>");
@@ -3909,15 +3907,15 @@ static const char *cmd_rewriterule(cmd_p
     }
 
     if (*(a2_end-1) == '?') {
-        *(a2_end-1) = '\0'; /* trailing ? has done its job */
-        /* a literal ? at the end of the unsubstituted rewrite rule */
-        if (!(newrule->flags & RULEFLAG_QSAPPEND))
-        {
+        if (!(newrule->flags & RULEFLAG_QSAPPEND) &&
+            !(newrule->flags & RULEFLAG_QSLAST)) {
+            /* Rule ends with a literal ?, make sure we don't end up with any query */
             newrule->flags |= RULEFLAG_QSNONE;
         }
     }
     else if (newrule->flags & RULEFLAG_QSDISCARD) {
         if (NULL == ap_strchr(newrule->output, '?')) {
+            /* QSD and no literal ? in substitution, make sure we don't end up with any query */
             newrule->flags |= RULEFLAG_QSNONE;
         }
     }



Re: svn commit: r1910650 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c

Posted by Eric Covener <co...@gmail.com>.
On Wed, Jun 28, 2023 at 8:14 AM Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 6/28/23 12:38 PM, covener@apache.org wrote:
> > Author: covener
> > Date: Wed Jun 28 10:38:27 2023
> > New Revision: 1910650
> >
> > URL: http://svn.apache.org/viewvc?rev=1910650&view=rev
> > Log:
> > act more like pre-r1908097 with rewrite
> >
> > - be more conservative with setting QSNONE
> > - allow the query to be split as historically, but then drop r->args if QSNONE
> >
> >
> > Modified:
> >     httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> >
> > Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910650&r1=1910649&r2=1910650&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 10:38:27 2023
> > @@ -806,12 +806,6 @@ static void splitout_queryargs(request_r
> >      int qsdiscard = flags & RULEFLAG_QSDISCARD;
> >      int qslast = flags & RULEFLAG_QSLAST;
> >
> > -    if (flags & RULEFLAG_QSNONE) {
> > -        rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
> > -        r->args = NULL;
> > -        return;
> > -    }
> > -
> >      /* don't touch, unless it's a scheme for which a query string makes sense.
> >       * See RFC 1738 and RFC 2368.
> >       */
> > @@ -853,6 +847,10 @@ static void splitout_queryargs(request_r
> >                 r->args[len-1] = '\0';
> >             }
> >          }
> > +        if (flags & RULEFLAG_QSNONE) {
> > +            rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
> > +            r->args = NULL;
> > +        }
>
> Why moving it here? This means that we remove '?' that where originally encoded in the URL and everything after this from the
> resulting URL due to the
>
> *q++ = '\0';
>
> above, but we don't put them in r->args either if RULEFLAG_QSNONE is set e.g. if we end on a '?' in the rewrite template and no
> QSA / QSD being set. I think in this case we should not touch any '?' in the result and just treat them as decoded '?' that get
> renencoded e.g. when forwarding the URL to a backend.
>

I see what you mean. I was overly focused on getting closer to the
original behavior and just ensuring an unexpected query doesn't sneak
in, but this will truncate the path if there is a capture/substitution
of an encoded query.

Re: svn commit: r1910650 - /httpd/httpd/trunk/modules/mappers/mod_rewrite.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 6/28/23 12:38 PM, covener@apache.org wrote:
> Author: covener
> Date: Wed Jun 28 10:38:27 2023
> New Revision: 1910650
> 
> URL: http://svn.apache.org/viewvc?rev=1910650&view=rev
> Log:
> act more like pre-r1908097 with rewrite
> 
> - be more conservative with setting QSNONE
> - allow the query to be split as historically, but then drop r->args if QSNONE
> 
> 
> Modified:
>     httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> 
> Modified: httpd/httpd/trunk/modules/mappers/mod_rewrite.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_rewrite.c?rev=1910650&r1=1910649&r2=1910650&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 10:38:27 2023
> @@ -806,12 +806,6 @@ static void splitout_queryargs(request_r
>      int qsdiscard = flags & RULEFLAG_QSDISCARD;
>      int qslast = flags & RULEFLAG_QSLAST;
>  
> -    if (flags & RULEFLAG_QSNONE) {
> -        rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
> -        r->args = NULL;
> -        return;
> -    }
> -
>      /* don't touch, unless it's a scheme for which a query string makes sense.
>       * See RFC 1738 and RFC 2368.
>       */
> @@ -853,6 +847,10 @@ static void splitout_queryargs(request_r
>                 r->args[len-1] = '\0';
>             }
>          }
> +        if (flags & RULEFLAG_QSNONE) {
> +            rewritelog(r, 2, NULL, "discarding query string, no parse from substitution");
> +            r->args = NULL;
> +        }

Why moving it here? This means that we remove '?' that where originally encoded in the URL and everything after this from the
resulting URL due to the

*q++ = '\0';

above, but we don't put them in r->args either if RULEFLAG_QSNONE is set e.g. if we end on a '?' in the rewrite template and no
QSA / QSD being set. I think in this case we should not touch any '?' in the result and just treat them as decoded '?' that get
renencoded e.g. when forwarding the URL to a backend.

Regards

Rüdiger