On 10/25/2011 12:04 PM, Jim Jagielski wrote: > > On Oct 18, 2011, at 5:59 PM, Stefan Fritsch wrote: > >> On Tue, 18 Oct 2011, Greg Ames wrote: >>> On Fri, Oct 14, 2011 at 2:34 PM, Stefan Fritsch <[email protected]> wrote: >>>> Btw, isn't the nmatch the number of () pairs in the regex? If so, then >>>> enforcing the AP_MAX_REG_MATCH limit could introduce behaviour change >>>> in existing configs: Previously, ap_pregsub on with a regex with more >>>> than 10 capturing () pairs would replace $1 ... $9 with the first nine >>>> matches. But now, it would just return the original string. >>> >>> >>> is there some other reasonable way we can return an error or avoid the >>> problem besides returning the original string? I would prefer that we >>> continue to replace $1 ... $9 in the error case in 2.2.x. >>> >>>> >>>> Also, just returning the original string does not allow to detect >>>> errors. Currently ap_pregsub in 2.2.x always succeeds in that it >>>> replaces $1 to $9. I am against changing this in a way that may return >>>> the unchanged string. Maybe it would be more appropriate to enforce >>>> AP_MAX_REG_MATCH at compile time in ap_regcomp()? Then the errors >>>> would be more obvious to the user. >>>> >>> >>> the latter sounds good. I doubt that very many admins have legitimate use >>> cases for more than AP_MAX_REG_MATCH matches. >> >> This problem seems to be much rarer than I thought. In trunk, only >> ap_rxplus_exec() (which is not in 2.2.x) passes the number of parens to >> ap_pregsub. All other callers pass AP_MAX_REG_MATCH or a smaller constant. >> While I haven't checked the 2.2 code, it seems the danger of behaviour >> changes only exists in third party modules. > > Which would be doing something out-of-the-ordinary if using >10
Well there often a legitimate call to substituting /$1/$2/$3/$1$2$3 (think of the way we break down mod_cache_disk file trees, or how some /homes/ are organized). So as a substitution counter, AP_MAX_REG_///MATCH/// is not the right construct.
