On Mon, Mar 13, 2023 at 8:31 AM Yann Ylavic <ylavic....@gmail.com> wrote:
>
> On Sat, Mar 11, 2023 at 11:10 PM <cove...@apache.org> wrote:
> >
> > Author: covener
> > Date: Sat Mar 11 22:10:09 2023
> > New Revision: 1908301
> >
> > URL: http://svn.apache.org/viewvc?rev=1908301&view=rev
> > Log:
> > add [BCTLS] alternative to [B] for 2.4.56 problems
>
> Nice, that was the missing bit I think. Would have been great if
> [BCTLS] was the default but I guess it's not possible with the
> redirect case which %-encodes by default (chicken and egg problem to
> avoid double encoding..).
>
> >
> > +++ httpd/httpd/trunk/modules/mappers/mod_rewrite.c Sat Mar 11 22:10:09 2023
> > @@ -684,14 +685,27 @@ static APR_INLINE unsigned char *c2x(uns
> > * Escapes a backreference in a similar way as php's urlencode does.
> > * Based on ap_os_escape_path in server/util.c
> > */
> > -static char *escape_backref(apr_pool_t *p, const char *path, const char
> > *escapeme, int noplus) {
> > +static char *escape_backref(apr_pool_t *p, const char *path, const char
> > *escapeme, int flags) {
> > char *copy = apr_palloc(p, 3 * strlen(path) + 3);
> > const unsigned char *s = (const unsigned char *)path;
> > unsigned char *d = (unsigned char *)copy;
> > unsigned c;
> > + int noplus = flags & RULEFLAG_ESCAPENOPLUS;
> > + int ctls = flags & RULEFLAG_ESCAPECTLS;
> >
> > while ((c = *s)) {
> > - if (!escapeme) {
> > + if (ctls) {
> > + if (c == ' ' && !noplus) {
> > + *d++ = '+';
> > + }
> > + else if (apr_iscntrl(c)) {
>
> Seems we should encode space here too => (apr_iscntrl(c) || c == ' ') ?
yeah looks wrong, thanks, but let's proceed with your flavor with the
extra features.
>
> > + d = c2x(c, '%', d);
> > + }
> > + else {
> > + *d++ = c;
> > + }
> > + }
> > + else if (!escapeme) {
> > if (apr_isalnum(c) || c == '_') {
> > *d++ = c;
> > }
> > @@ -702,9 +716,9 @@ static char *escape_backref(apr_pool_t *
> > d = c2x(c, '%', d);
> > }
> > }
> > - else {
> > + else {
>
> Maybe we could make [B=] and [BTCLS] not mutually exclusive (encode
> both controls and whatever in B=).
> I was thinking of a [BNEG] flag too (encode everything but what's in
> B=), and never encode alnum or '_', so all in all some further patch
> like the attached one. WDYT?
Looks good to me, I have an ancient patch where I did something very
similar to a copy of int:escape where you could set exceptions in
subprocess_env. The config is ugly so I never upstreamed it.
if you make the change I can add some tests/doc. Should caution
against plain [B] and refer to the others in the doc?
