On Mon, Mar 13, 2023 at 8:02 PM Eric Covener <cove...@gmail.com> wrote:
>
> On Mon, Mar 13, 2023 at 2:01 PM Yann Ylavic <ylavic....@gmail.com> wrote:
> >
> > On Mon, Mar 13, 2023 at 5:48 PM Yann Ylavic <ylavic....@gmail.com> wrote:
> > >
> > > On Mon, Mar 13, 2023 at 5:42 PM Eric Covener <cove...@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 13, 2023 at 12:31 PM Yann Ylavic <ylavic....@gmail.com>
> > > > wrote:
> > > > >
> > > > > On Mon, Mar 13, 2023 at 5:25 PM Eric Covener <cove...@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > On Mon, Mar 13, 2023 at 12:05 PM Yann Ylavic <ylavic....@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.
