Repository: corazawaf/coraza
Pull request #727 (2023-03-16 07:49:43)

Use bitset for inferred phases

codecov[bot]: ## [Codecov](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) Report
Patch coverage: **`100.00`**% and no project coverage change.
> Comparison is base [(`c10adce`)](https://codecov.io/gh/corazawaf/coraza/commit/c10adce59e7f902682837e38d9877399fe04e678?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) 81.52% compared to head [(`9bebb88`)](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) 81.53%.

<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           v3/dev     #727   +/-   ##
=======================================
  Coverage   81.52%   81.53%           
=======================================
  Files         148      148           
  Lines        8266     8270    +4     
=======================================
+ Hits         6739     6743    +4     
  Misses       1300     1300           
  Partials      227      227           
```

| Flag | Coverage Δ | |
|---|---|---|
| default | `77.52% <71.42%> (-0.02%)` | :arrow_down: |
| examples | `25.57% <71.42%> (+0.01%)` | :arrow_up: |
| ftw | `49.61% <71.42%> (+<0.01%)` | :arrow_up: |
| ftw-multiphase | `49.73% <100.00%> (+0.02%)` | :arrow_up: |
| tinygo | `76.64% <71.42%> (-0.02%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) | Coverage Δ | |
|---|---|---|
| [internal/corazawaf/rule.go](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf#diff-aW50ZXJuYWwvY29yYXphd2FmL3J1bGUuZ28=) | `77.58% <100.00%> (+0.19%)` | :arrow_up: |
| [internal/corazawaf/rulegroup.go](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf#diff-aW50ZXJuYWwvY29yYXphd2FmL3J1bGVncm91cC5nbw==) | `96.57% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf)

</details>

[:umbrella: View full report in Codecov by Sentry](https://codecov.io/gh/corazawaf/coraza/pull/727?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf).

jcchavezs: I know codecov hasn't complained here but I guess that is because the code added is private. Do you mind adding a couple of simple unit tests?
anuraaga: Sure added test

Patch:
diff --git a/internal/corazawaf/rule.go b/internal/corazawaf/rule.go
index 76af7d9..16f1d1a 100644
--- a/internal/corazawaf/rule.go
+++ b/internal/corazawaf/rule.go
@@ -79,6 +79,16 @@ type ruleTransformationParams struct {
 	Function rules.Transformation
 }
 
+type inferredPhases byte
+
+func (p *inferredPhases) has(phase types.RulePhase) bool {
+	return (*p & (1 << phase)) != 0
+}
+
+func (p *inferredPhases) set(phase types.RulePhase) {
+	*p |= 1 << phase
+}
+
 // Rule is used to test a Transaction against certain operators
 // and execute actions
 type Rule struct {
@@ -138,7 +148,7 @@ type Rule struct {
 
 	// inferredPhases is the inferred phases the rule is relevant for
 	// based on the processed variables.
-	inferredPhases [types.PhaseLogging + 1]bool
+	inferredPhases
 }
 
 func (r *Rule) ParentID() int {
diff --git a/internal/corazawaf/rulegroup.go b/internal/corazawaf/rulegroup.go
index 214f22c..3b7a161 100644
--- a/internal/corazawaf/rulegroup.go
+++ b/internal/corazawaf/rulegroup.go
@@ -34,13 +34,13 @@ func (rg *RuleGroup) Add(rule *Rule) error {
 	}
 
 	numInferred := 0
-	rule.inferredPhases[rule.Phase_] = true
+	rule.inferredPhases.set(rule.Phase_)
 	for _, v := range rule.variables {
 		min := minPhase(v.Variable)
 		if min != 0 {
 			// We infer the earliest phase a variable used by the rule may be evaluated for use when
 			// multiphase evaluation is enabled
-			rule.inferredPhases[min] = true
+			rule.inferredPhases.set(min)
 			numInferred++
 		}
 	}
@@ -128,7 +128,7 @@ RulesLoop:
 		// Rules with phase 0 will always run
 		if r.Phase_ != 0 && r.Phase_ != phase {
 			// Execute the rule in inferred phases too if multiphase evaluation is enabled
-			if !multiphaseEvaluation || !r.inferredPhases[phase] {
+			if !multiphaseEvaluation || !r.inferredPhases.has(phase) {
 				continue
 			}
 		}


Test patch:
diff --git a/internal/corazawaf/rule_test.go b/internal/corazawaf/rule_test.go
index 2a77fab..7f483cb 100644
--- a/internal/corazawaf/rule_test.go
+++ b/internal/corazawaf/rule_test.go
@@ -81,3 +81,34 @@ func TestVariablesRxAreCaseSensitive(t *testing.T) {
 		t.Error("variable key is not case insensitive")
 	}
 }
+
+func TestInferredPhase(t *testing.T) {
+	var b inferredPhases
+
+	if b.has(types.PhaseRequestHeaders) ||
+		b.has(types.PhaseRequestBody) ||
+		b.has(types.PhaseResponseHeaders) ||
+		b.has(types.PhaseResponseBody) {
+		t.Error("unexpected phase")
+	}
+
+	b.set(types.PhaseRequestHeaders)
+	if !b.has(types.PhaseRequestHeaders) {
+		t.Error("expected to have phase")
+	}
+
+	b.set(types.PhaseRequestBody)
+	if !b.has(types.PhaseRequestBody) {
+		t.Error("expected to have phase")
+	}
+
+	b.set(types.PhaseResponseHeaders)
+	if !b.has(types.PhaseResponseHeaders) {
+		t.Error("expected to have phase")
+	}
+
+	b.set(types.PhaseResponseBody)
+	if !b.has(types.PhaseResponseBody) {
+		t.Error("expected to have phase")
+	}
+}