On Wed, Jun 28, 2023 at 9:08 AM Ruediger Pluem <rpl...@apache.org> wrote:
>
>
>
> On 6/28/23 2:37 PM, cove...@apache.org wrote:
> > Author: covener
> > Date: Wed Jun 28 12:37:50 2023
> > New Revision: 1910662
> >
> > URL: http://svn.apache.org/viewvc?rev=1910662&view=rev
> > Log:
> > back to original patch in PR66672
> >
> > Since QSNONE will skip splitoutqueryargs, it's where we should fixup the
> > trailing ?
> >
> > 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=1910662&r1=1910661&r2=1910662&view=diff
> > ==============================================================================
> > --- httpd/httpd/trunk/modules/mappers/mod_rewrite.c (original)
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Wed Jun 28 12:37:50 2023
> > @@ -3909,10 +3909,12 @@ 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)) {
> > + /* trailing ? has done its job. with QSA, splitoutqueryargs
> > + * will handle it
>
> I think only if QSLAST is set. Otherwise substitutions or the matching URL
> may place a '?' left of the terminating '?' that came
> in as encoded '?' and thus would add the original querystring as "&&" +
> r->args. Hence we would end up with
>
> <one character after some decoded '?' somewhere in the URL or after
> substitution><further chars>+ "?&&" + r->args.
>
> Hence I think it should be:
>
> if (!(newrule->flags & RULEFLAG_QSAPPEND)) {
> /* trailing ? has done its job.
> */
> *(a2_end-1) = '\0';
> newrule->flags |= RULEFLAG_QSNONE;
> }
> else {
> /* with QSA, splitoutqueryargs will handle it if RULEFLAG_QSLAST
> is set.
> newrule->flags |= RULEFLAG_QSLAST;
> }
>
flipping it around and refreshing some comments:
if (*(a2_end-1) == '?') {
/* a literal ? at the end of the unsubstituted rewrite rule */
if (newrule->flags & RULEFLAG_QSAPPEND) {
/* with QSA, splitoutqueryargs will safely handle it if
RULEFLAG_QSLAST is set */
newrule->flags |= RULEFLAG_QSLAST;
}
else {
/* avoid getting a a query string via inadvertent capture */
newrule->flags |= RULEFLAG_QSNONE;
/* trailing ? has done its job, but splitoutqueryargs will
not chop it off */
*(a2_end-1) = '\0';
}
}
else if (newrule->flags & RULEFLAG_QSDISCARD) {
if (NULL == ap_strchr(newrule->output, '?')) {
newrule->flags |= RULEFLAG_QSNONE;
}
}
close?
--
Eric Covener
cove...@gmail.com
