From e40e60570f83b34d510795669a2e1a4d88f5631d Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Wed, 30 Oct 2024 15:55:24 +0100 Subject: [PATCH 1/7] wip: UseRequestBody --- internal/corazawaf/body_buffer.go | 18 +++++++ internal/corazawaf/transaction.go | 50 ++++++++++++++++++++ internal/corazawaf/transaction_test.go | 65 ++++++++++++++++++++------ types/transaction.go | 10 ++++ 4 files changed, 129 insertions(+), 14 deletions(-) diff --git a/internal/corazawaf/body_buffer.go b/internal/corazawaf/body_buffer.go index ac9010e54..4b8cffe16 100644 --- a/internal/corazawaf/body_buffer.go +++ b/internal/corazawaf/body_buffer.go @@ -94,6 +94,24 @@ func (br *BodyBuffer) Write(data []byte) (n int, err error) { return br.buffer.Write(data) } +// SetBuffer sets the buffer to the provided slice of bytes. +func (br *BodyBuffer) SetBuffer(data []byte) error { + if len(data) == 0 { + return errors.New("provided data is empty") + } + + // Check if the provided data exceeds the memory limit + if int64(len(data)) > br.options.MemoryLimit { + return errors.New("memoryLimit reached while writing") + } + + // Set the buffer to the provided slice + br.buffer = bytes.NewBuffer(data) + br.length = int64(len(data)) + + return nil +} + type bodyBufferReader struct { pos int br *BodyBuffer diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 2fe3585c8..36412e67a 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -968,6 +968,56 @@ func (tx *Transaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, in return tx.interruption, int(w), err } +// UseRequestBody directly sets the provided byte slice as the request body buffer. +// This is meant to be used when the entire request body is available, as it avoids +// the need for an extra copy into the request body buffer. Because of this, this method is expected to +// be called just once. Further calls to UseRequestBody will overwrite the previous body set. +// If the body size exceeds the limit and the action is to reject, an interruption will be returned. +// The caller should not use b slice after this call. +// +// It returns the relevant interruption, the final internal body buffer length and any error that occurs. +func (tx *Transaction) UseRequestBody(b []byte) (*types.Interruption, int, error) { + if tx.RuleEngine == types.RuleEngineOff { + return nil, 0, nil + } + + if !tx.RequestBodyAccess { + return nil, 0, nil + } + + bodySize := int64(len(b)) + var runProcessRequestBody bool + + if bodySize > tx.RequestBodyLimit { + tx.variables.inboundDataError.Set("1") + if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionReject { + // We interrupt this transaction in case RequestBodyLimitAction is Reject + return setAndReturnBodyLimitInterruption(tx) + } + + if tx.WAF.RequestBodyLimitAction == types.BodyLimitActionProcessPartial { + // Truncate the body slice to the configured limit + b = b[:tx.RequestBodyLimit] + bodySize = tx.RequestBodyLimit + runProcessRequestBody = true + } + } + + // Point the internal buffer to the provided slice + err := tx.requestBodyBuffer.SetBuffer(b) + if err != nil { + return nil, 0, err + } + + err = nil + if runProcessRequestBody { + tx.debugLogger.Warn().Msg("Processing request body whose size reached the configured limit (Action ProcessPartial)") + _, err = tx.ProcessRequestBody() + } + + return tx.interruption, int(bodySize), err +} + // ProcessRequestBody Performs the analysis of the request body (if any) // // It is recommended to call this method even if it is not expected to have a body. diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go index 64d548546..6bccdfded 100644 --- a/internal/corazawaf/transaction_test.go +++ b/internal/corazawaf/transaction_test.go @@ -146,16 +146,19 @@ func TestTxResponse(t *testing.T) { } var requestBodyWriters = map[string]func(tx *Transaction, body string) (*types.Interruption, int, error){ - "WriteRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { - return tx.WriteRequestBody([]byte(body)) - }, - "ReadRequestBodyFromKnownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { - return tx.ReadRequestBodyFrom(strings.NewReader(body)) - }, - "ReadRequestBodyFromUnknownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { - return tx.ReadRequestBodyFrom(struct{ io.Reader }{ - strings.NewReader(body), - }) + // "WriteRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { + // return tx.WriteRequestBody([]byte(body)) + // }, + // "ReadRequestBodyFromKnownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { + // return tx.ReadRequestBodyFrom(strings.NewReader(body)) + // }, + // "ReadRequestBodyFromUnknownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { + // return tx.ReadRequestBodyFrom(struct{ io.Reader }{ + // strings.NewReader(body), + // }) + // }, + "UseRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { + return tx.UseRequestBody([]byte(body)) }, } @@ -211,10 +214,14 @@ func TestWriteRequestBody(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - for name, writeRequestBody := range requestBodyWriters { - t.Run(name, func(t *testing.T) { + for writerName, writeRequestBody := range requestBodyWriters { + t.Run(writerName, func(t *testing.T) { for name, chunks := range bodyChunks { t.Run(name, func(t *testing.T) { + if name != "BodyInOneShot" && writerName == "UseRequestBody" { + // UseRequestBody is intended to be used when the whole body is available + return + } waf := NewWAF() waf.RuleEngine = types.RuleEngineOn waf.RequestBodyAccess = true @@ -298,6 +305,10 @@ func TestWriteRequestBodyOnLimitReached(t *testing.T) { t.Run(tName, func(t *testing.T) { for wName, writer := range requestBodyWriters { + if wName == "UseRequestBody" { + // Skip UseRequestBody test case. It is intended to be used when the whole body is available + continue + } t.Run(wName, func(t *testing.T) { tx := waf.NewTransaction() _, err := tx.requestBodyBuffer.Write([]byte("ab")) @@ -312,11 +323,11 @@ func TestWriteRequestBodyOnLimitReached(t *testing.T) { } if it != tCase.preexistingInterruption { - t.Fatalf("unexpected interruption") + t.Fatalf("unexpected interruption: %v", it) } if n != 0 { - t.Fatalf("unexpected number of bytes written") + t.Fatalf("unexpected number of bytes written. Expected 0, got %d", n) } if err := tx.Close(); err != nil { @@ -377,6 +388,32 @@ func TestWriteRequestBodyIsNopWhenBodyIsNotAccesible(t *testing.T) { } } +// TODO: test TestUseRequestBodyMultipleCalls return an error +// TODO: implement UseResponseBody +// func TestUseRequestBodyMultipleCalls(t *testing.T) { +// tx := makeTransaction(t) +// tx.RequestBodyAccess = true +// tx.RequestBodyLimit = 20 +// body := bytes.Repeat([]byte("a"), 100) +// it, n, err := tx.UseRequestBody(body) +// if err != nil { +// t.Fatalf("unexpected error: %s", err.Error()) +// } +// if it != nil { +// t.Fatalf("unexpected interruption") +// } +// if n != int(tx.RequestBodyLimit) { +// t.Fatalf("unexpected number of bytes written") +// } +// // UseRequestBody should not generate a copy of the data, +// // body and tx.Buffer are expected to point to the same data +// bodyPtr := unsafe.SliceData(body) +// txBufferPtr := &tx.requestBodyBuffer.buffer.Bytes()[0] +// if bodyPtr != txBufferPtr { +// t.Fatalf("body and tx.Buffer are not pointing to the same data") +// } +// } + func TestResponseHeader(t *testing.T) { tx := makeTransaction(t) tx.AddResponseHeader("content-type", "test") diff --git a/types/transaction.go b/types/transaction.go index b16448a20..cf615f37f 100644 --- a/types/transaction.go +++ b/types/transaction.go @@ -104,6 +104,16 @@ type Transaction interface { // It returns the corresponding interruption, the number of bytes written an error if any. ReadRequestBodyFrom(io.Reader) (*Interruption, int, error) + // UseRequestBody directly sets the provided byte slice as the request body buffer. + // This is meant to be used when the entire request body is available, as it avoids + // the need for an extra copy into the request body buffer. Because of this, this method is expected to + // be called just once. Further calls to UseRequestBody will overwrite the previous body set. + // If the body size exceeds the limit and the action is to reject, an interruption will be returned. + // The caller should not use b slice after this call. + // + // It returns the relevant interruption, the final internal body buffer length and any error that occurs. + UseRequestBody(b []byte) (*Interruption, int, error) + // AddResponseHeader Adds a response header variable // // With this method it is possible to feed Coraza with a response header. From 38d5e700ac7f4e233b9ee9eeb7df03c05ad3c273 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Fri, 8 Nov 2024 12:23:14 +0000 Subject: [PATCH 2/7] response body, tests --- experimental/transaction.go | 23 ++++ internal/corazawaf/transaction.go | 66 +++++++++- internal/corazawaf/transaction_test.go | 167 ++++++++++++++----------- types/transaction.go | 14 +-- 4 files changed, 188 insertions(+), 82 deletions(-) create mode 100644 experimental/transaction.go diff --git a/experimental/transaction.go b/experimental/transaction.go new file mode 100644 index 000000000..d5a941bc3 --- /dev/null +++ b/experimental/transaction.go @@ -0,0 +1,23 @@ +// Copyright 2024 Juan Pablo Tosso and the OWASP Coraza contributors +// SPDX-License-Identifier: Apache-2.0 + +package experimental + +import ( + "github.com/corazawaf/coraza/v3/types" +) + +type Transaction interface { + types.Transaction + // UseRequestBody directly sets the provided byte slice as the request body buffer. + // This is meant to be used when the entire request body is available, as it avoids + // the need for an extra copy into the request body buffer. Because of this, this method + // is expected to be called just once, further calls to UseRequestBody have to be avoided. + // If the body size exceeds the limit and the action is to reject, an interruption will be returned. + // The caller should not use b slice after this call. + // + // It returns the relevant interruption, the final internal body buffer length and any error that occurs. + UseRequestBody(b []byte) (*types.Interruption, int, error) + + UseResponseBody(b []byte) (*types.Interruption, int, error) +} diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 36412e67a..970a01e67 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -970,8 +970,8 @@ func (tx *Transaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, in // UseRequestBody directly sets the provided byte slice as the request body buffer. // This is meant to be used when the entire request body is available, as it avoids -// the need for an extra copy into the request body buffer. Because of this, this method is expected to -// be called just once. Further calls to UseRequestBody will overwrite the previous body set. +// the need for an extra copy into the request body buffer. Because of this, this method +// is expected to be called just once, further calls to UseRequestBody have to be avoided. // If the body size exceeds the limit and the action is to reject, an interruption will be returned. // The caller should not use b slice after this call. // @@ -985,6 +985,10 @@ func (tx *Transaction) UseRequestBody(b []byte) (*types.Interruption, int, error return nil, 0, nil } + if tx.lastPhase >= types.PhaseRequestBody { + return nil, 0, fmt.Errorf("request body buffer set more than once, which has been already been processed") + } + bodySize := int64(len(b)) var runProcessRequestBody bool @@ -1268,6 +1272,64 @@ func (tx *Transaction) ReadResponseBodyFrom(r io.Reader) (*types.Interruption, i return tx.interruption, int(w), err } +// If the body size exceeds the limit and the action is to reject, an interruption will be returned. +// The caller should not use b slice after this call. +// +// It returns the relevant interruption, the final internal body buffer length and any error that occurs. + +// UseResponseBody directly sets the provided byte slice as the response body buffer. +// This is meant to be used when the entire response body is available, as it avoids +// the need for an extra copy into the response body buffer. Because of this, this method is expected to +// be called just once, further calls to UseResponseBody have to be avoided. +// If the body size exceeds the limit and the action is to reject, an interruption will be returned. +// The caller should not use b slice after this call. +// +// It returns the relevant interruption, the final internal body buffer length and any error that occurs. +func (tx *Transaction) UseResponseBody(b []byte) (*types.Interruption, int, error) { + if tx.RuleEngine == types.RuleEngineOff { + return nil, 0, nil + } + + if !tx.ResponseBodyAccess { + return nil, 0, nil + } + + if tx.lastPhase >= types.PhaseResponseBody { + return nil, 0, fmt.Errorf("response body buffer set more than once, which has been already been processed") + } + + var ( + bodySize = int64(len(b)) + runProcessResponseBody = false + ) + if bodySize >= tx.ResponseBodyLimit { + tx.variables.outboundDataError.Set("1") + if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionReject { + // We interrupt this transaction in case ResponseBodyLimitAction is Reject + return setAndReturnBodyLimitInterruption(tx) + } + + if tx.WAF.ResponseBodyLimitAction == types.BodyLimitActionProcessPartial { + // Truncate the body slice to the configured limit + b = b[:tx.ResponseBodyLimit] + bodySize = tx.ResponseBodyLimit + runProcessResponseBody = true + } + } + // Point the internal buffer to the provided slice + err := tx.responseBodyBuffer.SetBuffer(b) + if err != nil { + return nil, 0, err + } + + err = nil + if runProcessResponseBody { + tx.debugLogger.Debug().Msg("Processing response body whose size reached the configured limit (Action ProcessPartial)") + _, err = tx.ProcessResponseBody() + } + return tx.interruption, int(bodySize), err +} + // ProcessResponseBody Perform the analysis of the the response body (if any) // // It is recommended to call this method even if it is not expected to have a body. diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go index 6bccdfded..dfe127efd 100644 --- a/internal/corazawaf/transaction_test.go +++ b/internal/corazawaf/transaction_test.go @@ -12,6 +12,7 @@ import ( "strconv" "strings" "testing" + "unsafe" "github.com/corazawaf/coraza/v3/collection" "github.com/corazawaf/coraza/v3/debuglog" @@ -114,49 +115,18 @@ func TestTxMultipart(t *testing.T) { } } -func TestTxResponse(t *testing.T) { - /* - tx := NewWAF().NewTransaction() - ht := []string{ - "HTTP/1.1 200 OK", - "Content-Type: text/html", - "Last-Modified: Mon, 14 Sep 2020 21:10:42 GMT", - "Accept-Ranges: bytes", - "ETag: \"0b5f480db8ad61:0\"", - "Vary: Accept-Encoding", - "Server: Microsoft-IIS/8.5", - "Content-Security-Policy: default-src: https:; frame-ancestors 'self' X-Frame-Options: SAMEORIGIN", - "Strict-Transport-Security: max-age=31536000; includeSubDomains; preload", - "Date: Wed, 16 Sep 2020 14:14:09 GMT", - "Connection: close", - "Content-Length: 10", - "", - "testcontent", - } - data := strings.Join(ht, "\r\n") - tx.ParseResponseString(nil, data) - - exp := map[string]string{ - "%{response_headers.content-length}": "10", - "%{response_headers.server}": "Microsoft-IIS/8.5", - } - - validateMacroExpansion(exp, tx, t) - */ -} - var requestBodyWriters = map[string]func(tx *Transaction, body string) (*types.Interruption, int, error){ - // "WriteRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { - // return tx.WriteRequestBody([]byte(body)) - // }, - // "ReadRequestBodyFromKnownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { - // return tx.ReadRequestBodyFrom(strings.NewReader(body)) - // }, - // "ReadRequestBodyFromUnknownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { - // return tx.ReadRequestBodyFrom(struct{ io.Reader }{ - // strings.NewReader(body), - // }) - // }, + "WriteRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { + return tx.WriteRequestBody([]byte(body)) + }, + "ReadRequestBodyFromKnownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { + return tx.ReadRequestBodyFrom(strings.NewReader(body)) + }, + "ReadRequestBodyFromUnknownLen": func(tx *Transaction, body string) (*types.Interruption, int, error) { + return tx.ReadRequestBodyFrom(struct{ io.Reader }{ + strings.NewReader(body), + }) + }, "UseRequestBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { return tx.UseRequestBody([]byte(body)) }, @@ -219,7 +189,7 @@ func TestWriteRequestBody(t *testing.T) { for name, chunks := range bodyChunks { t.Run(name, func(t *testing.T) { if name != "BodyInOneShot" && writerName == "UseRequestBody" { - // UseRequestBody is intended to be used when the whole body is available + // UseRequestBody is intended to be used only when the whole body is available return } waf := NewWAF() @@ -306,7 +276,8 @@ func TestWriteRequestBodyOnLimitReached(t *testing.T) { t.Run(tName, func(t *testing.T) { for wName, writer := range requestBodyWriters { if wName == "UseRequestBody" { - // Skip UseRequestBody test case. It is intended to be used when the whole body is available + // Skip UseRequestBody test case. It is intended to be used only when the whole body is available + // at once. This test gradually populates the body up to its limit. continue } t.Run(wName, func(t *testing.T) { @@ -388,31 +359,39 @@ func TestWriteRequestBodyIsNopWhenBodyIsNotAccesible(t *testing.T) { } } -// TODO: test TestUseRequestBodyMultipleCalls return an error -// TODO: implement UseResponseBody -// func TestUseRequestBodyMultipleCalls(t *testing.T) { -// tx := makeTransaction(t) -// tx.RequestBodyAccess = true -// tx.RequestBodyLimit = 20 -// body := bytes.Repeat([]byte("a"), 100) -// it, n, err := tx.UseRequestBody(body) -// if err != nil { -// t.Fatalf("unexpected error: %s", err.Error()) -// } -// if it != nil { -// t.Fatalf("unexpected interruption") -// } -// if n != int(tx.RequestBodyLimit) { -// t.Fatalf("unexpected number of bytes written") -// } -// // UseRequestBody should not generate a copy of the data, -// // body and tx.Buffer are expected to point to the same data -// bodyPtr := unsafe.SliceData(body) -// txBufferPtr := &tx.requestBodyBuffer.buffer.Bytes()[0] -// if bodyPtr != txBufferPtr { -// t.Fatalf("body and tx.Buffer are not pointing to the same data") -// } -// } +// UseRequestBody has not to be called more then once, the first call +// might already trigger the inspection and so a new buffer set might +// not be inspected anymore. If that happens, an error has to be returned. +func TestUseRequestBodyMultipleCalls(t *testing.T) { + tx := makeTransaction(t) + tx.lastPhase = 1 + tx.RequestBodyAccess = true + tx.RequestBodyLimit = 10 + tx.WAF.RequestBodyLimitAction = types.BodyLimitActionProcessPartial + body := bytes.Repeat([]byte("a"), 11) + it, n, err := tx.UseRequestBody(body) + if err != nil { + t.Fatalf("unexpected error: %s", err.Error()) + } + if it != nil { + t.Fatalf("unexpected interruption: %v", it) + } + if n != int(tx.RequestBodyLimit) { + t.Fatalf("unexpected number of bytes written. Expected %d, got %d", tx.RequestBodyLimit, n) + } + // UseRequestBody should not generate a copy of the data, + // body and tx.Buffer are expected to point to the same data + bodyPtr := unsafe.SliceData(body) + txBufferPtr := &tx.requestBodyBuffer.buffer.Bytes()[0] + if bodyPtr != txBufferPtr { + t.Fatalf("body and tx.Buffer are not pointing to the same data") + } + // Second call to UseRequestBody with phase 2 processed should trigger an error + _, _, err = tx.UseRequestBody(body) + if err == nil { + t.Fatalf("expected error calling UseRequestBody twice with phase 2 processed in between") + } +} func TestResponseHeader(t *testing.T) { tx := makeTransaction(t) @@ -516,6 +495,9 @@ var responseBodyWriters = map[string]func(tx *Transaction, body string) (*types. strings.NewReader(body), }) }, + "UseResponseBody": func(tx *Transaction, body string) (*types.Interruption, int, error) { + return tx.UseResponseBody([]byte(body)) + }, } func TestWriteResponseBody(t *testing.T) { @@ -567,10 +549,14 @@ func TestWriteResponseBody(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - for name, writeResponseBody := range responseBodyWriters { - t.Run(name, func(t *testing.T) { + for writerName, writeResponseBody := range responseBodyWriters { + t.Run(writerName, func(t *testing.T) { for name, chunks := range bodyChunks { t.Run(name, func(t *testing.T) { + if name != "BodyInOneShot" && writerName == "UseResponseBody" { + // UseResponseBody is intended to be used only when the whole body is available + return + } waf := NewWAF() waf.RuleEngine = types.RuleEngineOn waf.ResponseBodyMimeTypes = []string{"text/plain"} @@ -658,6 +644,11 @@ func TestWriteResponseBodyOnLimitReached(t *testing.T) { t.Run(tName, func(t *testing.T) { for wName, writer := range responseBodyWriters { + if wName == "UseResponseBody" { + // Skip UseResponseBody test case. It is intended to be used only when the whole body is available + // at once. This test gradually populates the body up to its limit. + continue + } t.Run(wName, func(t *testing.T) { tx := waf.NewTransaction() _, err := tx.responseBodyBuffer.Write([]byte("ab")) @@ -737,6 +728,40 @@ func TestWriteResponseBodyIsNopWhenBodyIsNotAccesible(t *testing.T) { } } +// UseResponseBody has not to be called more then once, the first call +// might already trigger the inspection and so a new buffer set might +// not be inspected anymore. If that happens, an error has to be returned. +func TestUseResponseBodyMultipleCalls(t *testing.T) { + tx := makeTransaction(t) + tx.lastPhase = 3 + tx.ResponseBodyAccess = true + tx.ResponseBodyLimit = 10 + tx.WAF.ResponseBodyLimitAction = types.BodyLimitActionProcessPartial + body := bytes.Repeat([]byte("a"), 11) + it, n, err := tx.UseResponseBody(body) + if err != nil { + t.Fatalf("unexpected error: %s", err.Error()) + } + if it != nil { + t.Fatalf("unexpected interruption: %v", it) + } + if n != int(tx.ResponseBodyLimit) { + t.Fatalf("unexpected number of bytes written. Expected %d, got %d", tx.ResponseBodyLimit, n) + } + // UseResponseBody should not generate a copy of the data, + // body and tx.Buffer are expected to point to the same data + bodyPtr := unsafe.SliceData(body) + txBufferPtr := &tx.responseBodyBuffer.buffer.Bytes()[0] + if bodyPtr != txBufferPtr { + t.Fatalf("body and tx.Buffer are not pointing to the same data") + } + // Second call to UseResponseBody with phase 4 processed should trigger an error + _, _, err = tx.UseResponseBody(body) + if err == nil { + t.Fatalf("expected error calling UseRequestBody twice with phase 2 processed in between") + } +} + func TestAuditLogFields(t *testing.T) { tx := makeTransaction(t) tx.AuditLogParts = types.AuditLogParts("ABCDEFGHIJK") diff --git a/types/transaction.go b/types/transaction.go index cf615f37f..c9a2cffb9 100644 --- a/types/transaction.go +++ b/types/transaction.go @@ -104,15 +104,8 @@ type Transaction interface { // It returns the corresponding interruption, the number of bytes written an error if any. ReadRequestBodyFrom(io.Reader) (*Interruption, int, error) - // UseRequestBody directly sets the provided byte slice as the request body buffer. - // This is meant to be used when the entire request body is available, as it avoids - // the need for an extra copy into the request body buffer. Because of this, this method is expected to - // be called just once. Further calls to UseRequestBody will overwrite the previous body set. - // If the body size exceeds the limit and the action is to reject, an interruption will be returned. - // The caller should not use b slice after this call. - // - // It returns the relevant interruption, the final internal body buffer length and any error that occurs. - UseRequestBody(b []byte) (*Interruption, int, error) + // Corrently exposed only as experimental + // UseRequestBody(b []byte) (*Interruption, int, error) // AddResponseHeader Adds a response header variable // @@ -157,6 +150,9 @@ type Transaction interface { // It returns the corresponding interruption, the number of bytes written an error if any. ReadResponseBodyFrom(io.Reader) (*Interruption, int, error) + // Corrently exposed only as experimental + // UseResponseBody(b []byte) (*Interruption, int, error) + // ProcessLogging Logging all information relative to this transaction. // At this point there is not need to hold the connection, the response can be // delivered prior to the execution of this method. From 464531184a44371fc10b19f3108de9545811ecaa Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Fri, 8 Nov 2024 12:41:19 +0000 Subject: [PATCH 3/7] go mods --- go.sum | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go.sum b/go.sum index bf0b4d1fb..7802797dc 100644 --- a/go.sum +++ b/go.sum @@ -100,7 +100,10 @@ golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= +<<<<<<< HEAD gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +======= +>>>>>>> 2659bf19 (go mods) rsc.io/binaryregexp v0.2.0 h1:HfqmD5MEmC0zvwBuF187nq9mdnXjXsSivRiXN7SmRkE= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= From 1a9daa1643eb25003d37ff2c30ce272920355814 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Tue, 12 Nov 2024 18:03:03 +0100 Subject: [PATCH 4/7] fix comments --- experimental/transaction.go | 9 +++++++++ internal/corazawaf/transaction.go | 5 ----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/experimental/transaction.go b/experimental/transaction.go index d5a941bc3..1e21b32b1 100644 --- a/experimental/transaction.go +++ b/experimental/transaction.go @@ -9,6 +9,7 @@ import ( type Transaction interface { types.Transaction + // UseRequestBody directly sets the provided byte slice as the request body buffer. // This is meant to be used when the entire request body is available, as it avoids // the need for an extra copy into the request body buffer. Because of this, this method @@ -19,5 +20,13 @@ type Transaction interface { // It returns the relevant interruption, the final internal body buffer length and any error that occurs. UseRequestBody(b []byte) (*types.Interruption, int, error) + // UseResponseBody directly sets the provided byte slice as the response body buffer. + // This is meant to be used when the entire response body is available, as it avoids + // the need for an extra copy into the response body buffer. Because of this, this method is expected to + // be called just once, further calls to UseResponseBody have to be avoided. + // If the body size exceeds the limit and the action is to reject, an interruption will be returned. + // The caller should not use b slice after this call. + // + // It returns the relevant interruption, the final internal body buffer length and any error that occurs. UseResponseBody(b []byte) (*types.Interruption, int, error) } diff --git a/internal/corazawaf/transaction.go b/internal/corazawaf/transaction.go index 970a01e67..3f1883f17 100644 --- a/internal/corazawaf/transaction.go +++ b/internal/corazawaf/transaction.go @@ -1272,11 +1272,6 @@ func (tx *Transaction) ReadResponseBodyFrom(r io.Reader) (*types.Interruption, i return tx.interruption, int(w), err } -// If the body size exceeds the limit and the action is to reject, an interruption will be returned. -// The caller should not use b slice after this call. -// -// It returns the relevant interruption, the final internal body buffer length and any error that occurs. - // UseResponseBody directly sets the provided byte slice as the response body buffer. // This is meant to be used when the entire response body is available, as it avoids // the need for an extra copy into the response body buffer. Because of this, this method is expected to From b816f3474b494c726d55762f30b64bd724d6054d Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Tue, 12 Nov 2024 19:37:04 +0100 Subject: [PATCH 5/7] benchmarks --- internal/corazawaf/transaction_test.go | 52 ++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/internal/corazawaf/transaction_test.go b/internal/corazawaf/transaction_test.go index dfe127efd..1bb1c01c8 100644 --- a/internal/corazawaf/transaction_test.go +++ b/internal/corazawaf/transaction_test.go @@ -393,6 +393,32 @@ func TestUseRequestBodyMultipleCalls(t *testing.T) { } } +func BenchmarkUseRequestBody(b *testing.B) { + body := bytes.Repeat([]byte("A"), 10*1024*1024) + + b.Run("WriteRequestBody", func(b *testing.B) { + for i := 0; i < b.N; i++ { + tx := makeTransaction(b) + tx.lastPhase = 1 + tx.RequestBodyAccess = true + tx.RequestBodyLimit = 11 * 1024 * 1024 + _, _, _ = tx.WriteRequestBody(body) + } + b.ReportAllocs() + }) + + b.Run("UseRequestBody", func(b *testing.B) { + for i := 0; i < b.N; i++ { + tx := makeTransaction(b) + tx.lastPhase = 1 + tx.RequestBodyAccess = true + tx.RequestBodyLimit = 11 * 1024 * 1024 + _, _, _ = tx.UseRequestBody(body) + } + b.ReportAllocs() + }) +} + func TestResponseHeader(t *testing.T) { tx := makeTransaction(t) tx.AddResponseHeader("content-type", "test") @@ -762,6 +788,32 @@ func TestUseResponseBodyMultipleCalls(t *testing.T) { } } +func BenchmarkUseResponseBody(b *testing.B) { + body := bytes.Repeat([]byte("A"), 10*1024*1024) + + b.Run("WriteResponseBody", func(b *testing.B) { + for i := 0; i < b.N; i++ { + tx := makeTransaction(b) + tx.lastPhase = 3 + tx.ResponseBodyAccess = true + tx.ResponseBodyLimit = 11 * 1024 * 1024 + _, _, _ = tx.WriteRequestBody(body) + } + b.ReportAllocs() + }) + + b.Run("UseResponseBody", func(b *testing.B) { + for i := 0; i < b.N; i++ { + tx := makeTransaction(b) + tx.lastPhase = 3 + tx.ResponseBodyAccess = true + tx.ResponseBodyLimit = 11 * 1024 * 1024 + _, _, _ = tx.UseResponseBody(body) + } + b.ReportAllocs() + }) +} + func TestAuditLogFields(t *testing.T) { tx := makeTransaction(t) tx.AuditLogParts = types.AuditLogParts("ABCDEFGHIJK") From 30d4a924549b176603574b381306497bb5b034d3 Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Wed, 13 Nov 2024 00:19:19 +0100 Subject: [PATCH 6/7] fix post rebase --- go.sum | 3 --- 1 file changed, 3 deletions(-) diff --git a/go.sum b/go.sum index 7802797dc..bf0b4d1fb 100644 --- a/go.sum +++ b/go.sum @@ -100,10 +100,7 @@ golang.org/x/tools v0.22.0/go.mod h1:aCwcsjqvq7Yqt6TNyX7QMU2enbQ/Gt0bo6krSeEri+c golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg= google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= -<<<<<<< HEAD gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -======= ->>>>>>> 2659bf19 (go mods) rsc.io/binaryregexp v0.2.0 h1:HfqmD5MEmC0zvwBuF187nq9mdnXjXsSivRiXN7SmRkE= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= From b0f432368cf3a54162f15a5da4bea764983107ef Mon Sep 17 00:00:00 2001 From: Matteo Pace Date: Wed, 13 Nov 2024 11:23:43 +0100 Subject: [PATCH 7/7] better docs --- experimental/transaction.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/experimental/transaction.go b/experimental/transaction.go index 1e21b32b1..ffe72d827 100644 --- a/experimental/transaction.go +++ b/experimental/transaction.go @@ -15,7 +15,9 @@ type Transaction interface { // the need for an extra copy into the request body buffer. Because of this, this method // is expected to be called just once, further calls to UseRequestBody have to be avoided. // If the body size exceeds the limit and the action is to reject, an interruption will be returned. - // The caller should not use b slice after this call. + // + // Note: The new internal buffer takes ownership of the provided data, the caller should NOT use b slice + // after this call. // // It returns the relevant interruption, the final internal body buffer length and any error that occurs. UseRequestBody(b []byte) (*types.Interruption, int, error) @@ -25,7 +27,9 @@ type Transaction interface { // the need for an extra copy into the response body buffer. Because of this, this method is expected to // be called just once, further calls to UseResponseBody have to be avoided. // If the body size exceeds the limit and the action is to reject, an interruption will be returned. - // The caller should not use b slice after this call. + // + // Note: The new internal buffer takes ownership of the provided data, the caller should NOT use b slice + // after this call. // // It returns the relevant interruption, the final internal body buffer length and any error that occurs. UseResponseBody(b []byte) (*types.Interruption, int, error)