Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refactor process body related logs and doc #1187

Merged
merged 4 commits into from
Nov 2, 2024
Merged

chore: refactor process body related logs and doc #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
350b696
refactor process body related logs and doc
M4tteoP Oct 31, 2024
72a9621
Merge branch 'main' into process_body
M4tteoP Oct 31, 2024
88277af
Merge branch 'main' into process_body
M4tteoP Nov 2, 2024
f7baad6
Merge branch 'main' into process_body
M4tteoP Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 2 additions & 19 deletions http/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,27 +86,10 @@ func processRequest(tx types.Transaction, req *http.Request) (*types.Interruptio

// Adds all remaining bytes beyond the coraza limit to its buffer
// It happens when the partial body has been processed and it did not trigger an interruption
body := io.MultiReader(rbr, req.Body)
bodyReader := io.MultiReader(rbr, req.Body)
// req.Body is transparently reinizialied with a new io.ReadCloser.
// The http handler will be able to read it.
// Prior to Go 1.19 NopCloser does not implement WriterTo if the reader implements it.
// - https://github.com/golang/go/issues/51566
// - https://tip.golang.org/doc/go1.19#minor_library_changes
// This avoid errors like "failed to process request: malformed chunked encoding" when
// using io.Copy.
// In Go 1.19 we just do `req.Body = io.NopCloser(reader)`
if rwt, ok := body.(io.WriterTo); ok {
req.Body = struct {
io.Reader
io.WriterTo
io.Closer
}{body, rwt, req.Body}
} else {
req.Body = struct {
io.Reader
io.Closer
}{body, req.Body}
}
req.Body = io.NopCloser(bodyReader)
}
}

