On Mon, Mar 13, 2023 at 4:13 PM Ruediger Pluem <rpl...@apache.org> wrote:
>
> On 3/13/23 4:04 PM, Eric Covener wrote:
> > On Mon, Mar 13, 2023 at 10:59 AM Ruediger Pluem <rpl...@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.
Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c (revision 1908349)
+++ modules/mappers/mod_rewrite.c (working copy)
@@ -699,8 +699,17 @@ static char *escape_backref(apr_pool_t *p, const c
unsigned char c;
while ((c = *s)) {
- if ((ctls ? !TEST_CHAR(c, T_VCHAR_OBSTEXT) : !escapeme)
- || (escapeme && ((ap_strchr_c(escapeme, c) != NULL) ^ neg))) {
+ int escape = (ctls) ? !TEST_CHAR(c, T_VCHAR_OBSTEXT) : !escapeme;
+ if (escapeme) {
+ int set = (ap_strchr_c(escapeme, c) != NULL);
+ if (set && neg) {
+ escape = 0;
+ }
+ else {
+ escape |= set ^ neg;
+ }
+ }
+ if (escape) {
if (apr_isalnum(c) || c == '_') {
*d++ = c;
}
Index: framework/t/conf/extra.conf.in
===================================================================
--- framework/t/conf/extra.conf.in (revision 1908349)
+++ framework/t/conf/extra.conf.in (working copy)
@@ -273,7 +273,7 @@
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_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]
Index: framework/t/modules/rewrite.t
===================================================================
--- framework/t/modules/rewrite.t (revision 1908349)
+++ framework/t/modules/rewrite.t (working copy)
@@ -39,7 +39,7 @@ my @bflags = (
[ "/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_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=/
);
