owasp-modsecurity / owasp-modsecurity/ModSecurity

Performance enhancement calling expand_macros()

Open
#3,010 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

2.x enhancement
Dominant language
C++
Stars
9.8k
Forks
1.8k
Avg merge
2h 46m
Merged PRs (30d)
1

Description

Hello,
I have a (very simple) proposal to optimize a portion of the code that is used more than 40 times in the project:

msc_string *str = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));
str->value = (char *)input_string;
str->value_len = strlen(str->value);
expand_macros(msr, str, rule, msr->mp);
output_string = str->value;

We could optimize that code by first checking that input_string contains a '%' (or even "%{"). If not, no need to allocate the memory, call the function, etc. We add a (very) little overhead for the check, but avoid a much bigger overhead in most of the cases where the value doesn't contain a macro.
That could be done in a wrapper function (the name could be adapted), that would simplify the code as well:

char* expand_macros_wrapper(modsec_rec *msr, char *var, msre_rule *rule, apr_pool_t *mptmp) {
 if (!strstr(var, "%{")) return var;
 msc_string *str = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));
 str->value = var;
 str->value_len = strlen(str->value);
 int changed = expand_macros(msr, str, rule, msr->mp);
if (!changed) return var; // Does this line add anything? Should we always return str->value?
 return str->value;
}

The calling code would simply be:
output_string = expand_macros_wrapper(msr, input_string, rule, msr->mp);

Any remark?

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating expand_macros() and its roughly 40 call sites; inspect how each prepares msc_string and handles the returned value. Compare the proposed wrapper's behavior with current macro expansion, then verify that unchanged inputs avoid allocation and expansion without altering outputs.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.