Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
Wasn't it intended for trunk to change the default behavior?
Regards
Rüdiger
On 08/21/2015 04:19 PM, yla...@apache.org wrote:
Author: ylavic
Date: Fri Aug 21 14:19:17 2015
New Revision: 1697002
URL: http://svn.apache.org/r1697002
Log:
mod_substitute: follow up r1684900.
Really inherit before when asked to (inverse if/else contents).
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=1697002r1=1697001r2=1697002view=diff
==
--- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_substitute.c Fri Aug 21 14:19:17
2015
@@ -97,13 +97,13 @@ static void *merge_substitute_dcfg(apr_p
* '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);
-}
-else {
a-patterns = apr_array_append(p, over-patterns,
base-patterns);
}
+else {
+a-patterns = apr_array_append(p, base-patterns,
+ over-patterns);
+}
a-max_line_length = over-max_line_length_set ?
over-max_line_length : base-max_line_length;
a-max_line_length_set = over-max_line_length_set
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote: On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote: Wasn't it intended for trunk to change the default behavior? Yes it was, still when inherit_before is set we need to do that. I meant, we need to do apr_array_append(p, over-patterns, base-patterns) and not the inverse.
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On 08/21/2015 04:31 PM, Yann Ylavic wrote: On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote: On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote: Wasn't it intended for trunk to change the default behavior? Yes it was, still when inherit_before is set we need to do that. I meant, we need to do apr_array_append(p, over-patterns, base-patterns) and not the inverse. But apr_array_append(p, over-patterns, base-patterns) means that we do NOT inherit the stuff from the base configuration before the configuration of our section. So the code before your recent patch was correct. Regards Rüdiger
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote:
Wasn't it intended for trunk to change the default behavior?
Yes it was, still when inherit_before is set we need to do that.
The code in trunk is:
if (a-inherit_before == 1) {
whereas 2.4 and 2.2 (would) have:
if (a-inherit_before) { = can be 1 but also -1
Regards,
Yann.
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 10:05 AM, Yann Ylavic ylavic@gmail.com wrote: On Fri, Aug 21, 2015 at 4:36 PM, Ruediger Pluem rpl...@apache.org wrote: On 08/21/2015 04:31 PM, Yann Ylavic wrote: On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote: On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote: Wasn't it intended for trunk to change the default behavior? Yes it was, still when inherit_before is set we need to do that. I meant, we need to do apr_array_append(p, over-patterns, base-patterns) and not the inverse. But apr_array_append(p, over-patterns, base-patterns) means that we do NOT inherit the stuff from the base configuration before the configuration of our section. So the code before your recent patch was correct. Hm, you are right, I was misleaded by the comment in trunk: /* SubstituteInheritBefore was the default behavior until 2.5.x, ... */ since SubstituteInheritBefore was *not* the default behavior until 2.5.x. To recap, apr_array_append(p, over-patterns, base-patterns) is the legacy but it is obviously not the inherited base which is merged before... So to keep the existing/prior-to-r1697002 logic in the code (which was good), I propose to revert r1697002 and rename SubstituteInheritBefore to SubstituteInheritAfter, WDYT? It's a toggle, please leave it alone. The default state will be one case in 2.4.x, and a different case in 2.5.x and later, but a user who puts that directive into their config will observe the same behavior when they run their config under 2.4 or later under 2.6.
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 4:36 PM, Ruediger Pluem rpl...@apache.org wrote: On 08/21/2015 04:31 PM, Yann Ylavic wrote: On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote: On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote: Wasn't it intended for trunk to change the default behavior? Yes it was, still when inherit_before is set we need to do that. I meant, we need to do apr_array_append(p, over-patterns, base-patterns) and not the inverse. But apr_array_append(p, over-patterns, base-patterns) means that we do NOT inherit the stuff from the base configuration before the configuration of our section. So the code before your recent patch was correct. Hm, you are right, I was misleaded by the comment in trunk: /* SubstituteInheritBefore was the default behavior until 2.5.x, ... */ since SubstituteInheritBefore was *not* the default behavior until 2.5.x. To recap, apr_array_append(p, over-patterns, base-patterns) is the legacy but it is obviously not the inherited base which is merged before... So to keep the existing/prior-to-r1697002 logic in the code (which was good), I propose to revert r1697002 and rename SubstituteInheritBefore to SubstituteInheritAfter, WDYT?
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 5:46 PM, William A Rowe Jr wr...@rowe-clan.net wrote: On Fri, Aug 21, 2015 at 10:32 AM, William A Rowe Jr wr...@rowe-clan.net wrote: It's a toggle, please leave it alone. The default state will be one case in 2.4.x, and a different case in 2.5.x and later, but a user who puts that directive into their config will observe the same behavior when they run their config under 2.4 or later under 2.6. (But correcting the comments and the docs in the respective code branches as applicable would be terrific, so users and the rest of us aren't confused again when this is reviewed again in the future.) Done in r1697013 and r1697015. Backports proposals to 2.4.x and 2.2.x updated to v5 too. Thanks Ruediger and Bill for the review.
Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c
On Fri, Aug 21, 2015 at 10:32 AM, William A Rowe Jr wr...@rowe-clan.net wrote: It's a toggle, please leave it alone. The default state will be one case in 2.4.x, and a different case in 2.5.x and later, but a user who puts that directive into their config will observe the same behavior when they run their config under 2.4 or later under 2.6. (But correcting the comments and the docs in the respective code branches as applicable would be terrific, so users and the rest of us aren't confused again when this is reviewed again in the future.)
