Am 13.10.2011 22:30, schrieb William A. Rowe Jr.:
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.
This no longer true on trunk. The longest config line is VARBUF_MAX_LEN
which is 16 MB.
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.
+1
Regards
Rüdiger
