diff --git a/CHANGES b/CHANGES index 6ec934d5..1ff92d19 100644 --- a/CHANGES +++ b/CHANGES @@ -8,7 +8,8 @@ * Do not log 'allow' action as intercepted in the debug log. -* Optimize regex execution to not capture unless 'capture' action used. +* Warn if a regular expression captures subexpressions, but the + "capture" action was not specified. * Performance improvements in memory management. diff --git a/apache2/msc_pcre.c b/apache2/msc_pcre.c index 4729d3b9..11fd9e72 100644 --- a/apache2/msc_pcre.c +++ b/apache2/msc_pcre.c @@ -91,3 +91,11 @@ int msc_regexec(msc_regex_t *regex, const char *s, unsigned int slen, return msc_regexec_capture(regex, s, slen, NULL, 0, error_msg); } + +/** + * Gets info on a compiled regex. + */ +int msc_fullinfo(msc_regex_t *regex, int what, void *where) +{ + return pcre_fullinfo(regex->re, regex->pe, what, where); +} diff --git a/apache2/msc_pcre.h b/apache2/msc_pcre.h index 8cc9dc33..d9af38b5 100644 --- a/apache2/msc_pcre.h +++ b/apache2/msc_pcre.h @@ -36,4 +36,6 @@ int DSOLOCAL msc_regexec_capture(msc_regex_t *regex, const char *s, int DSOLOCAL msc_regexec(msc_regex_t *regex, const char *s, unsigned int slen, char **error_msg); +int DSOLOCAL msc_fullinfo(msc_regex_t *regex, int what, void *where); + #endif diff --git a/apache2/re_operators.c b/apache2/re_operators.c index ff595e4f..30fb62aa 100644 --- a/apache2/re_operators.c +++ b/apache2/re_operators.c @@ -108,18 +108,19 @@ static int msre_op_rx_execute(modsec_rec *msr, msre_rule *rule, msre_var *var, c /* Are we supposed to capture subexpressions? */ capture = apr_table_get(rule->actionset->actions, "capture") ? 1 : 0; - if (capture) { - if (msr->txcfg->debuglog_level >= 9) { - msr_log(msr, 9, "Using captured regex execution."); + /* Warn when the regex captures but "capture" is not set */ + if (msr->txcfg->debuglog_level >= 2) { + int capcount; + rc = msc_fullinfo(regex, PCRE_INFO_CAPTURECOUNT, &capcount); + if ((capture == 0) && (capcount > 0)) { + msr_log(msr, 2, "Warning. regex captures, but \"capture\" action not set."); } - rc = msc_regexec_capture(regex, target, target_length, ovector, 30, &my_error_msg); - } - else { - if (msr->txcfg->debuglog_level >= 9) { - msr_log(msr, 9, "Using uncaptured regex execution."); - } - rc = msc_regexec(regex, target, target_length, &my_error_msg); } + + /* We always use capture so that ovector can be used as working space + * and no memory has to be allocated for any backreferences. + */ + rc = msc_regexec_capture(regex, target, target_length, ovector, 30, &my_error_msg); if (rc < -1) { *error_msg = apr_psprintf(msr->mp, "Regex execution failed: %s", my_error_msg); return -1;