Skip to content

perf: evaluate tags without macros only once #3691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chweidling wants to merge 3 commits into
base: v3/master
Choose a base branch
from
Open

perf: evaluate tags without macros only once #3691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ebb2cb
perf: evaluate tags without macros only once
chweidling Oct 15, 2026
7c829dd
Take the tag name as std::string_view in hasName()
chweidling Oct 15, 2026
d42e176
Add ctl regression cases for tags with a macro
chweidling Oct 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/actions/tag.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ std::string Tag::getName(Transaction *transaction) {
}


bool Tag::hasName(std::string_view name, Transaction *transaction) {
if (m_staticName) {
return *m_staticName == name;
}
return getName(transaction) == name;
}


bool Tag::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
std::string tag = getName(transaction);
ms_dbg_a(transaction, 9, "Rule tag: " + tag);
Expand Down
13 changes: 12 additions & 1 deletion src/actions/tag.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
*/

#include <string>
#include <string_view>
#include <memory>
#include <optional>
#include <utility>

#include "modsecurity/actions/action.h"
Expand All @@ -34,14 +36,23 @@ class Tag : public Action {
public:
explicit Tag(std::unique_ptr<RunTimeString> z)
: Action("tag"),
m_string(std::move(z)) { }
m_string(std::move(z)),
m_staticName(m_string->containsMacro()
? std::nullopt
: std::optional<std::string>(m_string->evaluate())) { }

std::string getName(Transaction *transaction);
bool hasName(std::string_view name, Transaction *transaction);

bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;

protected:
std::unique_ptr<RunTimeString> m_string;

private:
// Without macros, the name is the same in every transaction and is
// evaluated once. Empty if the tag contains macros.
const std::optional<std::string> m_staticName;
};


Expand Down
2 changes: 1 addition & 1 deletion src/rule_with_actions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,7 @@ void RuleWithActions::executeTransformations(

bool RuleWithActions::containsTag(const std::string& name, Transaction *t) {
for (auto &tag : m_actionsTag) {
if (tag != NULL && tag->getName(t) == name) {
if (tag != NULL && tag->hasName(name, t)) {
return true;
}
}
Expand Down
92 changes: 92 additions & 0 deletions test/test-cases/regression/action-ctl_rule_remove_by_tag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,5 +135,97 @@
"rules": [
"SecRule ARGS:key \".\" \"id:4,ctl:ruleRemoveByTag,whee\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "Testing ctl:ruleRemoveByTag with a macro in the tag, removal applies",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 200
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"",
"SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveByTag=test-a\"",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "Testing ctl:ruleRemoveByTag with a macro in the tag, removal does not apply",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 403
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"",
"SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveByTag=test-a\"",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\""
]
}
]
92 changes: 92 additions & 0 deletions test/test-cases/regression/action-ctl_rule_remove_target_by_tag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,5 +334,97 @@
"SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:9002100,phase:2,pass,ctl:ruleRemoveTargetByTag=CRS;ARGS:foo'bar\"",
"SecRule ARGS \"@contains attack\" \"id:1,phase:3,status:403,block,deny,tag:'CRS'\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "Testing CtlRuleRemoveTargetByTag with a macro in the tag, removal applies",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 200
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"",
"SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveTargetByTag=test-a;ARGS:key\"",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "Testing CtlRuleRemoveTargetByTag with a macro in the tag, removal does not apply",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 403
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"",
"SecRule REQUEST_FILENAME \"@unconditionalMatch\" \"id:3,phase:1,pass,nolog,ctl:ruleRemoveTargetByTag=test-a;ARGS:key\"",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:none,tag:'test-%{tx.suffix}',deny\""
]
}
]
92 changes: 92 additions & 0 deletions test/test-cases/regression/config-update-target-by-tag.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,5 +313,97 @@
"SecRuleUpdateTargetByTag test !ARGS:ref",
"SecRule ARGS \"@contains something\" \"id:1,pass,t:trim,tag:'test',deny\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "SecRuleUpdateTargetByTag Test (8/6) Tag with a macro, exclusion applies",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 200
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=a\"",
"SecRuleUpdateTargetByTag test-a !ARGS",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:trim,tag:'test-%{tx.suffix}',deny\""
]
},
{
"enabled": 1,
"version_min": 300000,
"title": "SecRuleUpdateTargetByTag Test (9/6) Tag with a macro, exclusion does not apply",
"client": {
"ip": "200.249.12.31",
"port": 123
},
"server": {
"ip": "200.249.12.31",
"port": 80
},
"request": {
"headers": {
"Host": "localhost",
"User-Agent": "curl/7.38.0",
"Accept": "*/*",
"Content-Length": "0"
},
"uri": "/?key=value",
"method": "GET",
"body": [
""
]
},
"response": {
"headers": {
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
"Content-Type": "text/html",
"Content-Length": "8"
},
"body": [
"no need."
]
},
"expected": {
"http_code": 403
},
"rules": [
"SecRuleEngine On",
"SecAction \"id:1,phase:1,pass,nolog,setvar:tx.suffix=b\"",
"SecRuleUpdateTargetByTag test-a !ARGS",
"SecRule ARGS \"@contains value\" \"id:2,phase:2,t:trim,tag:'test-%{tx.suffix}',deny\""
]
}
]