Repository: corazawaf/coraza (go)

Issue:
Split transaction AddArgument into AddRequest/ResponseArgument
We support request and response arguments, so we must make the difference.

Pull request:
refactor tx addargument
Closes #631

It splits tx.AddArguments into 4 functions for GET, POST, PATH, and RESPONSE

Patch:
diff --git a/.github/workflows/regression.yml b/.github/workflows/regression.yml
index 1d71075..c7627ad 100644
--- a/.github/workflows/regression.yml
+++ b/.github/workflows/regression.yml
@@ -30,27 +30,32 @@ jobs:
           cache: true
       - name: Tests and coverage
         run: go run mage.go coverage
-      - uses: codecov/codecov-action@v2
+      - name: "Codecov: General"
+        uses: codecov/codecov-action@v3
         if: ${{ matrix.go-version == '1.19.x' }}
         with:
           files: build/coverage.txt
           flags: default
-      - uses: codecov/codecov-action@v2
+      - name: "Codecov: Examples"
+        uses: codecov/codecov-action@v3
         if: ${{ matrix.go-version == '1.19.x' }}
         with:
           files: build/coverage-examples.txt
           flags: examples
-      - uses: codecov/codecov-action@v2
+      - name: "Codecov: FTW"
+        uses: codecov/codecov-action@v3
         if: ${{ matrix.go-version == '1.19.x' }}
         with:
           files: build/coverage-ftw.txt
           flags: ftw
-      - uses: codecov/codecov-action@v2
+      - name: "Codecov: FTW Multiphase tag"
+        uses: codecov/codecov-action@v3
         if: ${{ matrix.go-version == '1.19.x' }}
         with:
           files: build/coverage-ftw-multiphase.txt
           flags: ftw-multiphase
-      - uses: codecov/codecov-action@v2
+      - name: "Codecov: Tinygo"
+        uses: codecov/codecov-action@v3
         if: ${{ matrix.go-version == '1.19.x' }}
         with:
           files: build/coverage-tinygo.txt
diff --git a/codecov.yml b/codecov.yml
index b1e21d4..d8a0963 100644
--- a/codecov.yml
+++ b/codecov.yml
@@ -1,2 +1,6 @@
 ignore:
   - "**/generator/main.go" # generator tools usually involves writing files
+fixes:
+  # Apparently using build/ as prefix is not enough to ignore the directory
+  # Reference: https://community.codecov.com/t/codecov-website-can-not-show-line-by-line-coverage/3809
+  - "build/::" 
\ No newline at end of file
diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go
index 19edd4d..48ef31a 100644
--- a/internal/corazawaf/transaction.go
+++ b/internal/corazawaf/transaction.go
@@ -609,35 +609,38 @@ func (tx *Transaction) ProcessConnection(client string, cPort int, server string
 	tx.variables.serverPort.Set(p2)
 }
 
