You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Eric Covener <co...@gmail.com> on 2023/03/13 14:23:32 UTC
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Yann, can you check out the failure I committed and see if it's me or
unintended? Everything else went pretty smooth and looks useful in a
bind.
# Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
for foo/bar/ baz%0d
# rewritten query 'foo%2fbar%2f+baz%2f%0d'
# expected: 'foo/bar/ baz%0d'
# received: 'foo%2fbar%2f+baz%2f%0d'
not ok 67
RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
/?$1 "[B= ?,BNEG,BCTLS]"
On Mon, Mar 13, 2023 at 10:21 AM <co...@apache.org> wrote:
>
> Author: covener
> Date: Mon Mar 13 14:20:59 2023
> New Revision: 1908349
>
> URL: http://svn.apache.org/viewvc?rev=1908349&view=rev
> Log:
> test [B] and additions
>
> 1 failing
>
> Modified:
> httpd/test/framework/trunk/t/conf/extra.conf.in
> httpd/test/framework/trunk/t/modules/rewrite.t
>
> Modified: httpd/test/framework/trunk/t/conf/extra.conf.in
> URL: http://svn.apache.org/viewvc/httpd/test/framework/trunk/t/conf/extra.conf.in?rev=1908349&r1=1908348&r2=1908349&view=diff
> ==============================================================================
> --- httpd/test/framework/trunk/t/conf/extra.conf.in (original)
> +++ httpd/test/framework/trunk/t/conf/extra.conf.in Mon Mar 13 14:20:59 2023
> @@ -270,6 +270,12 @@
> RewriteRule ^/modules/rewrite/cookie/foo - [CO=NAME3:VAL:localhost:86400:/0:secure:httponly:foo]
>
> RewriteRule ^/modules/rewrite/escaping/local/(.*) /?$1
> + RewriteRule ^/modules/rewrite/escaping/local_b/(.*) /?$1 [B]
> + RewriteRule ^/modules/rewrite/escaping/local_bctls/(.*) /?$1 [BCTLS]
> + RewriteRule ^/modules/rewrite/escaping/local_bctls_andslash/(.*) /?$1 [B=/,BCTLS]
> + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*) /?$1 "[B= ?,BNEG,BCTLS]"
> + RewriteRule ^/modules/rewrite/escaping/local_b_noslash/(.*) /?$1 [B=/,BNEG]
> + RewriteRule ^/modules/rewrite/escaping/local_b_justslash/(.*) /?$1 [B=/]
> RewriteRule ^/modules/rewrite/escaping/redir/(.*) http://@SERVERNAME@:@PORT@/?$1 [R]
> RewriteRule ^/modules/rewrite/escaping/redir_ne/(.*) http://@SERVERNAME@:@PORT@/?$1 [R,NE]
> RewriteRule ^/modules/rewrite/escaping/proxy/(.*) http://@SERVERNAME@:@PORT@/?$1 [P]
> @@ -282,6 +288,9 @@
> RewriteRule proxy_ne/(.*) http://@SERVERNAME@:@PORT@/?$1 [P,NE]
> </LocationMatch>
>
> + <Location /modules/rewrite/escaping>
> + Header always set rewritten-query "expr=%{QUERY_STRING}"
> + </Location>
> <VirtualHost cve_2011_3368_rewrite>
> DocumentRoot @SERVERROOT@/htdocs/modules/proxy
> RewriteEngine On
>
> Modified: httpd/test/framework/trunk/t/modules/rewrite.t
> URL: http://svn.apache.org/viewvc/httpd/test/framework/trunk/t/modules/rewrite.t?rev=1908349&r1=1908348&r2=1908349&view=diff
> ==============================================================================
> --- httpd/test/framework/trunk/t/modules/rewrite.t (original)
> +++ httpd/test/framework/trunk/t/modules/rewrite.t Mon Mar 13 14:20:59 2023
> @@ -34,6 +34,15 @@ my @escapes = (
> [ "/modules/rewrite/escaping/fixups/proxy_ne/foo%20bar" => 403],
> );
>
> +my @bflags = (
> + # t/conf/extra.conf.in
> + [ "/modules/rewrite/escaping/local_b/foo/bar/%20baz%0d" => "foo%2fbar%2f+baz%0d"], # this is why [B] sucks
> + [ "/modules/rewrite/escaping/local_bctls/foo/bar/%20baz/%0d" => "foo/bar/+baz/%0d"], # spaces and ctls only
> + [ "/modules/rewrite/escaping/local_bctls_andslash/foo/bar/%20baz/%0d" => "foo%2fbar%2f+baz%2f%0d"], # not realistic, but opt in to slashes
> + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d" => "foo/bar/ baz%0d"], # CTLS but allow space
> + [ "/modules/rewrite/escaping/local_b_noslash/foo/bar/%20baz/%0d" => "foo/bar/+baz/%0d"], # negate something from [B]
> + [ "/modules/rewrite/escaping/local_b_justslash/foo/bar/%20baz/" => "foo%2fbar%2f baz%2f"], # test basic B=/
> +);
>
> if (!have_min_apache_version('2.4.19')) {
> # PR 50447, server context
> @@ -47,7 +56,7 @@ if (!have_min_apache_version('2.4')) {
> # Specific tests for PR 58231
> my $vary_header_tests = (have_min_apache_version("2.4.30") ? 9 : 0) + (have_min_apache_version("2.4.29") ? 4 : 0);
> my $cookie_tests = have_min_apache_version("2.4.47") ? 6 : 0;
> -my $escape_tests = have_min_apache_version("2.4.57") ? scalar(@escapes) : 0;
> +my $escape_tests = have_min_apache_version("2.4.57") ? scalar(@escapes) + scalar(@bflags) : 0;
>
> plan tests => @map * @num + 16 + $vary_header_tests + $cookie_tests + $escape_tests, todo => \@todo, need_module 'rewrite';
>
> @@ -216,6 +225,15 @@ if (have_min_apache_version("2.4.57")) {
> $r = GET($url, redirect_ok => 0);
> ok t_cmp $r->code, $expect;
> }
> + foreach my $t (@bflags) {
> + my $url= $t->[0];
> + my $expect= $t->[1];
> + t_debug "Check $url for $expect\n";
> + $r = GET($url, redirect_ok => 0);
> + t_debug("rewritten query '" . $r->header("rewritten-query") . "'");
> + ok t_cmp $r->header("rewritten-query"), $expect;
> + }
> +
> }
>
>
>
>
--
Eric Covener
covener@gmail.com
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 13, 2023 at 8:02 PM Eric Covener <co...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 2:01 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 5:48 PM Yann Ylavic <yl...@gmail.com> wrote:
> > >
> > > On Mon, Mar 13, 2023 at 5:42 PM Eric Covener <co...@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > > >
> > > > > On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > > > > >
> > > > > > > I could get where you want to with the attached patch (before you
> > > > > > > disabled the test in r1908350).
> > > > > > > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > > > > > > to let space through while still encoding controls.
> > > > > > > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > > > > > > (not expected in the result) and add a missing '/' in the result too.
> > > > > >
> > > > > >
> > > > > > + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > > > > > /?$1 "[B= /,BNEG,BCTLS]"
> > > > > > + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> > > > > > => "foo/bar/ baz/%0d"], # CTLS but allow space
> > > > > >
> > > > > > I'm not sure I understand this, / isn't a CTL so why do we need to
> > > > > > exclude it with B+BNEG to have it passed through in the test?
> > > > >
> > > > > With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
> > > > > (regardless of BCTLS) will encode '/'.
> > > >
> > > > Ah, I see. I thought/hoped it would become relative to CTLS. Maybe
> > > > best to keep it simple.
> > >
> > > That's one way to implement it though, if BCTLS is there it could be a
> > > negation of that only, no strong opinion on this..
> > > With the current implementation the BCTLS above is implied already, so
> > > "[B= /,BNEG]" would be the same.
> >
> > Maybe instead of BNEG we can implement BNE= (backref noescape
> > charset), so one could "[B,BNE=/]" to encode everything but '/', or
> > "[BCTLS,BNE= ?]" for controls but space, etc.. BNE= always wins over
> > B=, and is meaningless alone.
>
> Yeah, this does sound better for the externals,
Done in r1908359 (tests and docs updated in r1908358 and r1908360 respectively).
>
> > Something like the attached patch now?
>
> + if (entry && (entry->flags & (RULEFLAG_ESCAPEBACKREF |
> + RULEFLAG_NOESCAPEBACKREF))) {
>
> Is this extraneous? I think BNE by itself doesn't do anything. BCTLS
> sets RULEFLAG_ESCAPEBACKREF
>
> Nit, RULEFLAG_NOESCAPEBACKREF seems a little easy to misunderstand.
> Does it have any meaning without the val provided?
Yeah, RULEFLAG_NOESCAPEBACKREF and the above change were not needed actually.
Regards;
Yann.
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Eric Covener <co...@gmail.com>.
On Mon, Mar 13, 2023 at 2:01 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 5:48 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 5:42 PM Eric Covener <co...@gmail.com> wrote:
> > >
> > > On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
> > > > >
> > > > > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > > > >
> > > > > > I could get where you want to with the attached patch (before you
> > > > > > disabled the test in r1908350).
> > > > > > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > > > > > to let space through while still encoding controls.
> > > > > > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > > > > > (not expected in the result) and add a missing '/' in the result too.
> > > > >
> > > > >
> > > > > + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > > > > /?$1 "[B= /,BNEG,BCTLS]"
> > > > > + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> > > > > => "foo/bar/ baz/%0d"], # CTLS but allow space
> > > > >
> > > > > I'm not sure I understand this, / isn't a CTL so why do we need to
> > > > > exclude it with B+BNEG to have it passed through in the test?
> > > >
> > > > With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
> > > > (regardless of BCTLS) will encode '/'.
> > >
> > > Ah, I see. I thought/hoped it would become relative to CTLS. Maybe
> > > best to keep it simple.
> >
> > That's one way to implement it though, if BCTLS is there it could be a
> > negation of that only, no strong opinion on this..
> > With the current implementation the BCTLS above is implied already, so
> > "[B= /,BNEG]" would be the same.
>
> Maybe instead of BNEG we can implement BNE= (backref noescape
> charset), so one could "[B,BNE=/]" to encode everything but '/', or
> "[BCTLS,BNE= ?]" for controls but space, etc.. BNE= always wins over
> B=, and is meaningless alone.
Yeah, this does sound better for the externals,
> Something like the attached patch now?
+ if (entry && (entry->flags & (RULEFLAG_ESCAPEBACKREF |
+ RULEFLAG_NOESCAPEBACKREF))) {
Is this extraneous? I think BNE by itself doesn't do anything. BCTLS
sets RULEFLAG_ESCAPEBACKREF
Nit, RULEFLAG_NOESCAPEBACKREF seems a little easy to misunderstand.
Does it have any meaning without the val provided?
--
Eric Covener
covener@gmail.com
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 13, 2023 at 5:48 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 5:42 PM Eric Covener <co...@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <yl...@gmail.com> wrote:
> > >
> > > On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > > >
> > > > > I could get where you want to with the attached patch (before you
> > > > > disabled the test in r1908350).
> > > > > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > > > > to let space through while still encoding controls.
> > > > > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > > > > (not expected in the result) and add a missing '/' in the result too.
> > > >
> > > >
> > > > + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > > > /?$1 "[B= /,BNEG,BCTLS]"
> > > > + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> > > > => "foo/bar/ baz/%0d"], # CTLS but allow space
> > > >
> > > > I'm not sure I understand this, / isn't a CTL so why do we need to
> > > > exclude it with B+BNEG to have it passed through in the test?
> > >
> > > With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
> > > (regardless of BCTLS) will encode '/'.
> >
> > Ah, I see. I thought/hoped it would become relative to CTLS. Maybe
> > best to keep it simple.
>
> That's one way to implement it though, if BCTLS is there it could be a
> negation of that only, no strong opinion on this..
> With the current implementation the BCTLS above is implied already, so
> "[B= /,BNEG]" would be the same.
Maybe instead of BNEG we can implement BNE= (backref noescape
charset), so one could "[B,BNE=/]" to encode everything but '/', or
"[BCTLS,BNE= ?]" for controls but space, etc.. BNE= always wins over
B=, and is meaningless alone.
Something like the attached patch now?
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 13, 2023 at 5:42 PM Eric Covener <co...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
> > >
> > > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> > > >
> > > > I could get where you want to with the attached patch (before you
> > > > disabled the test in r1908350).
> > > > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > > > to let space through while still encoding controls.
> > > > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > > > (not expected in the result) and add a missing '/' in the result too.
> > >
> > >
> > > + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > > /?$1 "[B= /,BNEG,BCTLS]"
> > > + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> > > => "foo/bar/ baz/%0d"], # CTLS but allow space
> > >
> > > I'm not sure I understand this, / isn't a CTL so why do we need to
> > > exclude it with B+BNEG to have it passed through in the test?
> >
> > With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
> > (regardless of BCTLS) will encode '/'.
>
> Ah, I see. I thought/hoped it would become relative to CTLS. Maybe
> best to keep it simple.
That's one way to implement it though, if BCTLS is there it could be a
negation of that only, no strong opinion on this..
With the current implementation the BCTLS above is implied already, so
"[B= /,BNEG]" would be the same.
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Eric Covener <co...@gmail.com>.
On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> > >
> > > I could get where you want to with the attached patch (before you
> > > disabled the test in r1908350).
> > > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > > to let space through while still encoding controls.
> > > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > > (not expected in the result) and add a missing '/' in the result too.
> >
> >
> > + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > /?$1 "[B= /,BNEG,BCTLS]"
> > + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> > => "foo/bar/ baz/%0d"], # CTLS but allow space
> >
> > I'm not sure I understand this, / isn't a CTL so why do we need to
> > exclude it with B+BNEG to have it passed through in the test?
>
> With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
> (regardless of BCTLS) will encode '/'.
Ah, I see. I thought/hoped it would become relative to CTLS. Maybe
best to keep it simple.
--
Eric Covener
covener@gmail.com
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <co...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > I could get where you want to with the attached patch (before you
> > disabled the test in r1908350).
> > It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> > to let space through while still encoding controls.
> > I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> > (not expected in the result) and add a missing '/' in the result too.
>
>
> + RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> /?$1 "[B= /,BNEG,BCTLS]"
> + [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
> => "foo/bar/ baz/%0d"], # CTLS but allow space
>
> I'm not sure I understand this, / isn't a CTL so why do we need to
> exclude it with B+BNEG to have it passed through in the test?
With BNEG everything not in B= is encoded, so "[B= ?,BNEG]"
(regardless of BCTLS) will encode '/'.
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Eric Covener <co...@gmail.com>.
On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 4:13 PM Ruediger Pluem <rp...@apache.org> wrote:
> >
> > On 3/13/23 4:04 PM, Eric Covener wrote:
> > > On Mon, Mar 13, 2023 at 10:59 AM Ruediger Pluem <rp...@apache.org> wrote:
> > >>
> > >>
> > >>
> > >> On 3/13/23 3:23 PM, Eric Covener wrote:
> > >>> Yann, can you check out the failure I committed and see if it's me or
> > >>> unintended? Everything else went pretty smooth and looks useful in a
> > >>> bind.
> > >>>
> > >>> # Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
> > >>> for foo/bar/ baz%0d
> > >>> # rewritten query 'foo%2fbar%2f+baz%2f%0d'
> > >>> # expected: 'foo/bar/ baz%0d'
> > >>> # received: 'foo%2fbar%2f+baz%2f%0d'
> > >>> not ok 67
> > >>>
> > >>>
> > >>> RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > >>> /?$1 "[B= ?,BNEG,BCTLS]"
> > >>
> > >> I think the test is wrong. Due to BCTLS being set spaces get escaped.
> > >> I think you should change the flags to "[B= ?,BNEG]"
> > >
> > > I wonder if being able to negate items in BCTL is useful though? At
> > > least the space.
> > > In the test case, it's not so useful for the query string as that will
> > > be checked and fail currently (if we knew the user did escaping, maybe
> > > we could drop the check in mod_rewrite.c and fix more of the impacted
> > > corner cases)
> > >
> > > But a capture of a space could need to go into a local filename, without [PT].
>
> I could get where you want to with the attached patch (before you
> disabled the test in r1908350).
> It makes so that anything BNEG'ed overrides BCTLS, which can be useful
> to let space through while still encoding controls.
> I had to fix the local_bctls_nospace RewriteRule to not encode '/'
> (not expected in the result) and add a missing '/' in the result too.
+ RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
/?$1 "[B= /,BNEG,BCTLS]"
+ [ "/modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d"
=> "foo/bar/ baz/%0d"], # CTLS but allow space
I'm not sure I understand this, / isn't a CTL so why do we need to
exclude it with B+BNEG to have it passed through in the test?
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Mar 13, 2023 at 4:13 PM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 3/13/23 4:04 PM, Eric Covener wrote:
> > On Mon, Mar 13, 2023 at 10:59 AM Ruediger Pluem <rp...@apache.org> wrote:
> >>
> >>
> >>
> >> On 3/13/23 3:23 PM, Eric Covener wrote:
> >>> Yann, can you check out the failure I committed and see if it's me or
> >>> unintended? Everything else went pretty smooth and looks useful in a
> >>> bind.
> >>>
> >>> # Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
> >>> for foo/bar/ baz%0d
> >>> # rewritten query 'foo%2fbar%2f+baz%2f%0d'
> >>> # expected: 'foo/bar/ baz%0d'
> >>> # received: 'foo%2fbar%2f+baz%2f%0d'
> >>> not ok 67
> >>>
> >>>
> >>> RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> >>> /?$1 "[B= ?,BNEG,BCTLS]"
> >>
> >> I think the test is wrong. Due to BCTLS being set spaces get escaped.
> >> I think you should change the flags to "[B= ?,BNEG]"
> >
> > I wonder if being able to negate items in BCTL is useful though? At
> > least the space.
> > In the test case, it's not so useful for the query string as that will
> > be checked and fail currently (if we knew the user did escaping, maybe
> > we could drop the check in mod_rewrite.c and fix more of the impacted
> > corner cases)
> >
> > But a capture of a space could need to go into a local filename, without [PT].
I could get where you want to with the attached patch (before you
disabled the test in r1908350).
It makes so that anything BNEG'ed overrides BCTLS, which can be useful
to let space through while still encoding controls.
I had to fix the local_bctls_nospace RewriteRule to not encode '/'
(not expected in the result) and add a missing '/' in the result too.
>
> If you have captures and you just want them to go in the path I guess no B at all is needed.
> If you use captures in path and query string I guess the best approach is to use no B, but
> a probably modified/added int:escape RewriteMap on the captures that go into the query string.
Captures that go both to the path and query-string are tricky to
handle, maybe two separate rewrite rules with and without [B]..
I don't know int:escape though.
Regards;
Yann.
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/13/23 4:04 PM, Eric Covener wrote:
> On Mon, Mar 13, 2023 at 10:59 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>>
>>
>> On 3/13/23 3:23 PM, Eric Covener wrote:
>>> Yann, can you check out the failure I committed and see if it's me or
>>> unintended? Everything else went pretty smooth and looks useful in a
>>> bind.
>>>
>>> # Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
>>> for foo/bar/ baz%0d
>>> # rewritten query 'foo%2fbar%2f+baz%2f%0d'
>>> # expected: 'foo/bar/ baz%0d'
>>> # received: 'foo%2fbar%2f+baz%2f%0d'
>>> not ok 67
>>>
>>>
>>> RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
>>> /?$1 "[B= ?,BNEG,BCTLS]"
>>
>> I think the test is wrong. Due to BCTLS being set spaces get escaped.
>> I think you should change the flags to "[B= ?,BNEG]"
>
> I wonder if being able to negate items in BCTL is useful though? At
> least the space.
> In the test case, it's not so useful for the query string as that will
> be checked and fail currently (if we knew the user did escaping, maybe
> we could drop the check in mod_rewrite.c and fix more of the impacted
> corner cases)
>
> But a capture of a space could need to go into a local filename, without [PT].
>
If you have captures and you just want them to go in the path I guess no B at all is needed.
If you use captures in path and query string I guess the best approach is to use no B, but
a probably modified/added int:escape RewriteMap on the captures that go into the query string.
Regards
Rüdiger
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Eric Covener <co...@gmail.com>.
On Mon, Mar 13, 2023 at 10:59 AM Ruediger Pluem <rp...@apache.org> wrote:
>
>
>
> On 3/13/23 3:23 PM, Eric Covener wrote:
> > Yann, can you check out the failure I committed and see if it's me or
> > unintended? Everything else went pretty smooth and looks useful in a
> > bind.
> >
> > # Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
> > for foo/bar/ baz%0d
> > # rewritten query 'foo%2fbar%2f+baz%2f%0d'
> > # expected: 'foo/bar/ baz%0d'
> > # received: 'foo%2fbar%2f+baz%2f%0d'
> > not ok 67
> >
> >
> > RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> > /?$1 "[B= ?,BNEG,BCTLS]"
>
> I think the test is wrong. Due to BCTLS being set spaces get escaped.
> I think you should change the flags to "[B= ?,BNEG]"
I wonder if being able to negate items in BCTL is useful though? At
least the space.
In the test case, it's not so useful for the query string as that will
be checked and fail currently (if we knew the user did escaping, maybe
we could drop the check in mod_rewrite.c and fix more of the impacted
corner cases)
But a capture of a space could need to go into a local filename, without [PT].
Re: svn commit: r1908349 - in /httpd/test/framework/trunk/t: conf/extra.conf.in modules/rewrite.t
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/13/23 3:23 PM, Eric Covener wrote:
> Yann, can you check out the failure I committed and see if it's me or
> unintended? Everything else went pretty smooth and looks useful in a
> bind.
>
> # Check /modules/rewrite/escaping/local_bctls_nospace/foo/bar/%20baz/%0d
> for foo/bar/ baz%0d
> # rewritten query 'foo%2fbar%2f+baz%2f%0d'
> # expected: 'foo/bar/ baz%0d'
> # received: 'foo%2fbar%2f+baz%2f%0d'
> not ok 67
>
>
> RewriteRule ^/modules/rewrite/escaping/local_bctls_nospace/(.*)
> /?$1 "[B= ?,BNEG,BCTLS]"
I think the test is wrong. Due to BCTLS being set spaces get escaped.
I think you should change the flags to "[B= ?,BNEG]"
Regards
Rüdiger