You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ja...@apache.org on 2020/02/15 18:16:31 UTC

svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS

Author: jailletc36
Date: Sat Feb 15 18:16:31 2020
New Revision: 1874061

URL: http://svn.apache.org/viewvc?rev=1874061&view=rev
Log:
Add a note

Modified:
    httpd/httpd/branches/2.4.x/STATUS

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1874061&r1=1874060&r2=1874061&view=diff
==============================================================================
--- 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 :( ).
 
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]



Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Feb 16, 2020 at 7:57 PM Eric Covener <co...@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 3:48 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <co...@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.

Re: svn commit: r1874061 - /httpd/httpd/branches/2.4.x/STATUS

Posted by Eric Covener <co...@gmail.com>.
On Sat, Feb 15, 2020 at 3:48 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <co...@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

Posted by Yann Ylavic <yl...@gmail.com>.
On Sat, Feb 15, 2020 at 9:01 PM Eric Covener <co...@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

Posted by Eric Covener <co...@gmail.com>.
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 <co...@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 2:30 PM Yann Ylavic <yl...@gmail.com> wrote:
> >
> > On Sat, Feb 15, 2020 at 7:16 PM <ja...@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

Posted by Eric Covener <co...@gmail.com>.
On Sat, Feb 15, 2020 at 2:30 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Sat, Feb 15, 2020 at 7:16 PM <ja...@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

Posted by Yann Ylavic <yl...@gmail.com>.
On Sat, Feb 15, 2020 at 7:16 PM <ja...@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.