Skip to content

Commit

Permalink
chore: keep the handler error instead of trying to hijack it as it br…
Browse files Browse the repository at this point in the history
…eaks the user experience. (#85)

* chore: keep the handler error instead of trying to hijack it as it breaks the user experience.

Currently when a later in the chain middleware returned an error with a certain status code, we attempted to wrap that error with an own error which lead to misleading behaviours e.g. #83.

* chore: returns error to interrupt flow.
  • Loading branch information
jcchavezs authored Jul 10, 2023
1 parent b29f439 commit 5626d6c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 23 deletions.
28 changes: 11 additions & 17 deletions coraza.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package coraza

import (
"errors"
"net/http"
"path/filepath"
"strings"
Expand Down Expand Up @@ -91,6 +92,8 @@ func (m *corazaModule) Validate() error {
return nil
}

var errInterruptionTriggered = errors.New("interruption triggered")

// ServeHTTP implements caddyhttp.MiddlewareHandler.
func (m corazaModule) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
id := randomString(16)
Expand All @@ -115,35 +118,26 @@ func (m corazaModule) ServeHTTP(w http.ResponseWriter, r *http.Request, next cad
// It fails if any of these functions returns an error and it stops on interruption.
if it, err := processRequest(tx, r); err != nil {
return caddyhttp.HandlerError{
StatusCode: 500,
StatusCode: http.StatusInternalServerError,
ID: tx.ID(),
Err: err,
}
} else if it != nil {
w.WriteHeader(obtainStatusCodeFromInterruptionOrDefault(it, http.StatusOK))
return nil
return caddyhttp.HandlerError{
StatusCode: obtainStatusCodeFromInterruptionOrDefault(it, http.StatusOK),
ID: tx.ID(),
Err: errInterruptionTriggered,
}
}

ww, processResponse := wrap(w, r, tx)

// We continue with the other middlewares by catching the response
if err := next.ServeHTTP(ww, r); err != nil {
return caddyhttp.HandlerError{
StatusCode: 500,
ID: tx.ID(),
Err: err,
}
return err
}

if err := processResponse(tx, r); err != nil {
return caddyhttp.HandlerError{
StatusCode: 500,
ID: tx.ID(),
Err: err,
}
}

return nil
return processResponse(tx, r)
}

// Unmarshal Caddyfile implements caddyfile.Unmarshaler.
Expand Down
32 changes: 26 additions & 6 deletions interceptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"log"
"net/http"

"github.com/caddyserver/caddy/v2/modules/caddyhttp"
"github.com/corazawaf/coraza/v3/types"
)

Expand Down Expand Up @@ -128,31 +129,50 @@ func wrap(w http.ResponseWriter, r *http.Request, tx types.Transaction) (
}

if tx.IsResponseBodyAccessible() && tx.IsResponseBodyProcessable() {

if it, err := tx.ProcessResponseBody(); err != nil {
i.overrideWriteHeader(http.StatusInternalServerError)
i.flushWriteHeader()
return err

return caddyhttp.HandlerError{
ID: tx.ID(),
StatusCode: http.StatusInternalServerError,
Err: err,
}
} else if it != nil {
i.overrideWriteHeader(obtainStatusCodeFromInterruptionOrDefault(it, i.statusCode))
code := obtainStatusCodeFromInterruptionOrDefault(it, i.statusCode)
i.overrideWriteHeader(code)
i.flushWriteHeader()
return nil

return caddyhttp.HandlerError{
ID: tx.ID(),
StatusCode: code,
Err: errInterruptionTriggered,
}
}

// we release the buffer
reader, err := tx.ResponseBodyReader()
if err != nil {
i.overrideWriteHeader(http.StatusInternalServerError)
i.flushWriteHeader()
return fmt.Errorf("failed to release the response body reader: %v", err)

return caddyhttp.HandlerError{
ID: tx.ID(),
StatusCode: http.StatusInternalServerError,
Err: fmt.Errorf("failed to release the response body reader: %v", err),
}
}

// this is the last opportunity we have to report the resolved status code
// as next step is write into the response writer (triggering a 200 in the
// response status code.)
i.flushWriteHeader()
if _, err := io.Copy(w, reader); err != nil {
return fmt.Errorf("failed to copy the response body: %v", err)
return caddyhttp.HandlerError{
ID: tx.ID(),
StatusCode: http.StatusInternalServerError,
Err: fmt.Errorf("failed to copy the response body: %v", err),
}
}
} else {
i.flushWriteHeader()
Expand Down

0 comments on commit 5626d6c

Please sign in to comment.