Expand Down
40 changes: 24 additions & 16 deletions internal/corazawaf/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -969,9 +969,9 @@ func (tx *Transaction) ReadRequestBodyFrom(r io.Reader) (*types.Interruption, in

// ProcessRequestBody Performs the analysis of the request body (if any)
//
// This method perform the analysis on the request body. It is optional to
// call that function. If this API consumer already knows that there isn't a
// body for inspect it is recommended to skip this step.
// It is recommended to call this method even if it is not expected to have a body.
// It permits to execute rules belonging to request body phase, but not necesarily
// processing the request body.
//
// Remember to check for a possible intervention.
func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
Expand All @@ -985,11 +985,15 @@ func (tx *Transaction) ProcessRequestBody() (*types.Interruption, error) {
}

if tx.lastPhase != types.PhaseRequestHeaders {
if tx.lastPhase >= types.PhaseRequestBody {
// Phase already evaluated or skipped
tx.debugLogger.Warn().Msg("ProcessRequestBody should have already been called")
} else {
tx.debugLogger.Debug().Msg("Skipping request body processing, anomalous call before request headers evaluation")
switch {
case tx.lastPhase == types.PhaseRequestBody:
// This condition can happen quite often when ProcessPartial is used as the write body functions call ProcessRequestBody when
// the limit is reached
tx.debugLogger.Debug().Msg("Request body processing has been already performed")
case tx.lastPhase > types.PhaseRequestBody:
tx.debugLogger.Warn().Msg("Skipping anomalous call to ProcessRequestBody. It should have already been called")
default:
tx.debugLogger.Warn().Msg("Skipping anomalous call to ProcessRequestBody. It has been called before request headers evaluation")
}
return nil, nil
}
Expand Down Expand Up @@ -1215,9 +1219,9 @@ func (tx *Transaction) ReadResponseBodyFrom(r io.Reader) (*types.Interruption, i

// ProcessResponseBody Perform the analysis of the the response body (if any)
//
// This method perform the analysis on the response body. It is optional to
// call that method. If this API consumer already knows that there isn't a
// body for inspect it is recommended to skip this step.
// It is recommended to call this method even if it is not expected to have a body.
// It permits to execute rules belonging to request body phase, but not necesarily
// processing the response body.
//
// note Remember to check for a possible intervention.
func (tx *Transaction) ProcessResponseBody() (*types.Interruption, error) {
Expand All @@ -1231,14 +1235,18 @@ func (tx *Transaction) ProcessResponseBody() (*types.Interruption, error) {
}

if tx.lastPhase != types.PhaseResponseHeaders {
if tx.lastPhase >= types.PhaseResponseBody {
// Phase already evaluated or skipped
tx.debugLogger.Warn().Msg("ProcessResponseBody should have already been called")
} else {
switch {
case tx.lastPhase == types.PhaseResponseBody:
// This condition can happen quite often when ProcessPartial is used as the write body functions call ProcessResponseBody when
// the limit is reached
tx.debugLogger.Debug().Msg("Response body processing has been already performed")
case tx.lastPhase > types.PhaseResponseBody:
tx.debugLogger.Warn().Msg("Skipping anomalous call to ProcessResponseBody. It should have already been called")
default:
// Prevents evaluating response body rules if last phase has not been response headers. It may happen
// when a server returns an error prior to evaluating WAF rules, but ResponseBody is still called at
// the end of http stream
tx.debugLogger.Debug().Msg("Skipping response body processing, anomalous call before response headers evaluation")
tx.debugLogger.Warn().Msg("Skipping anomalous call to ProcessResponseBody. It has been called before response headers evaluation")
}
return nil, nil
}
Expand Down
4 changes: 2 additions & 2 deletions internal/corazawaf/transaction_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,10 +1018,10 @@ func TestProcessBodiesSkippedIfHeadersPhasesNotReached(t *testing.T) {
if want, have := 3, len(logEntries); want != have {
t.Fatalf("unexpected number of log entries, want %d, have %d", want, have)
}
if want, have := "anomalous call before request headers evaluation", logEntries[1]; !strings.Contains(have, want) {
if want, have := "has been called before request headers evaluation", logEntries[1]; !strings.Contains(have, want) {
t.Fatalf("unexpected message, want %q, have %q", want, have)
}
if want, have := "anomalous call before response headers evaluation", logEntries[2]; !strings.Contains(have, want) {
if want, have := "has been called before response headers evaluation", logEntries[2]; !strings.Contains(have, want) {
t.Fatalf("unexpected message, want %q, have %q", want, have)
}
if err := tx.Close(); err != nil {
Expand Down
16 changes: 10 additions & 6 deletions types/transaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ type Transaction interface {

// ProcessRequestBody Performs the analysis of the request body (if any)
//
// This method perform the analysis on the request body. It is optional to
// call that function. If this API consumer already knows that there isn't a
// body for inspect it is recommended to skip this step.
// It is recommended to call this method even if it is not expected to have a body.
// It permits to execute rules belonging to request body phase, but not necesarily
// processing the request body.
//
// Remember to check for a possible intervention.
ProcessRequestBody() (*Interruption, error)
Expand All @@ -88,6 +88,8 @@ type Transaction interface {
// returns an interruption if the body is bigger than the limit and the action is to
// reject. This is specially convenient to resolve an interruption before copying
// the body into the request body buffer.
// ProcessRequestBody is called automatically when the action is to process partially
// the body (up to the limit) if the limit is reached.
//
// It returns the corresponding interruption, the number of bytes written an error if any.
WriteRequestBody(b []byte) (*Interruption, int, error)
Expand All @@ -96,6 +98,8 @@ type Transaction interface {
// returns an interruption if the body is bigger than the limit and the action is to
// reject. This is specially convenient to resolve an interruption before copying
// the body into the request body buffer.
// ProcessRequestBody is called automatically when the action is to process partially
// the body (up to the limit) if the limit is reached.
//
// It returns the corresponding interruption, the number of bytes written an error if any.
ReadRequestBodyFrom(io.Reader) (*Interruption, int, error)
Expand All @@ -120,9 +124,9 @@ type Transaction interface {

// ProcessResponseBody Perform the analysis of the response body (if any)
//
// This method perform the analysis on the response body. It is optional to
// call that method. If this API consumer already knows that there isn't a
// body for inspect it is recommended to skip this step.
// It is recommended to call this method even if it is not expected to have a body.
// It permits to execute rules belonging to request body phase, but not necesarily
// processing the response body.
//
// note Remember to check for a possible intervention.
ProcessResponseBody() (*Interruption, error)
Expand Down
Loading