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.
But: What do we gain by limiting nmatch in ap_pregsub()? I don't see that
it would decrease memory usage. We could just document the current
behaviour of only substituting $1 to $9.
