deprecating Logdata field in transaction

corazawaf · Oct 22, 2023 · 2c955da · 2c955da
1 parent 955938a
commit 2c955da
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 7 deletions.
diff --git a/internal/actions/logdata.go b/internal/actions/logdata.go
@@ -37,7 +37,7 @@ func (a *logdataFn) Init(r plugintypes.RuleMetadata, data string) error {
 }
 
 func (a *logdataFn) Evaluate(r plugintypes.RuleMetadata, tx plugintypes.TransactionState) {
-	tx.(*corazawaf.Transaction).Logdata = r.(*corazawaf.Rule).LogData.Expand(tx)
+	// logdata macro expansion is performed after all other actions have been evaluated (and potentially all the needed variables have been set)
 }
 
 func (a *logdataFn) Type() plugintypes.ActionType {

diff --git a/internal/corazawaf/rule.go b/internal/corazawaf/rule.go
@@ -342,7 +342,7 @@ func (r *Rule) doEvaluate(phase types.RulePhase, tx *Transaction, collectiveMatc
 		}
 
 		// Expansion of Msg and LogData is postponed here. It allows to run it only if the whole rule/chain
-		// matches and to rely on MATCHED_* variables updated by the chain, not just by the fist rule.
+		// matches and to rely on MATCHED_* variables updated by the chain, not just by the first rule.
 		if !r.MultiMatch {
 			if r.Msg != nil {
 				matchedValues[0].(*corazarules.MatchData).Message_ = r.Msg.Expand(tx)

diff --git a/internal/corazawaf/rule_test.go b/internal/corazawaf/rule_test.go
@@ -95,7 +95,8 @@ func (*dummyFlowAction) Init(_ plugintypes.RuleMetadata, _ string) error {
 }
 
 func (*dummyFlowAction) Evaluate(_ plugintypes.RuleMetadata, tx plugintypes.TransactionState) {
-	tx.(*Transaction).Logdata = "flow action triggered"
+	// SkipAfter is used in a improper way, just for testing purposes ensuring that the action has been enforced
+	tx.(*Transaction).SkipAfter = "flow action triggered"
 }
 
 func (*dummyFlowAction) Type() plugintypes.ActionType {
@@ -116,7 +117,7 @@ func TestFlowActionIfDetectionOnlyEngine(t *testing.T) {
 	if len(matchdata) != 1 {
 		t.Errorf("Expected 1 matchdata, got %d", len(matchdata))
 	}
-	if tx.Logdata != "flow action triggered" {
+	if tx.SkipAfter != "flow action triggered" {
 		t.Errorf("Expected flow action triggered with DetectionOnly engine")
 	}
 }
@@ -128,7 +129,7 @@ func (*dummyNonDisruptiveAction) Init(_ plugintypes.RuleMetadata, _ string) erro
 }
 
 func (*dummyNonDisruptiveAction) Evaluate(_ plugintypes.RuleMetadata, tx plugintypes.TransactionState) {
-	tx.(*Transaction).Logdata = "action enforced"
+	tx.(*Transaction).SkipAfter = "action enforced"
 }
 
 func (*dummyNonDisruptiveAction) Type() plugintypes.ActionType {
@@ -142,7 +143,7 @@ func TestMatchVariableRunsActionTypeNondisruptive(t *testing.T) {
 	action := &dummyNonDisruptiveAction{}
 	_ = rule.AddAction("dummyNonDisruptiveAction", action)
 	rule.matchVariable(tx, md)
-	if tx.Logdata != "action enforced" {
+	if tx.SkipAfter != "action enforced" {
 		t.Errorf("Expected non disruptive action to be enforced during matchVariable")
 	}
 }

diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go
@@ -48,6 +48,7 @@ type Transaction struct {
 	interruption *types.Interruption
 
 	// This is used to store log messages
+	// Deprecated since Coraza 3.0.5: this variable is not used, logdata values are stored in the matched rules
 	Logdata string
 
 	// Rules will be skipped after a rule with this SecMarker is found

diff --git a/internal/corazawaf/waf.go b/internal/corazawaf/waf.go
@@ -153,7 +153,7 @@ func (w *WAF) newTransactionWithID(id string) *Transaction {
 	tx.id = id
 	tx.matchedRules = []types.MatchedRule{}
 	tx.interruption = nil
-	tx.Logdata = ""
+	tx.Logdata = "" // Deprecated, this variable is not used. Logdata for each matched rule is stored in the MatchData field.
 	tx.SkipAfter = ""
 	tx.AuditEngine = w.AuditEngine
 	tx.AuditLogParts = w.AuditLogParts