httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe Jr." <>
Subject Re: Really big regex results from ap_pregsub
Date Thu, 13 Oct 2011 20:30:46 GMT
On 10/13/2011 2:32 PM, Jim Jagielski wrote:
> We have 2 conditions:
>   1. nmatch > AP_MAX_REG_MATCH
>   2. really big string
> For #1 we can either choose to return NULL or "". Since
> most calls to ap_pregsub() aren't checked, "" might be safer.
> And, at least, it indicates an error (or can indicate one).
> For #2 we can choose to also limit the returned string size
> and only populate the string up to some large but safe limit
> OR return NULL OR return ""

The largest string value applicable to header values, to URI's
and any presentation string (to errorlog or access log etc) is
MAX_STRING_LEN.  The longest config line is MAX_STRING_LEN.
I don't see a lot of reasons supporting something longer.

> All in all, after really mulling it over, using "" as our
> default error seems safest for pre-2.4. For 2.4, we should
> fix our errors to return NULL (and update our own modules
> to catch that) and fix that API.

Every case of httpd 2.2 using ap_pregsub is for a header value
or a URI value.  All, that is, except the deprecated mod_substitute.
I'd suggest mod_substitute is the exception, and as this is not
a streaming API, a poor choice of API application.  For that single
case, duplicating the ap_pregsub and tuning it appropriately seems
to be the best choice.  See mod_proxy and mod_include for examples
where ap_pregsub was [appropriately] not used.

To quote the source;

/* This function substitutes for $0-$9, filling in regular expression
 * submatches. Pass it the same nmatch and pmatch arguments that you
 * passed ap_regexec(). pmatch should not be greater than the maximum number
 * of subexpressions - i.e. one more than the re_nsub member of ap_regex_t.
 * input should be the string with the $-expressions, source should be the
 * string that was matched against.
 * It returns the substituted string, or NULL on error.
 * Parts of this code are based on Henry Spencer's regsub(), from his
 * AT&T V8 regexp package.

AP_DECLARE(char *) ap_pregsub(apr_pool_t *p, const char *input,
                              const char *source, size_t nmatch,
                              ap_regmatch_t pmatch[])

This was always unambiguous, NULL on error.  The doxygen has *nothing*
to say about the result value.

So... I'd suggest we fix cases that did not expect NULL and return NULL
on any substitution failure.  I don't even see the need for an MMN bump.

View raw message