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

Reply via email to