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.
