Mailing List Archive

Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS
On Sat, Feb 15, 2020 at 7:16 PM <jailletc36@apache.org> wrote:
>
> --- httpd/httpd/branches/2.4.x/STATUS (original)
> +++ httpd/httpd/branches/2.4.x/STATUS Sat Feb 15 18:16:31 2020
> @@ -133,6 +133,8 @@ RELEASE SHOWSTOPPERS:
> trunk patch: http://svn.apache.org/viewvc?rev=1873941&view=rev
> 2.4.x patch: http://people.apache.org/~covener/patches/httpd-2.4.x-substitute-nodotall.diff
> +1 covener, rpluem, jim, ylavic
> + jailletc36: This needs some doc update (RegexDefaultOptions and certainly somewhere
> + for the util_regex.c stuff, but I've not found where :( ).

I wonder if, instead of "exposing" NO_DOTALL, we shouldn't have
AP_REG_NO_DEFAULT used internally only.
There is already a way to disable DOTALL in RegexDefaultOptions (with
-DOTALL), and the internal changes to ap_regcomp() and
ap_rxplus_compile() could use AP_REG_NO_DEFAULT instead of
AP_REG_NO_DOTALL, avoiding to add more NO_* options later (possibly).
For mod_substitute, AP_REG_NO_DEFAULT (besides sensible
AP_REG_DOLLAR_ENDONLY) would be set since flags can be set explicitely
too.

Something like the attached patch (over trunk's r1873941)?

Regards,
Yann.
Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS [ In reply to ]
On Sat, Feb 15, 2020 at 2:30 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 7:16 PM <jailletc36@apache.org> wrote:
> >
> > --- httpd/httpd/branches/2.4.x/STATUS (original)
> > +++ httpd/httpd/branches/2.4.x/STATUS Sat Feb 15 18:16:31 2020
> > @@ -133,6 +133,8 @@ RELEASE SHOWSTOPPERS:
> > trunk patch: http://svn.apache.org/viewvc?rev=1873941&view=rev
> > 2.4.x patch: http://people.apache.org/~covener/patches/httpd-2.4.x-substitute-nodotall.diff
> > +1 covener, rpluem, jim, ylavic
> > + jailletc36: This needs some doc update (RegexDefaultOptions and certainly somewhere
> > + for the util_regex.c stuff, but I've not found where :( ).
>
> I wonder if, instead of "exposing" NO_DOTALL, we shouldn't have
> AP_REG_NO_DEFAULT used internally only.
> There is already a way to disable DOTALL in RegexDefaultOptions (with
> -DOTALL), and the internal changes to ap_regcomp() and
> ap_rxplus_compile() could use AP_REG_NO_DEFAULT instead of
> AP_REG_NO_DOTALL, avoiding to add more NO_* options later (possibly).
> For mod_substitute, AP_REG_NO_DEFAULT (besides sensible
> AP_REG_DOLLAR_ENDONLY) would be set since flags can be set explicitely
> too.
>
> Something like the attached patch (over trunk's r1873941)?

looks better, +1
Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS [ In reply to ]
Index: server/util_regex.c
===================================================================
--- server/util_regex.c (revision 1874061)
+++ server/util_regex.c (working copy)
@@ -94,6 +94,7 @@ AP_DECLARE(ap_rxplus_t*) ap_rxplus_compile(apr_poo
}

/* anything after the current delimiter is flags */
+ ret->flags |= AP_REG_DOLLAR_ENDONLY;

what's going on there?

On Sat, Feb 15, 2020 at 2:43 PM Eric Covener <covener@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 2:30 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >
> > On Sat, Feb 15, 2020 at 7:16 PM <jailletc36@apache.org> wrote:
> > >
> > > --- httpd/httpd/branches/2.4.x/STATUS (original)
> > > +++ httpd/httpd/branches/2.4.x/STATUS Sat Feb 15 18:16:31 2020
> > > @@ -133,6 +133,8 @@ RELEASE SHOWSTOPPERS:
> > > trunk patch: http://svn.apache.org/viewvc?rev=1873941&view=rev
> > > 2.4.x patch: http://people.apache.org/~covener/patches/httpd-2.4.x-substitute-nodotall.diff
> > > +1 covener, rpluem, jim, ylavic
> > > + jailletc36: This needs some doc update (RegexDefaultOptions and certainly somewhere
> > > + for the util_regex.c stuff, but I've not found where :( ).
> >
> > I wonder if, instead of "exposing" NO_DOTALL, we shouldn't have
> > AP_REG_NO_DEFAULT used internally only.
> > There is already a way to disable DOTALL in RegexDefaultOptions (with
> > -DOTALL), and the internal changes to ap_regcomp() and
> > ap_rxplus_compile() could use AP_REG_NO_DEFAULT instead of
> > AP_REG_NO_DOTALL, avoiding to add more NO_* options later (possibly).
> > For mod_substitute, AP_REG_NO_DEFAULT (besides sensible
> > AP_REG_DOLLAR_ENDONLY) would be set since flags can be set explicitely
> > too.
> >
> > Something like the attached patch (over trunk's r1873941)?
>
> looks better, +1



--
Eric Covener
covener@gmail.com
Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS [ In reply to ]
On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <covener@gmail.com> wrote:
>
> Index: server/util_regex.c
> ===================================================================
> --- server/util_regex.c (revision 1874061)
> +++ server/util_regex.c (working copy)
> @@ -94,6 +94,7 @@ AP_DECLARE(ap_rxplus_t*) ap_rxplus_compile(apr_poo
> }
>
> /* anything after the current delimiter is flags */
> + ret->flags |= AP_REG_DOLLAR_ENDONLY;
>
> what's going on there?

I assumed we'd still always want DOLLAR_ENDONLY (which otherwise is
unset by NO_DEFAULT).
It's a sensible default/hardcoding IMHO, MULTILINE is possibly what
users want to match '$' before an end-of-line (note that
DOLLAR_ENDONLY is ignored when MULTILINE is set).
Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS [ In reply to ]
On Sat, Feb 15, 2020 at 3:48 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <covener@gmail.com> wrote:
> >
> > Index: server/util_regex.c
> > ===================================================================
> > --- server/util_regex.c (revision 1874061)
> > +++ server/util_regex.c (working copy)
> > @@ -94,6 +94,7 @@ AP_DECLARE(ap_rxplus_t*) ap_rxplus_compile(apr_poo
> > }
> >
> > /* anything after the current delimiter is flags */
> > + ret->flags |= AP_REG_DOLLAR_ENDONLY;
> >
> > what's going on there?
>
> I assumed we'd still always want DOLLAR_ENDONLY (which otherwise is
> unset by NO_DEFAULT).
> It's a sensible default/hardcoding IMHO, MULTILINE is possibly what
> users want to match '$' before an end-of-line (note that
> DOLLAR_ENDONLY is ignored when MULTILINE is set).

LGTM. I am confused every time about the ap_rxplus interface on top.

--
Eric Covener
covener@gmail.com
Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS [ In reply to ]
On Sun, Feb 16, 2020 at 7:57 PM Eric Covener <covener@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 3:48 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
> >
> > On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <covener@gmail.com> wrote:
> > >
> > > Index: server/util_regex.c
> > > ===================================================================
> > > --- server/util_regex.c (revision 1874061)
> > > +++ server/util_regex.c (working copy)
> > > @@ -94,6 +94,7 @@ AP_DECLARE(ap_rxplus_t*) ap_rxplus_compile(apr_poo
> > > }
> > >
> > > /* anything after the current delimiter is flags */
> > > + ret->flags |= AP_REG_DOLLAR_ENDONLY;
> > >
> > > what's going on there?
> >
> > I assumed we'd still always want DOLLAR_ENDONLY (which otherwise is
> > unset by NO_DEFAULT).
> > It's a sensible default/hardcoding IMHO, MULTILINE is possibly what
> > users want to match '$' before an end-of-line (note that
> > DOLLAR_ENDONLY is ignored when MULTILINE is set).
>
> LGTM. I am confused every time about the ap_rxplus interface on top.

Finally (in r1874090), for ap_rxplus_compile() and mod_substitute I used:
AP_REG_NO_DEFAULT | (ap_regcomp_get_default_cflags() & AP_REG_DOLLAR_ENDONLY)
to set AP_REG_DOLLAR_ENDONLY only if it's not globally disabled by
RegexDefaultOptions.

Thanks,
Yann.