Repository: google/go-safeweb
Pull request #222 (2020-11-30 07:33:58)

Interceptors' `Commit` and `OnError` methods can no longer affect the flow by calling write operations
`Commit` and `OnError` interceptor methods can no longer invoke write calls on a `ResponseWriter`. Instead, they receive a `ResponseHeadersWriter` that allows them to modify the HTTP response headers without affecting the control flow. Similarly, they can change the `Response` that's passed to them.

The only way to affect the control flow is for the interceptor to panic, in which case it is treated as an `InternalServerError`.

This introduces a conceptual change in how the framework operates.

1. Once the server has committed to a response (either via `Write` or `WriteError`), no more write operations are allowed. This simplifies further error handling mechanics. Call cycle detection is not needed anymore (e.g. `Write` -> `WriteError` cannot be introduced by user's code).
2. `Commit` and `OnError` can still modify the response, but has to do so by modifying the headers using `ResponseHeadersWriter` and altering the received `Response` object.

#164, #79


Patch:
diff --git a/safehttp/interceptor.go b/safehttp/interceptor.go
index ce8cbbe..23a43e5 100644
--- a/safehttp/interceptor.go
+++ b/safehttp/interceptor.go
@@ -29,12 +29,12 @@ type Interceptor interface {
 	// Commit runs before the response is written by the Dispatcher. If an error
 	// is written to the ResponseWriter, then the Commit phases from the
 	// remaining interceptors won't execute.
-	Commit(w ResponseWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig) Result
+	Commit(w ResponseHeadersWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig)
 
 	// OnError runs when ResponseWriter.WriteError is called, before the
 	// actual error response is written. An attempt to write to the
 	// ResponseWriter in this phase will result in an irrecoverable error.
-	OnError(w ResponseWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig) Result
+	OnError(w ResponseHeadersWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig)
 }
 
 // InterceptorConfig is a configuration of an interceptor.
@@ -61,13 +61,13 @@ func (ci *ConfiguredInterceptor) Before(w ResponseWriter, r *IncomingRequest) Re
 // Commit runs before the response is written by the Dispatcher. If an error
 // is written to the ResponseWriter, then the Commit phases from the
 // remaining interceptors won't execute.
-func (ci *ConfiguredInterceptor) Commit(w ResponseWriter, r *IncomingRequest, resp Response) Result {
-	return ci.interceptor.Commit(w, r, resp, ci.config)
+func (ci *ConfiguredInterceptor) Commit(w ResponseHeadersWriter, r *IncomingRequest, resp Response) {
+	ci.interceptor.Commit(w, r, resp, ci.config)
 }
 
 // OnError runs when ResponseWriter.WriteError is called, before the
 // actual error response is written. An attempt to write to the
 // ResponseWriter in this phase will result in an irrecoverable error.
-func (ci *ConfiguredInterceptor) OnError(w ResponseWriter, r *IncomingRequest, resp Response) Result {
-	return ci.interceptor.OnError(w, r, resp, ci.config)
+func (ci *ConfiguredInterceptor) OnError(w ResponseHeadersWriter, r *IncomingRequest, resp Response) {
+	ci.interceptor.OnError(w, r, resp, ci.config)
 }
diff --git a/safehttp/plugins/coop/coop.go b/safehttp/plugins/coop/coop.go
index e460c53..54db927 100644
--- a/safehttp/plugins/coop/coop.go
+++ b/safehttp/plugins/coop/coop.go
@@ -91,16 +91,14 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
 //
 // TODO: should OnError take as argument something that notifies it the commit
 // phase was already called?
-func (it Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // Overrider is a safehttp.InterceptorConfig that allows to override COOP for a specific handler.
diff --git a/safehttp/plugins/cors/cors.go b/safehttp/plugins/cors/cors.go
index 3f00fc0..6a4919d 100644
--- a/safehttp/plugins/cors/cors.go
+++ b/safehttp/plugins/cors/cors.go
@@ -155,8 +155,7 @@ func (it *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingReq
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it *Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 func appendToVary(w safehttp.ResponseWriter, val string) {
@@ -169,8 +168,7 @@ func appendToVary(w safehttp.ResponseWriter, val string) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // preflight handles requests that have the method OPTIONS.
diff --git a/safehttp/plugins/csp/csp.go b/safehttp/plugins/csp/csp.go
index 47065a3..75d50d3 100644
--- a/safehttp/plugins/csp/csp.go
+++ b/safehttp/plugins/csp/csp.go
@@ -26,9 +26,10 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"github.com/google/go-safeweb/safehttp/plugins/htmlinject"
 	"strings"
 
+	"github.com/google/go-safeweb/safehttp/plugins/htmlinject"
+
 	"github.com/google/go-safeweb/safehttp"
 )
 
@@ -219,17 +220,17 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 // Commit adds the nonce to the safehttp.TemplateResponse which is going to be
 // injected as the value of the nonce attribute in <script> and <link> tags. The
 // nonce is going to be unique for each safehttp.IncomingRequest.
-func (it Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (it Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	tmplResp, ok := resp.(safehttp.TemplateResponse)
 	if !ok {
-		return safehttp.NotWritten()
+		return
 	}
 
 	nonce, err := Nonce(r.Context())
 	if err != nil {
 		// The nonce should have been added in the Before stage and, if that is
 		// not the case, a server misconfiguration occured.
-		return w.WriteError(safehttp.StatusInternalServerError)
+		panic("no CSP nonce")
 	}
 
 	// TODO(maramihali@): Change the key when function names are exported by
@@ -237,10 +238,9 @@ func (it Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 	// TODO: What should happen if the CSPNonce is not present in the
 	// tr.FuncMap?
 	tmplResp.FuncMap[htmlinject.CSPNoncesDefaultFuncName] = func() string { return nonce }
-	return safehttp.NotWritten()
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
+	return
 }
diff --git a/safehttp/plugins/fetch_metadata/fetch_metadata.go b/safehttp/plugins/fetch_metadata/fetch_metadata.go
index b1779ec..85eafc6 100644
--- a/safehttp/plugins/fetch_metadata/fetch_metadata.go
+++ b/safehttp/plugins/fetch_metadata/fetch_metadata.go
@@ -170,11 +170,9 @@ func (p *Plugin) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest,
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (p *Plugin) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p *Plugin) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (p *Plugin) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p *Plugin) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/hostcheck/hostcheck.go b/safehttp/plugins/hostcheck/hostcheck.go
index eaa373e..d20cabf 100644
--- a/safehttp/plugins/hostcheck/hostcheck.go
+++ b/safehttp/plugins/hostcheck/hostcheck.go
@@ -51,11 +51,9 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
index f69d725..c395a52 100644
--- a/safehttp/plugins/hsts/hsts.go
+++ b/safehttp/plugins/hsts/hsts.go
@@ -113,11 +113,9 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/staticheaders/staticheaders.go b/safehttp/plugins/staticheaders/staticheaders.go
index 39afd22..b29d953 100644
--- a/safehttp/plugins/staticheaders/staticheaders.go
+++ b/safehttp/plugins/staticheaders/staticheaders.go
@@ -51,11 +51,9 @@ func (Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/xsrf/angular/xsrf.go b/safehttp/plugins/xsrf/angular/xsrf.go
index f8428bf..b206931 100644
--- a/safehttp/plugins/xsrf/angular/xsrf.go
+++ b/safehttp/plugins/xsrf/angular/xsrf.go
@@ -18,9 +18,10 @@ import (
 	"crypto/rand"
 	"encoding/base64"
 	"fmt"
+	"time"
+
 	"github.com/google/go-safeweb/safehttp"
 	"github.com/google/go-safeweb/safehttp/plugins/xsrf"
-	"time"
 )
 
 // Interceptor provides protection against Cross-Site Request Forgery attacks
@@ -73,7 +74,7 @@ func (it *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingReq
 	return safehttp.NotWritten()
 }
 
-func (it *Interceptor) addTokenCookie(w safehttp.ResponseWriter) error {
+func (it *Interceptor) addTokenCookie(w safehttp.ResponseHeadersWriter) error {
 	tok := make([]byte, 20)
 	if _, err := rand.Read(tok); err != nil {
 		return fmt.Errorf("crypto/rand.Read: %v", err)
@@ -95,26 +96,24 @@ func (it *Interceptor) addTokenCookie(w safehttp.ResponseWriter) error {
 // preserving request (GET, HEAD or OPTION) and sets it in the response. On
 // every subsequent request the cookie is expected alongside a header that
 // matches its value.
-func (it *Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
+func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 	if c, err := r.Cookie(it.TokenCookieName); err == nil && c.Value() != "" {
 		// The XSRF cookie is there so we don't need to do anything else.
-		return safehttp.NotWritten()
+		return
 	}
 
 	if !xsrf.StatePreserving(r) {
 		// This should never happen as, if this is a state-changing request and
 		// it lacks the cookie, it would've been already rejected by Before.
-		return w.WriteError(safehttp.StatusInternalServerError)
+		panic("not state preserving and no cookie in Commit")
 	}
 
 	if err := it.addTokenCookie(w); err != nil {
 		// This is a server misconfiguration.
-		return w.WriteError(safehttp.StatusInternalServerError)
+		panic("cannot add token cookie")
 	}
-	return safehttp.NotWritten()
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it *Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it *Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/xsrf/html/xsrf.go b/safehttp/plugins/xsrf/html/xsrf.go
index a65fdb5..a3a2599 100644
--- a/safehttp/plugins/xsrf/html/xsrf.go
+++ b/safehttp/plugins/xsrf/html/xsrf.go
@@ -18,6 +18,7 @@ import (
 	"crypto/rand"
 	"encoding/base64"
 	"fmt"
+
 	"github.com/google/go-safeweb/safehttp"
 	"github.com/google/go-safeweb/safehttp/plugins/htmlinject"
 	"github.com/google/go-safeweb/safehttp/plugins/xsrf"
@@ -40,7 +41,7 @@ type Interceptor struct {
 
 var _ safehttp.Interceptor = &Interceptor{}
 
-func addCookieID(w safehttp.ResponseWriter) (*safehttp.Cookie, error) {
+func addCookieID(w safehttp.ResponseHeadersWriter) (*safehttp.Cookie, error) {
 	buf := make([]byte, 20)
 	if _, err := rand.Read(buf); err != nil {
 		return nil, fmt.Errorf("crypto/rand.Read: %v", err)
@@ -103,33 +104,33 @@ func (it *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingReq
 // For every authorized request, the interceptor also generates a
 // cryptographically-safe XSRF token using the appKey, the cookie and the path
 // visited. This is then injected as a hidden input field in HTML forms.
-func (it *Interceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
+func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 	cookieID, err := r.Cookie(cookieIDKey)
 	if err != nil {
 		if !xsrf.StatePreserving(r) {
 			// This should never happen as, if this is a state-changing request and it lacks the cookie, it would've been already rejected by Before.
-			return w.WriteError(safehttp.StatusInternalServerError)
+			panic("not state preserving and no cookie in Commit")
 		}
 		cookieID, err = addCookieID(w)
 		if err != nil {
 			// This is a server misconfiguration.
-			return w.WriteError(safehttp.StatusInternalServerError)
+			panic("cannot add cookie ID")
 		}
 	}
 
 	tmplResp, ok := resp.(safehttp.TemplateResponse)
 	if !ok {
-		return safehttp.NotWritten()
+		// If it's not a template response, we cannot inject the token.
+		// TODO: should this be an error?
+		return
 	}
 
 	tok := xsrftoken.Generate(it.SecretAppKey, cookieID.Value(), r.URL.Path())
 	// TODO: what should happen if the XSRFToken key is not present in the
 	// tr.FuncMap?
 	tmplResp.FuncMap[htmlinject.XSRFTokensDefaultFuncName] = func() string { return tok }
-	return safehttp.NotWritten()
 }
 
 // OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it *Interceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (it *Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/response_writer.go b/safehttp/response_writer.go
index 41e32d6..440f28d 100644
--- a/safehttp/response_writer.go
+++ b/safehttp/response_writer.go
@@ -21,6 +21,8 @@ package safehttp
 //
 // A ResponseWriter may not be used after the Handler.ServeHTTP method has returned.
 type ResponseWriter interface {
+	ResponseHeadersWriter
+
 	// Write writes a safe response.
 	Write(resp Response) Result
 
@@ -38,7 +40,12 @@ type ResponseWriter interface {
 	//
 	// If the ResponseWriter has already been written to, then this method panics.
 	Redirect(r *IncomingRequest, url string, code StatusCode) Result
+}
 
+// ResponseHeadersWriter is used to alter the HTTP response headers.
+//
+// A ResponseHeadersWriter may not be used after the Handler.ServeHTTP method has returned.
+type ResponseHeadersWriter interface {
 	// Header returns the collection of headers that will be set
 	// on the response. Headers must be set before writing a
 	// response (e.g. Write, WriteTemplate).


Test patch:
diff --git a/safehttp/mux_test.go b/safehttp/mux_test.go
index 373ec6b..47fb638 100644
--- a/safehttp/mux_test.go
+++ b/safehttp/mux_test.go
@@ -174,12 +174,10 @@ func (p setHeaderInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.Inco
 	return safehttp.NotWritten()
 }
 
-func (p setHeaderInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p setHeaderInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
-func (p setHeaderInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p setHeaderInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type internalErrorInterceptor struct{}
@@ -188,13 +186,11 @@ func (internalErrorInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.In
 	return w.WriteError(safehttp.StatusInternalServerError)
 }
 
-func (internalErrorInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (internalErrorInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	w.Header().Set("Foo", "this should not be reached")
-	return safehttp.NotWritten()
 }
 
-func (internalErrorInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (internalErrorInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type claimHeaderInterceptor struct {
@@ -209,12 +205,10 @@ func (p *claimHeaderInterceptor) Before(w safehttp.ResponseWriter, r *safehttp.I
 	return safehttp.NotWritten()
 }
 
-func (p *claimHeaderInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p *claimHeaderInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
-func (p *claimHeaderInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p *claimHeaderInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 func claimInterceptorSetHeader(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, value string) {
@@ -228,12 +222,10 @@ func (panickingInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.Incomi
 	panic("bad")
 }
 
-func (panickingInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (panickingInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
-func (panickingInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (panickingInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type committerInterceptor struct{}
@@ -242,13 +234,11 @@ func (committerInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.Incomi
 	return safehttp.NotWritten()
 }
 
-func (committerInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (committerInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	w.Header().Set("foo", "bar")
-	return safehttp.NotWritten()
 }
 
-func (committerInterceptor) OnError(w safehttp.ResponseWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (committerInterceptor) OnError(w safehttp.ResponseHeadersWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type setHeaderErroringInterceptor struct{}
@@ -257,14 +247,12 @@ func (setHeaderErroringInterceptor) Before(w safehttp.ResponseWriter, _ *safehtt
 	return w.WriteError(safehttp.StatusForbidden)
 }
 
-func (setHeaderErroringInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (setHeaderErroringInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	w.Header().Set("name", "foo")
-	return safehttp.Result{}
 }
 
-func (setHeaderErroringInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (setHeaderErroringInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	w.Header().Set("name", "bar")
-	return safehttp.Result{}
 }
 
 func TestMuxInterceptors(t *testing.T) {
@@ -448,7 +436,7 @@ func (p setHeaderConfigInterceptor) Before(w safehttp.ResponseWriter, _ *safehtt
 	return safehttp.NotWritten()
 }
 
-func (p setHeaderConfigInterceptor) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (p setHeaderConfigInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	name := "Commit-Pizza"
 	value := "Hawaii"
 	if c, ok := cfg.(setHeaderConfig); ok {
@@ -456,11 +444,9 @@ func (p setHeaderConfigInterceptor) Commit(w safehttp.ResponseWriter, r *safehtt
 		value = c.value
 	}
 	w.Header().Set(name, value)
-	return safehttp.NotWritten()
 }
 
-func (p setHeaderConfigInterceptor) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (p setHeaderConfigInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type noInterceptorConfig struct{}
@@ -541,60 +527,54 @@ func (interceptorOne) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 	return safehttp.NotWritten()
 }
 
-func (interceptorOne) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (interceptorOne) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	if w.Header().Get("Commit2") != "b" {
-		w.WriteError(safehttp.StatusInternalServerError)
+		panic("server bug")
 	}
 	w.Header().Set("Commit1", "a")
-	return safehttp.NotWritten()
 }
 
-func (interceptorOne) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (interceptorOne) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type interceptorTwo struct{}
 
 func (interceptorTwo) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
 	if w.Header().Get("pizza") != "diavola" {
-		w.WriteError(safehttp.StatusInternalServerError)
+		panic("server bug")
 	}
 	w.Header().Set("spaghetti", "bolognese")
 	return safehttp.NotWritten()
 }
 
-func (interceptorTwo) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (interceptorTwo) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	if w.Header().Get("Commit3") != "c" {
-		w.WriteError(safehttp.StatusInternalServerError)
+		panic("server bug")
 	}
 	w.Header().Set("Commit2", "b")
-	return safehttp.NotWritten()
 }
 
-func (interceptorTwo) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (interceptorTwo) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type interceptorThree struct{}
 
 func (interceptorThree) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
 	if w.Header().Get("spaghetti") != "bolognese" {
-		w.WriteError(safehttp.StatusInternalServerError)
+		panic("server bug")
 	}
 	w.Header().Set("dessert", "tiramisu")
 	return safehttp.NotWritten()
 }
 
-func (interceptorThree) Commit(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
+func (interceptorThree) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 	if w.Header().Get("Dessert") != "tiramisu" {
-		w.WriteError(safehttp.StatusInternalServerError)
+		panic("server bug")
 	}
 	w.Header().Set("Commit3", "c")
-	return safehttp.NotWritten()
 }
 
-func (interceptorThree) OnError(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.NotWritten()
+func (interceptorThree) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 func TestMuxDeterministicInterceptorOrder(t *testing.T) {
@@ -667,13 +647,11 @@ func (onErrorWriteInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.Inc
 	return w.WriteError(safehttp.StatusBadRequest)
 }
 
-func (onErrorWriteInterceptor) Commit(_ safehttp.ResponseWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	return safehttp.Result{}
+func (onErrorWriteInterceptor) Commit(_ safehttp.ResponseHeadersWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, _ safehttp.InterceptorConfig) {
 }
 
-func (onErrorWriteInterceptor) OnError(w safehttp.ResponseWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, _ safehttp.InterceptorConfig) safehttp.Result {
-	w.Header().Set("header", "bad")
-	return w.WriteError(safehttp.StatusInsufficientStorage)
+func (onErrorWriteInterceptor) OnError(w safehttp.ResponseHeadersWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, _ safehttp.InterceptorConfig) {
+	panic("panic during onError")
 }
 
 func TestMuxInterceptorWriteInOnError(t *testing.T) {
diff --git a/safehttp/plugins/csp/csp_test.go b/safehttp/plugins/csp/csp_test.go
index 6b53cd3..dbbc026 100644
--- a/safehttp/plugins/csp/csp_test.go
+++ b/safehttp/plugins/csp/csp_test.go
@@ -278,26 +278,13 @@ func TestCommitMissingNonce(t *testing.T) {
 
 	it := Interceptor{}
 	tr := safehttp.TemplateResponse{FuncMap: map[string]interface{}{}}
-	it.Commit(rec.ResponseWriter, req, tr, nil)
-
-	wantFuncMap := map[string]interface{}{}
-	if diff := cmp.Diff(wantFuncMap, tr.FuncMap); diff != "" {
-		t.Errorf("tr.FuncMap: mismatch (-want +got):\n%s", diff)
-	}
 
-	if got, want := rec.Status(), safehttp.StatusInternalServerError; got != want {
-		t.Errorf("rec.Status(): got %v, want %v", got, want)
-	}
-	wantHeaders := map[string][]string{
-		"Content-Type":           {"text/plain; charset=utf-8"},
-		"X-Content-Type-Options": {"nosniff"},
-	}
-	if diff := cmp.Diff(wantHeaders, map[string][]string(rec.Header())); diff != "" {
-		t.Errorf("rw.header mismatch (-want +got):\n%s", diff)
-	}
-	if got, want := rec.Body(), "Internal Server Error\n"; got != want {
-		t.Errorf("rec.Body(): got %q want %q", got, want)
-	}
+	defer func() {
+		if r := recover(); r == nil {
+			t.Fatal("expected panic")
+		}
+	}()
+	it.Commit(rec.ResponseWriter, req, tr, nil)
 }
 
 func TestCommitNotTemplateResponse(t *testing.T) {
diff --git a/safehttp/plugins/xsrf/angular/xsrf_test.go b/safehttp/plugins/xsrf/angular/xsrf_test.go
index efc7733..8d4cae1 100644
--- a/safehttp/plugins/xsrf/angular/xsrf_test.go
+++ b/safehttp/plugins/xsrf/angular/xsrf_test.go
@@ -15,11 +15,12 @@
 package xsrfangular
 
 import (
+	"strings"
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-safeweb/safehttp"
 	"github.com/google/go-safeweb/safehttp/safehttptest"
-	"strings"
-	"testing"
 )
 
 const (
@@ -78,23 +79,13 @@ func TestAddCookieFail(t *testing.T) {
 	req := safehttptest.NewRequest(safehttp.MethodGet, "/", nil)
 	rec := safehttptest.NewResponseRecorder()
 	it := &Interceptor{}
-	it.Commit(rec.ResponseWriter, req, nil, nil)
 
-	if wantStatus := safehttp.StatusInternalServerError; rec.Status() != wantStatus {
-		t.Errorf("rec.Status(): got %v want %v", rec.Status(), wantStatus)
-	}
-
-	wantHeaders := map[string][]string{
-		"Content-Type":           {"text/plain; charset=utf-8"},
-		"X-Content-Type-Options": {"nosniff"},
-	}
-	if diff := cmp.Diff(wantHeaders, map[string][]string(rec.Header())); diff != "" {
-		t.Errorf("rec.Header(): mismatch (-want +got):\n%s", diff)
-	}
-
-	if wantBody, gotBody := "Internal Server Error\n", rec.Body(); wantBody != gotBody {
-		t.Errorf("response body: got %v, want %v", gotBody, wantBody)
-	}
+	defer func() {
+		if r := recover(); r == nil {
+			t.Fatal("expected panic")
+		}
+	}()
+	it.Commit(rec.ResponseWriter, req, nil, nil)
 }
 
 func TestPostProtection(t *testing.T) {
diff --git a/safehttp/plugins/xsrf/html/xsrf_test.go b/safehttp/plugins/xsrf/html/xsrf_test.go
index e50238a..3bee81b 100644
--- a/safehttp/plugins/xsrf/html/xsrf_test.go
+++ b/safehttp/plugins/xsrf/html/xsrf_test.go
@@ -15,12 +15,13 @@
 package xsrfhtml
 
 import (
+	"strings"
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-safeweb/safehttp"
 	"github.com/google/go-safeweb/safehttp/safehttptest"
 	"golang.org/x/net/xsrftoken"
-	"strings"
-	"testing"
 )
 
 var (
@@ -247,6 +248,7 @@ func TestMissingCookieInPostRequest(t *testing.T) {
 		name       string
 		stage      func(it *Interceptor, rw safehttp.ResponseWriter, req *safehttp.IncomingRequest)
 		wantStatus safehttp.StatusCode
+		wantPanic  bool
 	}{
 		{
 			name: "In Before stage",
@@ -261,7 +263,7 @@ func TestMissingCookieInPostRequest(t *testing.T) {
 			stage: func(it *Interceptor, rw safehttp.ResponseWriter, req *safehttp.IncomingRequest) {
 				it.Commit(rw, req, nil, nil)
 			},
-			wantStatus: safehttp.StatusInternalServerError,
+			wantPanic: true,
 		},
 	}
 
@@ -271,8 +273,20 @@ func TestMissingCookieInPostRequest(t *testing.T) {
 			req := safehttptest.NewRequest(safehttp.MethodPost, "/", strings.NewReader("foo=bar"))
 			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
 
+			if test.wantPanic {
+				defer func() {
+					if r := recover(); r == nil {
+						t.Fatal("expected panic")
+					}
+				}()
+			}
+
 			test.stage(&Interceptor{SecretAppKey: "testSecretAppKey"}, rec.ResponseWriter, req)
 
+			if test.wantPanic {
+				t.Fatal("this piece of the test should never be reached")
+			}
+
 			if gotStatus := rec.Status(); gotStatus != test.wantStatus {
 				t.Errorf("rec.Status(): got %v, want %v", gotStatus, test.wantStatus)
 			}