Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Jim Jagielski wrote: Just looking for verification here ;) No worries. Thx! On Mar 19, 2014, at 12:46 PM, Jim Jagielski j...@jagunet.com wrote: As noted, from how I understand it, currently we allow it to build BUT the behavior is not as expected or designed, since the expected behavior *requires* PCRE_DUPNAMES. If we require PCRE_DUPNAMES then we require it, right? Correct. As far as I remember Grahams proposal was to put the different behaviors in the documentation as the behavior only differs in specific cases (same capture name used more then once). So IMHO the feature still makes sense even without PCRE_DUPNAMES available. To be honest that documentation part somehow felt through the cracks :-). Regards Rüdiger
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
As I understand it, we require PCRE_DUPNAMES functionality, right? So I think we need to check for it at configure/build time and bail if it isn't available. The configure part is done in http://svn.apache.org/r1579259
RE: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
-Original Message- From: Jim Jagielski Sent: Mittwoch, 19. März 2014 16:37 To: httpd Subject: Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c As I understand it, we require PCRE_DUPNAMES functionality, right? So I think we need to check for it at configure/build time and bail if it isn't available. The configure part is done in http://svn.apache.org/r1579259 Keep in mind that this causes trunk to fail building on RHEL 5 with the system provided PCRE
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
As noted, from how I understand it, currently we allow it to build BUT the behavior is not as expected or designed, since the expected behavior *requires* PCRE_DUPNAMES. If we require PCRE_DUPNAMES then we require it, right? On Mar 19, 2014, at 12:26 PM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: -Original Message- From: Jim Jagielski Sent: Mittwoch, 19. März 2014 16:37 To: httpd Subject: Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c server/util_pcre.c As I understand it, we require PCRE_DUPNAMES functionality, right? So I think we need to check for it at configure/build time and bail if it isn't available. The configure part is done in http://svn.apache.org/r1579259 Keep in mind that this causes trunk to fail building on RHEL 5 with the system provided PCRE
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Just looking for verification here ;) Thx! On Mar 19, 2014, at 12:46 PM, Jim Jagielski j...@jagunet.com wrote: As noted, from how I understand it, currently we allow it to build BUT the behavior is not as expected or designed, since the expected behavior *requires* PCRE_DUPNAMES. If we require PCRE_DUPNAMES then we require it, right?
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Sounds good to me :) On Jan 14, 2014, at 2:41 PM, Ruediger Pluem rpl...@apache.org wrote: Ping? Regards Rüdiger Ruediger Pluem wrote: minf...@apache.org wrote: Author: minfrin Date: Mon Dec 30 19:50:52 2013 New Revision: 1554300 URL: http://svn.apache.org/r1554300 Log: core: Support named groups and backreferences within the LocationMatch, DirectoryMatch, FilesMatch and ProxyMatch directives. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/include/ap_mmn.h httpd/httpd/trunk/include/ap_regex.h httpd/httpd/trunk/include/http_core.h httpd/httpd/trunk/modules/proxy/mod_proxy.c httpd/httpd/trunk/modules/proxy/mod_proxy.h httpd/httpd/trunk/server/core.c httpd/httpd/trunk/server/request.c httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300r1=1554299r2=1554300view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013 #define APR_WANT_STRFUNC @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -int options = 0; +int options = PCRE_DUPNAMES; This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6 on RHEL 5. How about Index: util_pcre.c === --- util_pcre.c (revision 1556947) +++ util_pcre.c (working copy) @@ -125,7 +125,12 @@ const char *errorptr; int erroffset; int errcode = 0; +/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +#ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; +#else +int options = 0; +#endif if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; instead? if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; Regards Rüdiger
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
On 15 Jan 2014, at 3:04 PM, Jim Jagielski j...@jagunet.com wrote: Sounds good to me :) Had to do some digging to get my head around the impact. If the PCRE_DUPNAMES is missing, the list of names of variables is shorter than the list of variables defined, and you could have a variable value applied to the wrong name. I think we can live with this as long as we clearly document that people should expect undefined behaviour on older versions of pcre if they use duplicate names inside regexes. Example: /(?sitename[^/]+)/(?sitename[^/]+)/(?othername[^/]+) In older pcre, the second captured value sitename will be applied to othername. Regards, Graham --
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Ahhh. Likely we can catch this at build time via configure On Jan 15, 2014, at 8:15 AM, Graham Leggett minf...@sharp.fm wrote: On 15 Jan 2014, at 3:04 PM, Jim Jagielski j...@jagunet.com wrote: Sounds good to me :) Had to do some digging to get my head around the impact. If the PCRE_DUPNAMES is missing, the list of names of variables is shorter than the list of variables defined, and you could have a variable value applied to the wrong name. I think we can live with this as long as we clearly document that people should expect undefined behaviour on older versions of pcre if they use duplicate names inside regexes. Example: /(?sitename[^/]+)/(?sitename[^/]+)/(?othername[^/]+) In older pcre, the second captured value sitename will be applied to othername. Regards, Graham --
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Ping? Regards Rüdiger Ruediger Pluem wrote: minf...@apache.org wrote: Author: minfrin Date: Mon Dec 30 19:50:52 2013 New Revision: 1554300 URL: http://svn.apache.org/r1554300 Log: core: Support named groups and backreferences within the LocationMatch, DirectoryMatch, FilesMatch and ProxyMatch directives. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/include/ap_mmn.h httpd/httpd/trunk/include/ap_regex.h httpd/httpd/trunk/include/http_core.h httpd/httpd/trunk/modules/proxy/mod_proxy.c httpd/httpd/trunk/modules/proxy/mod_proxy.h httpd/httpd/trunk/server/core.c httpd/httpd/trunk/server/request.c httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300r1=1554299r2=1554300view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013 #define APR_WANT_STRFUNC @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -int options = 0; +int options = PCRE_DUPNAMES; This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6 on RHEL 5. How about Index: util_pcre.c === --- util_pcre.c (revision 1556947) +++ util_pcre.c (working copy) @@ -125,7 +125,12 @@ const char *errorptr; int erroffset; int errcode = 0; +/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +#ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; +#else +int options = 0; +#endif if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; instead? if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; Regards Rüdiger
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
minf...@apache.org wrote: Author: minfrin Date: Mon Dec 30 19:50:52 2013 New Revision: 1554300 URL: http://svn.apache.org/r1554300 Log: core: Support named groups and backreferences within the LocationMatch, DirectoryMatch, FilesMatch and ProxyMatch directives. Modified: httpd/httpd/trunk/CHANGES httpd/httpd/trunk/include/ap_mmn.h httpd/httpd/trunk/include/ap_regex.h httpd/httpd/trunk/include/http_core.h httpd/httpd/trunk/modules/proxy/mod_proxy.c httpd/httpd/trunk/modules/proxy/mod_proxy.h httpd/httpd/trunk/server/core.c httpd/httpd/trunk/server/request.c httpd/httpd/trunk/server/util_pcre.c Modified: httpd/httpd/trunk/server/util_pcre.c URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/util_pcre.c?rev=1554300r1=1554299r2=1554300view=diff == --- httpd/httpd/trunk/server/util_pcre.c (original) +++ httpd/httpd/trunk/server/util_pcre.c Mon Dec 30 19:50:52 2013 #define APR_WANT_STRFUNC @@ -124,7 +125,7 @@ AP_DECLARE(int) ap_regcomp(ap_regex_t * const char *errorptr; int erroffset; int errcode = 0; -int options = 0; +int options = PCRE_DUPNAMES; This fails to compile on older PCRE versions that do not know PCRE_DUPNAMES, like 6.6 on RHEL 5. How about Index: util_pcre.c === --- util_pcre.c (revision 1556947) +++ util_pcre.c (working copy) @@ -125,7 +125,12 @@ const char *errorptr; int erroffset; int errcode = 0; +/* PCRE_DUPNAMES is only present in more recent versions of PCRE */ +#ifdef PCRE_DUPNAMES int options = PCRE_DUPNAMES; +#else +int options = 0; +#endif if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; instead? if ((cflags AP_REG_ICASE) != 0) options |= PCRE_CASELESS; Regards Rüdiger
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
On 01 Jan 2014, at 7:26 PM, Stefan Fritsch s...@sfritsch.de wrote: I am in favor of adding a prefix. If there are important use cases for setting arbitrary variables, one could (later) add a special opt-in mechanism, e.g. using noprefix:foo in the regex leads to variable foo without the prefix being set. MATCH_ prefix added in r1555266, and the backport vote reset. Regards, Graham --
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
On 01.01.2014 18:26, Stefan Fritsch wrote: Am Mittwoch, 1. Januar 2014, 14:06:17 schrieb Graham Leggett: Maybe making ap_regname() accept an optional prefix string that is prepended to each name would be a good idea? Maybe the use in LocationMatch and friends should add some prefix to the names? Like m_ or match_ or m:? This would make it more difficult to shoot oneself in the foot by allowing a remote attacker to set env variables that have some special meanings elsewhere in httpd (or in an executed cgi script). And/or maybe these values should be filtered out again when exporting them to cgi env variables? I wondered about this, on one hand it is nice to be able to set any variable, but on the other hand there is a lot of safety in preventing someone from being able to shadow an existing variable. I had MATCH_FOO in mind originally. I am in favor of adding a prefix. If there are important use cases for setting arbitrary variables, one could (later) add a special opt-in mechanism, e.g. using noprefix:foo in the regex leads to variable foo without the prefix being set. +1, mandatory prefix for now, optional configurability. Default should be with prefix. Of your three examples I like match_ best. m_ might be less likely for future conflicts, m: looks good, but I don't know whether then using the variable name in other config directives could lead to problems because of the colon in the name. Thanks for pushing this nice enhancement! Regards, Rainer
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Am Montag, 30. Dezember 2013, 19:50:53 schrieb minf...@apache.org: Author: minfrin Date: Mon Dec 30 19:50:52 2013 New Revision: 1554300 URL: http://svn.apache.org/r1554300 Log: core: Support named groups and backreferences within the LocationMatch, DirectoryMatch, FilesMatch and ProxyMatch directives. I definitely like this idea. While I haven't done a full review of the patch, I have a few questions: Aren't the apr_table keys case insensitive anyway? Why do we need the case conversion of the key names? Maybe making ap_regname() accept an optional prefix string that is prepended to each name would be a good idea? Maybe the use in LocationMatch and friends should add some prefix to the names? Like m_ or match_ or m:? This would make it more difficult to shoot oneself in the foot by allowing a remote attacker to set env variables that have some special meanings elsewhere in httpd (or in an executed cgi script). And/or maybe these values should be filtered out again when exporting them to cgi env variables? Cheers, Stefan
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
On 01 Jan 2014, at 1:59 PM, Stefan Fritsch s...@sfritsch.de wrote: I definitely like this idea. While I haven't done a full review of the patch, I have a few questions: Aren't the apr_table keys case insensitive anyway? Why do we need the case conversion of the key names? All the variables in subprocess_env are all uppecased, before I added the uppercasing the variables were the only ones lowercased when they were listed and it looked wrong. Maybe making ap_regname() accept an optional prefix string that is prepended to each name would be a good idea? Maybe the use in LocationMatch and friends should add some prefix to the names? Like m_ or match_ or m:? This would make it more difficult to shoot oneself in the foot by allowing a remote attacker to set env variables that have some special meanings elsewhere in httpd (or in an executed cgi script). And/or maybe these values should be filtered out again when exporting them to cgi env variables? I wondered about this, on one hand it is nice to be able to set any variable, but on the other hand there is a lot of safety in preventing someone from being able to shadow an existing variable. I had MATCH_FOO in mind originally. Regards, Graham --
Re: svn commit: r1554300 - in /httpd/httpd/trunk: CHANGES include/ap_mmn.h include/ap_regex.h include/http_core.h modules/proxy/mod_proxy.c modules/proxy/mod_proxy.h server/core.c server/request.c ser
Am Mittwoch, 1. Januar 2014, 14:06:17 schrieb Graham Leggett: Maybe making ap_regname() accept an optional prefix string that is prepended to each name would be a good idea? Maybe the use in LocationMatch and friends should add some prefix to the names? Like m_ or match_ or m:? This would make it more difficult to shoot oneself in the foot by allowing a remote attacker to set env variables that have some special meanings elsewhere in httpd (or in an executed cgi script). And/or maybe these values should be filtered out again when exporting them to cgi env variables? I wondered about this, on one hand it is nice to be able to set any variable, but on the other hand there is a lot of safety in preventing someone from being able to shadow an existing variable. I had MATCH_FOO in mind originally. I am in favor of adding a prefix. If there are important use cases for setting arbitrary variables, one could (later) add a special opt-in mechanism, e.g. using noprefix:foo in the regex leads to variable foo without the prefix being set.
