Repository: corazawaf/coraza
Pull request #607 (2023-01-31 08:03:35)

Use unknown instead of nil for macro without variable
Should be a bit faster, I think it's clearer too

codecov-commenter: # [Codecov](https://codecov.io/gh/corazawaf/coraza/pull/607?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) Report
Base: **77.58**% // Head: **77.58**% // No change to project coverage :thumbsup:
> Coverage data is based on head [(`4d61d31`)](https://codecov.io/gh/corazawaf/coraza/pull/607?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) compared to base [(`37b5a3b`)](https://codecov.io/gh/corazawaf/coraza/commit/37b5a3bd3732b09af0fa170bcc48375026b9f6d5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf).
> Patch coverage: 100.00% of modified lines in pull request are covered.

:mega: This organization is not using Codecov’s [GitHub App Integration](https://github.com/apps/codecov). We recommend you install it so Codecov can continue to function properly for your repositories. [Learn more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf)

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


```diff
@@           Coverage Diff           @@
##           v3/dev     #607   +/-   ##
=======================================
  Coverage   77.58%   77.58%           
=======================================
  Files         144      144           
  Lines        7481     7481           
=======================================
  Hits         5804     5804           
  Misses       1446     1446           
  Partials      231      231           
```

| Flag | Coverage Δ | |
|---|---|---|
| default | `73.00% <100.00%> (ø)` | |
| examples | `28.37% <40.00%> (ø)` | |
| ftw | `54.67% <100.00%> (ø)` | |
| ftw-multiphase | `54.77% <100.00%> (ø)` | |
| tinygo | `71.13% <100.00%> (ø)` | |

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/607?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf) | Coverage Δ | |
|---|---|---|
| [macro/macro.go](https://codecov.io/gh/corazawaf/coraza/pull/607?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=corazawaf#diff-bWFjcm8vbWFjcm8uZ28=) | `86.36% <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 at Codecov](https://codecov.io/gh/corazawaf/coraza/pull/607?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).


Patch:
diff --git a/http/middleware.go b/http/middleware.go
index 16345ae..7b53051 100644
--- a/http/middleware.go
+++ b/http/middleware.go
@@ -44,9 +44,13 @@ func processRequest(tx types.Transaction, req *http.Request) (*types.Interruptio
 			tx.AddRequestHeader(k, v)
 		}
 	}
-	// Host will always be removed from req.Headers(), so we manually add it
+
+	// Host will always be removed from req.Headers() and promoted to the
+	// Request.Host field, so we manually add it
 	if req.Host != "" {
 		tx.AddRequestHeader("Host", req.Host)
+		// This connector relies on the host header (now host field) to populate ServerName
+		tx.SetServerName(req.Host)
 	}
 
 	in = tx.ProcessRequestHeaders()
diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go
index 14de634..8443ea5 100644
--- a/internal/corazawaf/transaction.go
+++ b/internal/corazawaf/transaction.go
@@ -340,14 +340,15 @@ func (tx *Transaction) AddRequestHeader(key string, value string) {
 	tx.variables.requestHeadersNames.AddUniqueCS(keyl, key, keyl)
 	tx.variables.requestHeaders.AddCS(keyl, key, value)
 
-	if keyl == "content-type" {
+	switch keyl {
+	case "content-type":
 		val := strings.ToLower(value)
 		if val == "application/x-www-form-urlencoded" {
 			tx.variables.reqbodyProcessor.Set("URLENCODED")
 		} else if strings.HasPrefix(val, "multipart/form-data") {
 			tx.variables.reqbodyProcessor.Set("MULTIPART")
 		}
-	} else if keyl == "cookie" {
+	case "cookie":
 		// Cookies use the same syntax as GET params but with semicolon (;) separator
 		values := urlutil.ParseQuery(value, ';')
 		for k, vr := range values {
@@ -631,7 +632,6 @@ func (tx *Transaction) RemoveRuleByID(id int) {
 // ProcessConnection should be called at very beginning of a request process, it is
 // expected to be executed prior to the virtual host resolution, when the
 // connection arrives on the server.
-// Important: Remember to check for a possible intervention.
 func (tx *Transaction) ProcessConnection(client string, cPort int, server string, sPort int) {
 	p := strconv.Itoa(cPort)
 	p2 := strconv.Itoa(sPort)
@@ -742,6 +742,17 @@ func (tx *Transaction) ProcessURI(uri string, method string, httpVersion string)
 	tx.variables.queryString.Set(query)
 }
 
+// SetServerName allows to set server name details.
+//
+// The API consumer is in charge of retrieving the value (e.g. from the host header).
+// It is expected to be executed before calling ProcessRequestHeaders.
+func (tx *Transaction) SetServerName(serverName string) {
+	if tx.LastPhase >= types.PhaseRequestHeaders {
+		tx.WAF.Logger.Warn("SetServerName has been called after ProcessRequestHeaders")
+	}
+	tx.variables.serverName.Set(serverName)
+}
+
 // ProcessRequestHeaders Performs the analysis on the request readers.
 //
 // This method perform the analysis on the request headers, notice however
diff --git a/macro/macro.go b/macro/macro.go
index f328699..11d7fbf 100644
--- a/macro/macro.go
+++ b/macro/macro.go
@@ -29,7 +29,7 @@ func NewMacro(data string) (Macro, error) {
 
 type macroToken struct {
 	text     string
-	variable *variables.RuleVariable
+	variable variables.RuleVariable
 	key      string
 }
 
@@ -59,10 +59,10 @@ func (m *macro) Expand(tx rules.TransactionState) string {
 }
 
 func expandToken(tx rules.TransactionState, token macroToken) string {
-	if token.variable == nil {
+	if token.variable == variables.Unknown {
 		return token.text
 	}
-	switch col := tx.Collection(*token.variable).(type) {
+	switch col := tx.Collection(token.variable).(type) {
 	case *collection.Map:
 		if c := col.Get(token.key); len(c) > 0 {
 			return c[0]
@@ -100,7 +100,7 @@ func (m *macro) compile(input string) error {
 				// we add the text token
 				m.tokens = append(m.tokens, macroToken{
 					text:     currentToken.String(),
-					variable: nil,
+					variable: variables.Unknown,
 					key:      "",
 				})
 			}
@@ -121,7 +121,7 @@ func (m *macro) compile(input string) error {
 				// we add the variable token
 				m.tokens = append(m.tokens, macroToken{
 					text:     currentToken.String(),
-					variable: &v,
+					variable: v,
 					key:      strings.ToLower(key),
 				})
 				currentToken.Reset()
@@ -137,7 +137,7 @@ func (m *macro) compile(input string) error {
 	if currentToken.Len() > 0 {
 		m.tokens = append(m.tokens, macroToken{
 			text:     currentToken.String(),
-			variable: nil,
+			variable: variables.Unknown,
 			key:      "",
 		})
 	}
diff --git a/types/transaction.go b/types/transaction.go
index ec1640c..637e217 100644
--- a/types/transaction.go
+++ b/types/transaction.go
@@ -33,7 +33,6 @@ type Transaction interface {
 	// ProcessConnection should be called at very beginning of a request process, it is
 	// expected to be executed prior to the virtual host resolution, when the
 	// connection arrives on the server.
-	// Important: Remember to check for a possible intervention.
 	ProcessConnection(client string, cPort int, server string, sPort int)
 
 	// ProcessURI Performs the analysis on the URI and all the query string variables.
@@ -48,6 +47,13 @@ type Transaction interface {
 	// note: This function won't add GET arguments, they must be added with AddArgument
 	ProcessURI(uri string, method string, httpVersion string)
 
+	// SetServerName allows to set server name details.
+	// The API consumer is in charge of retrieving the value (e.g. from the host header)
+	// before providing it to this method.
+	// In order to be able to check SERVER_NAME variable since phase 1, it is expected
+	// to execute SetServerName before calling ProcessRequestHeaders.
+	SetServerName(serverName string)
+
 	// AddRequestHeader Adds a request header
 	//
 	// With this method it is possible to feed Coraza with a request header.


Test patch:
diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go
index 3e13ba4..3c42612 100644
--- a/internal/corazawaf/transaction_test.go
+++ b/internal/corazawaf/transaction_test.go
@@ -946,6 +946,28 @@ func TestTxProcessConnection(t *testing.T) {
 	}
 }
 
+func TestTxSetServerName(t *testing.T) {
+
+	l := &inspectableLogger{}
+	waf := NewWAF()
+	waf.Logger.SetOutput(l)
+	waf.Logger.SetLevel(loggers.LogLevelWarn)
+	tx := waf.NewTransaction()
+	tx.LastPhase = types.PhaseRequestHeaders
+	tx.SetServerName("coraza.io")
+	if tx.variables.serverName.String() != "coraza.io" {
+		t.Error("failed to set server name")
+	}
+	if want, have := 1, len(l.entries); want != have {
+		t.Fatalf("unexpected number of log entries, want %d, have %d", want, have)
+	}
+
+	if want, have := "SetServerName has been called after ProcessRequestHeaders", l.entries[0]; !strings.Contains(have, want) {
+		t.Fatalf("unexpected message, want %q, have %q", want, have)
+	}
+
+}
+
 func TestTxAddArgument(t *testing.T) {
 	waf := NewWAF()
 	tx := waf.NewTransaction()