You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2015/06/30 03:27:43 UTC
svn commit: r1688339 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Author: wrowe
Date: Tue Jun 30 01:27:42 2015
New Revision: 1688339
URL: http://svn.apache.org/r1688339
Log:
Very difficult to read, and therefore was wrong.
Assert that the SubstituteInheritBefore option was explicitly toggled,
and do not default in 2.x to this legacy behavior.
Modified:
httpd/httpd/trunk/modules/filters/mod_substitute.c
Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1688339&r1=1688338&r2=1688339&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_substitute.c Tue Jun 30 01:27:42 2015
@@ -86,9 +86,16 @@ static void *merge_substitute_dcfg(apr_p
subst_dir_conf *base = (subst_dir_conf *) basev;
subst_dir_conf *over = (subst_dir_conf *) overv;
- a->inherit_before = (over->inherit_before > 0 || (over->inherit_before < 0 &&
- base->inherit_before > 0));
- if (a->inherit_before) {
+ a->inherit_before = (over->inherit_before != -1)
+ ? over->inherit_before
+ : base->inherit_before;
+ /* SubstituteInheritBefore was the default behavior until 2.5.x,
+ * and may be re-enabled as desired; this original default behavior
+ * was to apply inherited subst patterns before locally scoped patterns.
+ * In later 2.2 and 2.4 versions, SubstituteInheritBefore may be toggled
+ * 'off' to follow the corrected/expected behavior, without violating POLS.
+ */
+ if (a->inherit_before == 1) {
a->patterns = apr_array_append(p, base->patterns,
over->patterns);
}
Re: svn commit: r1688339 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Mon, Jun 29, 2015 at 9:44 PM, William A Rowe Jr <wr...@rowe-clan.net>
wrote:
> You ALWAYS preserve unset state. How else do you perform the THIRD merge?
>
To be more specific, httpd is allowed to merge whatever merges it likes.
If it wants
to optimize for the directory and then merge the base server to the merged
directory,
that's legit. Overthinking the merge function yields worthless results for
any of the
possible optimization strategies.
Re: svn commit: r1688339 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
You ALWAYS preserve unset state. How else do you perform the THIRD merge?
On Mon, Jun 29, 2015 at 9:01 PM, Yann Ylavic <yl...@gmail.com> wrote:
> This won't work for eg, this second level inheritance: server context
> is on, vhost and inner Location are unset.
> Location->inherit_before will be unset whereas it should be on.
> We should not preserve the unset state IMHO.
>
> On Tue, Jun 30, 2015 at 3:27 AM, <wr...@apache.org> wrote:
> > Author: wrowe
> > Date: Tue Jun 30 01:27:42 2015
> > New Revision: 1688339
> >
> > URL: http://svn.apache.org/r1688339
> > Log:
> > Very difficult to read, and therefore was wrong.
> >
> > Assert that the SubstituteInheritBefore option was explicitly toggled,
> > and do not default in 2.x to this legacy behavior.
> >
> >
> > Modified:
> > httpd/httpd/trunk/modules/filters/mod_substitute.c
> >
> > Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1688339&r1=1688338&r2=1688339&view=diff
> >
> ==============================================================================
> > --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
> > +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Tue Jun 30
> 01:27:42 2015
> > @@ -86,9 +86,16 @@ static void *merge_substitute_dcfg(apr_p
> > subst_dir_conf *base = (subst_dir_conf *) basev;
> > subst_dir_conf *over = (subst_dir_conf *) overv;
> >
> > - a->inherit_before = (over->inherit_before > 0 ||
> (over->inherit_before < 0 &&
> > -
> base->inherit_before > 0));
> > - if (a->inherit_before) {
> > + a->inherit_before = (over->inherit_before != -1)
> > + ? over->inherit_before
> > + : base->inherit_before;
> > + /* SubstituteInheritBefore was the default behavior until 2.5.x,
> > + * and may be re-enabled as desired; this original default behavior
> > + * was to apply inherited subst patterns before locally scoped
> patterns.
> > + * In later 2.2 and 2.4 versions, SubstituteInheritBefore may be
> toggled
> > + * 'off' to follow the corrected/expected behavior, without
> violating POLS.
> > + */
> > + if (a->inherit_before == 1) {
> > a->patterns = apr_array_append(p, base->patterns,
> > over->patterns);
> > }
> >
> >
>
Re: svn commit: r1688339 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Posted by Yann Ylavic <yl...@gmail.com>.
This won't work for eg, this second level inheritance: server context
is on, vhost and inner Location are unset.
Location->inherit_before will be unset whereas it should be on.
We should not preserve the unset state IMHO.
On Tue, Jun 30, 2015 at 3:27 AM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Tue Jun 30 01:27:42 2015
> New Revision: 1688339
>
> URL: http://svn.apache.org/r1688339
> Log:
> Very difficult to read, and therefore was wrong.
>
> Assert that the SubstituteInheritBefore option was explicitly toggled,
> and do not default in 2.x to this legacy behavior.
>
>
> Modified:
> httpd/httpd/trunk/modules/filters/mod_substitute.c
>
> Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1688339&r1=1688338&r2=1688339&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Tue Jun 30 01:27:42 2015
> @@ -86,9 +86,16 @@ static void *merge_substitute_dcfg(apr_p
> subst_dir_conf *base = (subst_dir_conf *) basev;
> subst_dir_conf *over = (subst_dir_conf *) overv;
>
> - a->inherit_before = (over->inherit_before > 0 || (over->inherit_before < 0 &&
> - base->inherit_before > 0));
> - if (a->inherit_before) {
> + a->inherit_before = (over->inherit_before != -1)
> + ? over->inherit_before
> + : base->inherit_before;
> + /* SubstituteInheritBefore was the default behavior until 2.5.x,
> + * and may be re-enabled as desired; this original default behavior
> + * was to apply inherited subst patterns before locally scoped patterns.
> + * In later 2.2 and 2.4 versions, SubstituteInheritBefore may be toggled
> + * 'off' to follow the corrected/expected behavior, without violating POLS.
> + */
> + if (a->inherit_before == 1) {
> a->patterns = apr_array_append(p, base->patterns,
> over->patterns);
> }
>
>
Re: svn commit: r1688339 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
For 2.2/2.4 the delta is a one line change to trunk's behavior;
On Mon, Jun 29, 2015 at 8:27 PM, <wr...@apache.org> wrote:
> Author: wrowe
> Date: Tue Jun 30 01:27:42 2015
> New Revision: 1688339
>
> URL: http://svn.apache.org/r1688339
> Log:
> Very difficult to read, and therefore was wrong.
>
> Assert that the SubstituteInheritBefore option was explicitly toggled,
> and do not default in 2.x to this legacy behavior.
>
>
> Modified:
> httpd/httpd/trunk/modules/filters/mod_substitute.c
>
> Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1688339&r1=1688338&r2=1688339&view=diff
>
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Tue Jun 30 01:27:42
> 2015
> @@ -86,9 +86,16 @@ static void *merge_substitute_dcfg(apr_p
> subst_dir_conf *base = (subst_dir_conf *) basev;
> subst_dir_conf *over = (subst_dir_conf *) overv;
>
> - a->inherit_before = (over->inherit_before > 0 ||
> (over->inherit_before < 0 &&
> -
> base->inherit_before > 0));
> - if (a->inherit_before) {
> + a->inherit_before = (over->inherit_before != -1)
> + ? over->inherit_before
> + : base->inherit_before;
> + /* SubstituteInheritBefore was the default behavior until 2.5.x,
> + * and may be re-enabled as desired; this original default behavior
> + * was to apply inherited subst patterns before locally scoped
> patterns.
> + * In later 2.2 and 2.4 versions, SubstituteInheritBefore may be
> toggled
> + * 'off' to follow the corrected/expected behavior, without violating
> POLS.
> + */
> + if (a->inherit_before == 1) {
>
This becomes
if (a->inherit_before)
both the default case and explicitly toggled case will honor the legacy
2.2/2.4 behavior.
Explicitly toggling 'SubstitueInheritBefore off' will 'upgrade' to the
expected 2.5 behavior.