-// ExtractArguments transforms an url encoded string to a map and creates
-// ARGS_POST|GET
-func (tx *Transaction) ExtractArguments(orig types.ArgumentType, uri string) {
+// ExtractGetArguments transforms an url encoded string to a map and creates ARGS_GET
+func (tx *Transaction) ExtractGetArguments(uri string) {
 	data := urlutil.ParseQuery(uri, '&')
 	for k, vs := range data {
 		for _, v := range vs {
-			tx.AddArgument(orig, k, v)
+			tx.AddGetRequestArgument(k, v)
 		}
 	}
 }
 
-// AddArgument Add arguments GET or POST
-// This will set ARGS_(GET|POST), ARGS, ARGS_NAMES, ARGS_COMBINED_SIZE and
-// ARGS_(GET|POST)_NAMES
-func (tx *Transaction) AddArgument(argType types.ArgumentType, key string, value string) {
+// AddGetRequestArgument
+func (tx *Transaction) AddGetRequestArgument(key string, value string) {
 	// TODO implement ARGS value limit using ArgumentsLimit
-	var vals collection.Map
-	switch argType {
-	case types.ArgumentGET:
-		vals = tx.variables.argsGet
-	case types.ArgumentPOST:
-		vals = tx.variables.argsPost
-	case types.ArgumentPATH:
-		vals = tx.variables.argsPath
-	default:
-		return
-	}
+	tx.variables.argsGet.Add(key, value)
+}
 
-	vals.Add(key, value)
+// AddPostRequestArgument
+func (tx *Transaction) AddPostRequestArgument(key string, value string) {
+	// TODO implement ARGS value limit using ArgumentsLimit
+	tx.variables.argsPost.Add(key, value)
+}
+
+// AddPathRequestArgument
+func (tx *Transaction) AddPathRequestArgument(key string, value string) {
+	// TODO implement ARGS value limit using ArgumentsLimit
+	tx.variables.argsPath.Add(key, value)
+}
+
+// AddResponseArgument
+func (tx *Transaction) AddResponseArgument(key string, value string) {
+	// TODO implement ARGS value limit using ArgumentsLimit
+	// tx.variables.argsResponse.Add(key, value)
 }
 
 // ProcessURI Performs the analysis on the URI and all the query string variables.
@@ -684,7 +687,7 @@ func (tx *Transaction) ProcessURI(uri string, method string, httpVersion string)
 			tx.Variables.RequestUri.Set(uri)
 		*/
 	} else {
-		tx.ExtractArguments(types.ArgumentGET, parsedURL.RawQuery)
+		tx.ExtractGetArguments(parsedURL.RawQuery)
 		tx.variables.requestURI.Set(parsedURL.String())
 		path = parsedURL.Path
 		query = parsedURL.RawQuery
diff --git a/types/transaction.go b/types/transaction.go
index 5b8510e..11c634a 100644
--- a/types/transaction.go
+++ b/types/transaction.go
@@ -7,21 +7,6 @@ import (
 	"io"
 )
 
-// ArgumentType is used to define corazatypes of argument for transactions
-// There are three supported corazatypes: POST, GET and PATH
-type ArgumentType int
-
-const (
-	// ArgumentInvalid is used to define invalid argument corazatypes
-	ArgumentInvalid ArgumentType = iota
-	// ArgumentGET is used to define GET arguments
-	ArgumentGET
-	// ArgumentPOST is used to define POST arguments
-	ArgumentPOST
-	// ArgumentPATH is used to define PATH arguments
-	ArgumentPATH
-)
-
 // Transaction is created from a WAF instance to handle web requests and responses,
 // it contains a copy of most WAF configurations that can be safely changed.
 // Transactions are used to store all data like URLs, request and response
@@ -74,12 +59,20 @@ type Transaction interface {
 	// within the Transaction while also passing it further in an HTTP framework.
 	RequestBodyReader() (io.Reader, error)
 
-	// AddArgument Add arguments GET or POST
-	// This will set ARGS_(GET|POST), ARGS, ARGS_NAMES, ARGS_COMBINED_SIZE and
-	// ARGS_(GET|POST)_NAMES
-	// With this method it is possible to feed Coraza with a GET or POST argument
-	// providing granular control over the arguments.
-	AddArgument(orig ArgumentType, key string, value string)
+	// AddGetRequestArgument Add arguments GET, this will feed ARGS_GET, ARGS_GET_NAMES,
+	// ARGS, ARGS_NAMES, and ARGS_COMBINED_SIZE variables.
+	AddGetRequestArgument(key string, value string)
+
+	// AddPostRequestArgument Add arguments POST, this will feed ARGS_POST, ARGS_POST_NAMES,
+	// ARGS, ARGS_NAMES, and ARGS_COMBINED_SIZE variables.
+	AddPostRequestArgument(key string, value string)
+
+	// AddPathRequestArgument Add arguments PATH, this will feed ARGS_PATH, ARGS_PATH_NAMES,
+	// ARGS, ARGS_NAMES, and ARGS_COMBINED_SIZE variables.
+	AddPathRequestArgument(key string, value string)
+
+	// AddResponseArgument Add arguments to the response, this will feed ARGS_RESPONSE
+	AddResponseArgument(key string, value string)
 
 	// ProcessRequestBody Performs the analysis of the request body (if any)
 	//


Test patch:
diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go
index c1d9de2..de5339f 100644
--- a/internal/corazawaf/transaction_test.go
+++ b/internal/corazawaf/transaction_test.go
@@ -983,15 +983,15 @@ func TestTxAddArgument(t *testing.T) {
 	waf := NewWAF()
 	tx := waf.NewTransaction()
 	tx.ProcessConnection("127.0.0.1", 80, "127.0.0.2", 8080)
-	tx.AddArgument(types.ArgumentGET, "test", "testvalue")
+	tx.AddGetRequestArgument("test", "testvalue")
 	if tx.variables.argsGet.Get("test")[0] != "testvalue" {
 		t.Error("failed to set args get")
 	}
-	tx.AddArgument(types.ArgumentPOST, "ptest", "ptestvalue")
+	tx.AddPostRequestArgument("ptest", "ptestvalue")
 	if tx.variables.argsPost.Get("ptest")[0] != "ptestvalue" {
 		t.Error("failed to set args post")
 	}
-	tx.AddArgument(types.ArgumentPATH, "ptest2", "ptestvalue")
+	tx.AddPathRequestArgument("ptest2", "ptestvalue")
 	if tx.variables.argsPath.Get("ptest2")[0] != "ptestvalue" {
 		t.Error("failed to set args post")
 	}
@@ -1279,3 +1279,10 @@ func TestIterationStops(t *testing.T) {
 		})
 	}
 }
+
+func TestTxAddResponseArgs(t *testing.T) {
+	waf := NewWAF()
+	tx := waf.NewTransaction()
+	tx.AddResponseArgument("samplekey", "samplevalue")
+	t.Log("This is a placeholder for tx.AddResponseArgs")
+}
diff --git a/internal/seclang/rules_test.go b/internal/seclang/rules_test.go
index 4919f39..cd9af61 100644
--- a/internal/seclang/rules_test.go
+++ b/internal/seclang/rules_test.go
@@ -51,7 +51,7 @@ func TestRuleMatchWithRegex(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "id_test", "123")
+	tx.AddGetRequestArgument("id_test", "123")
 	tx.ProcessRequestHeaders()
 	if len(tx.MatchedRules()) != 1 {
 		t.Errorf("failed to match rules with %d", len(tx.MatchedRules()))
@@ -143,8 +143,8 @@ func TestRuleLogging(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test1", "123")
-	tx.AddArgument(types.ArgumentGET, "test2", "456")
+	tx.AddGetRequestArgument("test1", "123")
+	tx.AddGetRequestArgument("test2", "456")
 	tx.ProcessRequestHeaders()
 	if len(tx.MatchedRules()) != 3 {
 		t.Errorf("failed to match rules with %d", len(tx.MatchedRules()))
@@ -180,8 +180,8 @@ func TestRuleChains(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test1", "123")
-	tx.AddArgument(types.ArgumentGET, "test2", "456")
+	tx.AddGetRequestArgument("test1", "123")
+	tx.AddGetRequestArgument("test2", "456")
 	tx.ProcessRequestHeaders()
 	if len(tx.MatchedRules()) != 1 {
 		t.Errorf("failed to match rules with %d matches, expected 1", len(tx.MatchedRules()))
@@ -202,8 +202,8 @@ func TestTagsAreNotPrintedTwice(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test1", "123")
-	tx.AddArgument(types.ArgumentGET, "test2", "456")
+	tx.AddGetRequestArgument("test1", "123")
+	tx.AddGetRequestArgument("test2", "456")
 	tx.ProcessRequestHeaders()
 	if len(tx.MatchedRules()) != 1 {
 		t.Errorf("failed to match rules with %d", len(tx.MatchedRules()))
@@ -234,8 +234,8 @@ func TestStatusFromInterruptions(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test1", "123")
-	tx.AddArgument(types.ArgumentGET, "test2", "456")
+	tx.AddGetRequestArgument("test1", "123")
+	tx.AddGetRequestArgument("test2", "456")
 	it := tx.ProcessRequestHeaders()
 	if it == nil {
 		t.Error("failed to interrupt")
@@ -272,9 +272,9 @@ func TestLogsAreNotPrintedManyTimes(t *testing.T) {
 		t.Error(err.Error())
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test1", "123")
-	tx.AddArgument(types.ArgumentGET, "test2", "456")
-	tx.AddArgument(types.ArgumentGET, "test2", "789")
+	tx.AddGetRequestArgument("test1", "123")
+	tx.AddGetRequestArgument("test2", "456")
+	tx.AddGetRequestArgument("test2", "789")
 	tx.AddRequestHeader("test", "123")
 	tx.ProcessRequestHeaders()
 	if len(tx.MatchedRules()) != 1 {
@@ -370,7 +370,7 @@ func TestIssue176(t *testing.T) {
 	//		t.Error(err.Error())
 	//	}
 	//	tx = waf.NewTransaction()
-	//	tx.AddArgument(types.ArgumentGET, "Test1", "123")
+	//	tx.AddGetRequestArgument("Test1", "123")
 	//	it = tx.ProcessRequestHeaders()
 	//	if it == nil {
 	//		t.Error("failed to test argument case-sensitive")
@@ -383,7 +383,7 @@ func TestIssue176(t *testing.T) {
 	//		t.Error(err.Error())
 	//	}
 	//	tx = waf.NewTransaction()
-	//	tx.AddArgument(types.ArgumentGET, "Test2", "123")
+	//	tx.AddGetRequestArgument("Test2", "123")
 	//	it = tx.ProcessRequestHeaders()
 	//	if it != nil {
 	//		t.Error("failed to test argument case-sensitive")
@@ -451,7 +451,7 @@ func TestUnicode(t *testing.T) {
 	}
 
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "var", `ハローワールド`)
+	tx.AddPostRequestArgument("var", `ハローワールド`)
 	it, err := tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -503,7 +503,7 @@ func Test941310(t *testing.T) {
 	// }
 	//
 	// tx := waf.NewTransaction()
-	// tx.AddArgument(types.ArgumentPOST, "var", `\\xbcscript\\xbealert(\xa2XSS\xa2)\xbc/script\xbe`)
+	// tx.AddPostRequestArgument("var", `\\xbcscript\\xbealert(\xa2XSS\xa2)\xbc/script\xbe`)
 	// it, err := tx.ProcessRequestBody()
 	// if err != nil {
 	// 	t.Error(err)
@@ -525,7 +525,7 @@ func TestArgumentNamesCaseSensitive(t *testing.T) {
 	}
 	/*
 		tx := waf.NewTransaction()
-		tx.AddArgument(types.ArgumentPOST, "Test1", "Xyz")
+		tx.AddPostRequestArgument("Test1", "Xyz")
 		it, err := tx.ProcessRequestBody()
 		if err != nil {
 			t.Error(err)
@@ -535,7 +535,7 @@ func TestArgumentNamesCaseSensitive(t *testing.T) {
 		}
 
 		tx = waf.NewTransaction()
-		tx.AddArgument(types.ArgumentPOST, "TEST1", "Xyz")
+		tx.AddPostRequestArgument("TEST1", "Xyz")
 		it, err = tx.ProcessRequestBody()
 		if err != nil {
 			t.Error(err)
@@ -545,7 +545,7 @@ func TestArgumentNamesCaseSensitive(t *testing.T) {
 		}
 
 		tx = waf.NewTransaction()
-		tx.AddArgument(types.ArgumentPOST, "test1", "Xyz")
+		tx.AddPostRequestArgument("test1", "Xyz")
 		it, err = tx.ProcessRequestBody()
 		if err != nil {
 			t.Error(err)
@@ -568,7 +568,7 @@ func TestArgumentsCaseSensitive(t *testing.T) {
 	}
 
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "Test1", "Xyz")
+	tx.AddPostRequestArgument("Test1", "Xyz")
 	it, err := tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -578,7 +578,7 @@ func TestArgumentsCaseSensitive(t *testing.T) {
 	}
 
 	tx = waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "TEST1", "Xyz")
+	tx.AddPostRequestArgument("TEST1", "Xyz")
 	it, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -588,7 +588,7 @@ func TestArgumentsCaseSensitive(t *testing.T) {
 	}
 
 	tx = waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "test1", "Xyz")
+	tx.AddPostRequestArgument("test1", "Xyz")
 	it, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -598,7 +598,7 @@ func TestArgumentsCaseSensitive(t *testing.T) {
 	}
 
 	tx = waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "test1", "xyz")
+	tx.AddPostRequestArgument("test1", "xyz")
 	it, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -608,7 +608,7 @@ func TestArgumentsCaseSensitive(t *testing.T) {
 	}
 
 	tx = waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "test1", "XYZ")
+	tx.AddPostRequestArgument("test1", "XYZ")
 	it, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -754,9 +754,9 @@ func TestParameterPollution(t *testing.T) {
 	}
 
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "test1", "xyz")
-	tx.AddArgument(types.ArgumentPOST, "Test1", "Xyz")
-	tx.AddArgument(types.ArgumentPOST, "TEST1", "XYZ")
+	tx.AddPostRequestArgument("test1", "xyz")
+	tx.AddPostRequestArgument("Test1", "Xyz")
+	tx.AddPostRequestArgument("TEST1", "XYZ")
 	_, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
@@ -773,10 +773,10 @@ func TestParameterPollution(t *testing.T) {
 	}
 
 	tx = waf.NewTransaction()
-	tx.AddArgument(types.ArgumentPOST, "test1", "xyz")
-	tx.AddArgument(types.ArgumentPOST, "Test1", "Xyz")
-	tx.AddArgument(types.ArgumentPOST, "tesT1", "Xyz")
-	tx.AddArgument(types.ArgumentPOST, "TEST1", "XYZ")
+	tx.AddPostRequestArgument("test1", "xyz")
+	tx.AddPostRequestArgument("Test1", "Xyz")
+	tx.AddPostRequestArgument("tesT1", "Xyz")
+	tx.AddPostRequestArgument("TEST1", "XYZ")
 	_, err = tx.ProcessRequestBody()
 	if err != nil {
 		t.Error(err)
diff --git a/testing/auditlog_test.go b/testing/auditlog_test.go
index 084b7ec..2090493 100644
--- a/testing/auditlog_test.go
+++ b/testing/auditlog_test.go
@@ -18,7 +18,6 @@ import (
 	"github.com/corazawaf/coraza/v3/internal/corazawaf"
 	"github.com/corazawaf/coraza/v3/internal/seclang"
 	"github.com/corazawaf/coraza/v3/loggers"
-	"github.com/corazawaf/coraza/v3/types"
 )
 
 func TestAuditLogMessages(t *testing.T) {
@@ -43,7 +42,7 @@ func TestAuditLogMessages(t *testing.T) {
 	}
 	defer os.Remove(file.Name())
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test", "test")
+	tx.AddGetRequestArgument("test", "test")
 	tx.ProcessRequestHeaders()
 	al := tx.AuditLog()
 	if len(al.Messages) != 1 {
@@ -92,7 +91,7 @@ func TestAuditLogRelevantOnly(t *testing.T) {
 		t.Error(err)
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test", "test")
+	tx.AddGetRequestArgument("test", "test")
 	tx.ProcessRequestHeaders()
 	// now we read file
 	if _, err := file.Seek(0, 0); err != nil {
@@ -129,7 +128,7 @@ func TestAuditLogRelevantOnlyOk(t *testing.T) {
 		t.Error(err)
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test", "test")
+	tx.AddGetRequestArgument("test", "test")
 	tx.ProcessRequestHeaders()
 	// now we read file
 	if _, err := file.Seek(0, 0); err != nil {
@@ -166,7 +165,7 @@ func TestAuditLogRelevantOnlyNoAuditlog(t *testing.T) {
 		t.Error(err)
 	}
 	tx := waf.NewTransaction()
-	tx.AddArgument(types.ArgumentGET, "test", "test")
+	tx.AddGetRequestArgument("test", "test")
 	tx.ProcessRequestHeaders()
 	// now we read file
 	if _, err := file.Seek(0, 0); err != nil {