chore: refactor process body related logs and doc (#1187)

refactor process body related logs and doc

http/middleware.go

@@ -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)
 		}
 	}
 

internal/corazawaf/transaction.go

@@ -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) {
@@ -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
 	}
@@ -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) {
@@ -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
 	}

internal/corazawaf/transaction_test.go

@@ -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 {

types/transaction.go

@@ -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)
@@ -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)
@@ -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)
@@ -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)