Repository: google/go-safeweb
Pull request #231 (2020-12-22 13:38:17)

Custom error responses
Adding custom error responses to the framework.

1. `ErrorResponse` is now an interface type. `StatusCode` implements it.
2. The `Dispatcher` now implement an `Error` method, accepting an `ErrorResponse` interface object.
3. Error responses written through `safehttp.ResponseWriter.WriteError` now go through the `Commit` phase. This means that `Interceptor.OnError` is not needed anymore.
4. The `ServeMuxConfig.Dispatcher` is now a public field (discovered that it's needed when writing the tests).

kele: Fixes #79.
Fixes #179.

Patch:
diff --git a/safehttp/default_dispatcher.go b/safehttp/default_dispatcher.go
index fa644e9..bacce2a 100644
--- a/safehttp/default_dispatcher.go
+++ b/safehttp/default_dispatcher.go
@@ -82,3 +82,16 @@ func (DefaultDispatcher) Write(rw http.ResponseWriter, resp Response) error {
 		return fmt.Errorf("%T is not a safe response type and it cannot be written", resp)
 	}
 }
+
+// Error writes the error response to the http.ResponseWriter if it's deemed
+// safe. It returns a non-nil error if the response is deemed unsafe or if the
+// writing operation fails.
+func (DefaultDispatcher) Error(rw http.ResponseWriter, resp ErrorResponse) error {
+	switch x := resp.(type) {
+	case StatusCode:
+		http.Error(rw, http.StatusText(int(x.Code())), int(x.Code()))
+		return nil
+	default:
+		return fmt.Errorf("%T is not a safe response type and it cannot be written", resp)
+	}
+}
diff --git a/safehttp/dispatcher.go b/safehttp/dispatcher.go
index 8aefe1f..af0dfa8 100644
--- a/safehttp/dispatcher.go
+++ b/safehttp/dispatcher.go
@@ -39,4 +39,11 @@ type Dispatcher interface {
 	// provided Response should not be written to the http.ResponseWriter
 	// because it's unsafe.
 	Write(rw http.ResponseWriter, resp Response) error
+
+	// Error writes an ErrorResponse to the underlying http.ResponseWriter.
+	//
+	// It should return an error if the writing operation fails or if the
+	// provided Response should not be written to the http.ResponseWriter
+	// because it's unsafe.
+	Error(rw http.ResponseWriter, resp ErrorResponse) error
 }
diff --git a/safehttp/flight.go b/safehttp/flight.go
index bb7a923..81619ba 100644
--- a/safehttp/flight.go
+++ b/safehttp/flight.go
@@ -44,6 +44,9 @@ type flight struct {
 // TODO(kele): remove this once we have a better option to provide a
 // ResponseWriter implementation for interceptor testing.
 func DeprecatedNewResponseWriter(rw http.ResponseWriter, dispatcher Dispatcher) ResponseWriter {
+	if dispatcher == nil {
+		dispatcher = DefaultDispatcher{}
+	}
 	return &flight{
 		cfg:    HandlerConfig{Dispatcher: dispatcher},
 		rw:     rw,
@@ -70,7 +73,7 @@ func processRequest(cfg HandlerConfig, rw http.ResponseWriter, req *http.Request
 	// The net/http package handles all panics. In the early days of the
 	// framework we were handling them ourselves and running interceptors after
 	// a panic happened, but this adds lots of complexity to the codebase and
-	// still isn't perfect (e.g. what if OnError panics?). Instead, we just make
+	// still isn't perfect (e.g. what if Commit panics?). Instead, we just make
 	// sure to clear all the headers and cookies.
 	defer func() {
 		if r := recover(); r != nil {
@@ -139,15 +142,17 @@ func (f *flight) NoContent() Result {
 // status code.
 //
 // If the ResponseWriter has already been written to, then this method will panic.
-func (f *flight) WriteError(code StatusCode) Result {
+func (f *flight) WriteError(resp ErrorResponse) Result {
 	// TODO: accept custom error responses that need to go through the dispatcher.
 	if f.written {
 		panic("ResponseWriter was already written to")
 	}
 	f.written = true
-	resp := &ErrorResponse{Code: code}
-	f.errorPhase(resp)
-	http.Error(f.rw, http.StatusText(int(resp.Code)), int(resp.Code))
+	f.commitPhase(resp)
+	f.rw.WriteHeader(int(resp.Code()))
+	if err := f.cfg.Dispatcher.Error(f.rw, resp); err != nil {
+		panic(err)
+	}
 	return Result{}
 }
 
@@ -210,15 +215,6 @@ func (f *flight) commitPhase(resp Response) {
 	}
 }
 
-// errorPhase calls the OnError phases of all the interceptors associated with
-// a handler. This stage runs before an error response is written to the
-// ResponseWriter.
-func (f *flight) errorPhase(resp Response) {
-	for i := len(f.cfg.Interceptors) - 1; i >= 0; i-- {
-		f.cfg.Interceptors[i].OnError(f, f.req, resp)
-	}
-}
-
 // Result is the result of writing an HTTP response.
 //
 // Use ResponseWriter methods to obtain it.
diff --git a/safehttp/interceptor.go b/safehttp/interceptor.go
index 23a43e5..b4f7bcb 100644
--- a/safehttp/interceptor.go
+++ b/safehttp/interceptor.go
@@ -30,11 +30,6 @@ type Interceptor interface {
 	// is written to the ResponseWriter, then the Commit phases from the
 	// remaining interceptors won't execute.
 	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 ResponseHeadersWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig)
 }
 
 // InterceptorConfig is a configuration of an interceptor.
@@ -64,10 +59,3 @@ func (ci *ConfiguredInterceptor) Before(w ResponseWriter, r *IncomingRequest) Re
 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 ResponseHeadersWriter, r *IncomingRequest, resp Response) {
-	ci.interceptor.OnError(w, r, resp, ci.config)
-}
diff --git a/safehttp/mux.go b/safehttp/mux.go
index 6dc72ed..fb41b1b 100644
--- a/safehttp/mux.go
+++ b/safehttp/mux.go
@@ -109,7 +109,7 @@ func (m *ServeMux) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 // ServeMuxConfig is a builder for ServeMux.
 type ServeMuxConfig struct {
-	dispatcher   Dispatcher
+	Dispatcher   Dispatcher
 	handlers     []handlerRegistration
 	interceptors []Interceptor
 }
@@ -147,7 +147,7 @@ func (s *ServeMuxConfig) Intercept(i Interceptor) {
 
 // Mux returns the ServeMux with a copy of the current configuration.
 func (s *ServeMuxConfig) Mux() *ServeMux {
-	dispatcher := s.dispatcher
+	dispatcher := s.Dispatcher
 	if dispatcher == nil {
 		dispatcher = DefaultDispatcher{}
 	}
@@ -195,7 +195,7 @@ func configureInterceptors(interceptors []Interceptor, cfgs []InterceptorConfig)
 // plugins and some common handlers.
 func (s *ServeMuxConfig) Clone() *ServeMuxConfig {
 	c := &ServeMuxConfig{
-		dispatcher:   s.dispatcher,
+		Dispatcher:   s.Dispatcher,
 		handlers:     make([]handlerRegistration, len(s.handlers)),
 		interceptors: make([]Interceptor, len(s.interceptors)),
 	}
diff --git a/safehttp/plugins/coop/coop.go b/safehttp/plugins/coop/coop.go
index 54db927..1a179e3 100644
--- a/safehttp/plugins/coop/coop.go
+++ b/safehttp/plugins/coop/coop.go
@@ -94,13 +94,6 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
-
 // Overrider is a safehttp.InterceptorConfig that allows to override COOP for a specific handler.
 type Overrider serializedPolicies
 
diff --git a/safehttp/plugins/cors/cors.go b/safehttp/plugins/cors/cors.go
index 6a4919d..76bb92c 100644
--- a/safehttp/plugins/cors/cors.go
+++ b/safehttp/plugins/cors/cors.go
@@ -167,10 +167,6 @@ 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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
-
 // preflight handles requests that have the method OPTIONS.
 func (it *Interceptor) preflight(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.StatusCode {
 	rh := r.Header
diff --git a/safehttp/plugins/csp/csp.go b/safehttp/plugins/csp/csp.go
index 5f6c544..2a99fc1 100644
--- a/safehttp/plugins/csp/csp.go
+++ b/safehttp/plugins/csp/csp.go
@@ -238,8 +238,3 @@ func (it Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Incom
 	}
 	tmplResp.FuncMap[htmlinject.CSPNoncesDefaultFuncName] = func() string { return nonce }
 }
-
-// OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-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 85eafc6..53b685b 100644
--- a/safehttp/plugins/fetch_metadata/fetch_metadata.go
+++ b/safehttp/plugins/fetch_metadata/fetch_metadata.go
@@ -172,7 +172,3 @@ 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.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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/plugins/hostcheck/hostcheck.go b/safehttp/plugins/hostcheck/hostcheck.go
index d20cabf..3d86548 100644
--- a/safehttp/plugins/hostcheck/hostcheck.go
+++ b/safehttp/plugins/hostcheck/hostcheck.go
@@ -53,7 +53,3 @@ 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.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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
index c395a52..4056f14 100644
--- a/safehttp/plugins/hsts/hsts.go
+++ b/safehttp/plugins/hsts/hsts.go
@@ -115,7 +115,3 @@ 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.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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/plugins/staticheaders/staticheaders.go b/safehttp/plugins/staticheaders/staticheaders.go
index b29d953..9557e10 100644
--- a/safehttp/plugins/staticheaders/staticheaders.go
+++ b/safehttp/plugins/staticheaders/staticheaders.go
@@ -53,7 +53,3 @@ 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.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.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/plugins/xsrf/xsrfangular/xsrf.go b/safehttp/plugins/xsrf/xsrfangular/xsrf.go
index 6437cc5..400b90f 100644
--- a/safehttp/plugins/xsrf/xsrfangular/xsrf.go
+++ b/safehttp/plugins/xsrf/xsrfangular/xsrf.go
@@ -112,7 +112,3 @@ func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inco
 		panic("cannot add token cookie")
 	}
 }
-
-// OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it *Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
index b254bc0..6e40a40 100644
--- a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
+++ b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
@@ -131,7 +131,3 @@ func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inco
 	}
 	tmplResp.FuncMap[htmlinject.XSRFTokensDefaultFuncName] = func() string { return tok }
 }
-
-// OnError is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (it *Interceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
diff --git a/safehttp/response.go b/safehttp/response.go
index d6b195c..1e7c7e6 100644
--- a/safehttp/response.go
+++ b/safehttp/response.go
@@ -23,6 +23,12 @@ import (
 // supported by the Dispatcher.
 type Response interface{}
 
+// ErrorResponse is an HTTP error response. The Dispatcher is responsible for
+// determining whether it is safe.
+type ErrorResponse interface {
+	Code() StatusCode
+}
+
 // JSONResponse should encapsulate a valid JSON object that will be serialised
 // and written to the http.ResponseWriter using a JSON encoder.
 type JSONResponse struct {
@@ -72,10 +78,3 @@ func ExecuteTemplateWithFuncs(w ResponseWriter, t Template, data interface{}, fm
 // NoContentResponse is sent to the commit phase when it's initiated from
 // ResponseWriter.NoContent.
 type NoContentResponse struct{}
-
-// ErrorResponse is sent to the on error phase when initiated from
-// ResponseWriter.WriteError, encapsulating the status code with which the
-// method was called.
-type ErrorResponse struct {
-	Code StatusCode
-}
diff --git a/safehttp/response_writer.go b/safehttp/response_writer.go
index 440f28d..bbc4e9f 100644
--- a/safehttp/response_writer.go
+++ b/safehttp/response_writer.go
@@ -31,10 +31,10 @@ type ResponseWriter interface {
 	// If the ResponseWriter has already been written to, then this method panics.
 	NoContent() Result
 
-	// WriteError writes an error response (400-599) according to the provided status code.
+	// WriteError writes an error response (400-599).
 	//
 	// If the ResponseWriter has already been written to, then this method panics.
-	WriteError(code StatusCode) Result
+	WriteError(resp ErrorResponse) Result
 
 	// Redirect responds with a redirect to the given url, using code as the status code.
 	//
diff --git a/safehttp/status.go b/safehttp/status.go
index 80909a1..34c5023 100644
--- a/safehttp/status.go
+++ b/safehttp/status.go
@@ -14,6 +14,8 @@
 
 package safehttp
 
+import "net/http"
+
 // StatusCode contains HTTP status codes as registered with IANA.
 // See: https://www.iana.org/assignments/http-status-codes/http-status-codes.xhtml
 type StatusCode int
@@ -88,3 +90,12 @@ const (
 	StatusNotExtended                   StatusCode = 510 // RFC 2774, 7
 	StatusNetworkAuthenticationRequired StatusCode = 511 // RFC 6585, 6
 )
+
+// Code implements ErrorResponse.
+func (c StatusCode) Code() StatusCode {
+	return c
+}
+
+func (c StatusCode) String() string {
+	return http.StatusText(int(c))
+}


Test patch:
diff --git a/safehttp/flight_test.go b/safehttp/flight_test.go
index ddf33c5..c5fab10 100644
--- a/safehttp/flight_test.go
+++ b/safehttp/flight_test.go
@@ -42,12 +42,6 @@ func (p panickingInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safeht
 	}
 }
 
-func (p panickingInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-	if p.onError {
-		panic("onError")
-	}
-}
-
 func TestFlightInterceptorPanic(t *testing.T) {
 	tests := []struct {
 		desc        string
@@ -64,11 +58,6 @@ func TestFlightInterceptorPanic(t *testing.T) {
 			interceptor: panickingInterceptor{commit: true},
 			wantPanic:   true,
 		},
-		{
-			desc:        "panic in OnError, but handler finishes successfully, so it doesn't happen",
-			interceptor: panickingInterceptor{onError: true},
-			wantPanic:   false,
-		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
diff --git a/safehttp/mux_test.go b/safehttp/mux_test.go
index 4abe298..68dcd0c 100644
--- a/safehttp/mux_test.go
+++ b/safehttp/mux_test.go
@@ -177,9 +177,6 @@ func (p setHeaderInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.Inco
 func (p setHeaderInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
-func (p setHeaderInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-}
-
 type internalErrorInterceptor struct{}
 
 func (internalErrorInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
@@ -187,10 +184,6 @@ func (internalErrorInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.In
 }
 
 func (internalErrorInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-	w.Header().Set("Foo", "this should not be reached")
-}
-
-func (internalErrorInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
 type claimHeaderInterceptor struct {
@@ -208,9 +201,6 @@ func (p *claimHeaderInterceptor) Before(w safehttp.ResponseWriter, r *safehttp.I
 func (p *claimHeaderInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
 }
 
-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) {
 	f := r.Context().Value(claimCtxKey{}).(func([]string))
 	f([]string{value})
@@ -226,9 +216,6 @@ func (committerInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp
 	w.Header().Set("foo", "bar")
 }
 
-func (committerInterceptor) OnError(w safehttp.ResponseHeadersWriter, _ *safehttp.IncomingRequest, _ safehttp.Response, cfg safehttp.InterceptorConfig) {
-}
-
 type setHeaderErroringInterceptor struct{}
 
 func (setHeaderErroringInterceptor) Before(w safehttp.ResponseWriter, _ *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
@@ -239,10 +226,6 @@ func (setHeaderErroringInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *
 	w.Header().Set("name", "foo")
 }
 
-func (setHeaderErroringInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-	w.Header().Set("name", "bar")
-}
-
 func TestMuxInterceptors(t *testing.T) {
 	tests := []struct {
 		name        string
@@ -334,27 +317,6 @@ func TestMuxInterceptors(t *testing.T) {
 			},
 			wantBody: "&lt;h1&gt;Hello World!&lt;/h1&gt;",
 		},
-		{
-			name: "OnError phase sets header",
-			mux: func() *safehttp.ServeMux {
-				mb := &safehttp.ServeMuxConfig{}
-				mb.Intercept(setHeaderErroringInterceptor{})
-
-				registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-					return w.Write(safehtml.HTMLEscaped("<h1>Hello World!</h1>"))
-				})
-				mb.Handle("/bar", safehttp.MethodGet, registeredHandler)
-
-				return mb.Mux()
-			}(),
-			wantStatus: safehttp.StatusForbidden,
-			wantHeaders: map[string][]string{
-				"Content-Type":           {"text/plain; charset=utf-8"},
-				"X-Content-Type-Options": {"nosniff"},
-				"Name":                   {"bar"},
-			},
-			wantBody: "Forbidden\n",
-		},
 	}
 
 	for _, tt := range tests {
@@ -414,9 +376,6 @@ func (p setHeaderConfigInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *
 	w.Header().Set(name, value)
 }
 
-func (p setHeaderConfigInterceptor) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-}
-
 type noInterceptorConfig struct{}
 
 func (noInterceptorConfig) Match(i safehttp.Interceptor) bool {
@@ -502,9 +461,6 @@ func (interceptorOne) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Incom
 	w.Header().Set("Commit1", "a")
 }
 
-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 {
@@ -522,9 +478,6 @@ func (interceptorTwo) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Incom
 	w.Header().Set("Commit2", "b")
 }
 
-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 {
@@ -542,9 +495,6 @@ func (interceptorThree) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inc
 	w.Header().Set("Commit3", "c")
 }
 
-func (interceptorThree) OnError(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
-}
-
 func TestMuxDeterministicInterceptorOrder(t *testing.T) {
 	mb := &safehttp.ServeMuxConfig{}
 	mb.Intercept(interceptorOne{})
diff --git a/tests/integration/errors/errors_test.go b/tests/integration/errors/errors_test.go
new file mode 100644
index 0000000..0bc7121
--- /dev/null
+++ b/tests/integration/errors/errors_test.go
@@ -0,0 +1,153 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package errors_test
+
+import (
+	"fmt"
+	"math"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/safehtml"
+
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/safehttptest"
+	"github.com/google/safehtml/template"
+)
+
+var myErrorTmpl = template.Must(template.New("not found").Parse(`<h1>Error: {{ .Code }}</h1>
+<p>{{ .Message }}</p>
+`))
+
+type myError struct {
+	safehttp.StatusCode
+	Message string
+}
+
+type myDispatcher struct {
+	safehttp.DefaultDispatcher
+}
+
+func (m myDispatcher) Error(rw http.ResponseWriter, resp safehttp.ErrorResponse) error {
+	if x, ok := resp.(myError); ok {
+		return myErrorTmpl.Execute(rw, x)
+	}
+	return m.DefaultDispatcher.Error(rw, resp)
+}
+
+// TestCustomErrors tests a scenario where custom errors are implemented using a
+// custom dispatcher implementation. XSRF interceptor is added to check that
+// error responses go through the commit phase.
+func TestCustomErrors(t *testing.T) {
+	mb := &safehttp.ServeMuxConfig{Dispatcher: myDispatcher{}}
+
+	mb.Handle("/compute", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+		qs, err := r.URL.Query()
+		if err != nil {
+			return w.WriteError(safehttp.StatusBadRequest)
+		}
+		a := qs.Int64("a", math.MaxInt64)
+		if qs.Err() != nil || a == math.MaxInt64 {
+			return w.WriteError(myError{StatusCode: safehttp.StatusBadRequest, Message: "missing parameter 'a'"})
+		}
+		if a > 10 {
+			return w.WriteError(myError{StatusCode: safehttp.StatusNotImplemented, Message: "we can't process queries with large numbers yet"})
+		}
+		return w.Write(safehtml.HTMLEscaped(fmt.Sprintf("Result: %d", a*a)))
+	}))
+
+	m := mb.Mux()
+
+	t.Run("correct request", func(t *testing.T) {
+		b := strings.Builder{}
+		rr := safehttptest.NewTestResponseWriter(&b)
+
+		req := httptest.NewRequest(safehttp.MethodGet, "https://foo.com/compute?a=3", nil)
+		m.ServeHTTP(rr, req)
+
+		if got, want := rr.Status(), safehttp.StatusOK; got != want {
+			t.Errorf("rr.Status() got: %v want: %v", got, want)
+		}
+		want := "Result: 9"
+		if diff := cmp.Diff(want, b.String()); diff != "" {
+			t.Errorf("response body diff (-want,+got): \n%s\ngot %q, want %q", diff, b.String(), want)
+		}
+	})
+
+	t.Run("missing parameter", func(t *testing.T) {
+		b := strings.Builder{}
+		rr := safehttptest.NewTestResponseWriter(&b)
+
+		req := httptest.NewRequest(safehttp.MethodGet, "https://foo.com/compute?foo=3", nil)
+		m.ServeHTTP(rr, req)
+
+		if got, want := rr.Status(), safehttp.StatusBadRequest; got != want {
+			t.Errorf("rr.Status() got: %v want: %v", got, want)
+		}
+
+		want := `<h1>Error: Bad Request</h1>
+<p>missing parameter &#39;a&#39;</p>
+`
+		if diff := cmp.Diff(want, b.String()); diff != "" {
+			t.Errorf("response body diff (-want,+got): \n%s\ngot %q, want %q", diff, b.String(), want)
+		}
+	})
+
+}
+
+type interceptor struct {
+	errBefore bool
+}
+
+func (it interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, _ safehttp.InterceptorConfig) safehttp.Result {
+	if it.errBefore {
+		return w.WriteError(myError{StatusCode: safehttp.StatusForbidden, Message: "forbidden in Before"})
+	}
+	return safehttp.NotWritten()
+}
+
+func (it interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
+}
+
+func TestCustomErrorsInBefore(t *testing.T) {
+	mb := &safehttp.ServeMuxConfig{Dispatcher: myDispatcher{}}
+	mb.Intercept(interceptor{errBefore: true})
+
+	mb.Handle("/compute", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+		return w.Write(safehtml.HTMLEscaped("the handler code doesn't matter in this test case"))
+	}))
+
+	m := mb.Mux()
+	t.Run("error in Before", func(t *testing.T) {
+		b := strings.Builder{}
+		rr := safehttptest.NewTestResponseWriter(&b)
+
+		req := httptest.NewRequest(safehttp.MethodGet, "https://foo.com/compute?a=3", nil)
+		m.ServeHTTP(rr, req)
+
+		if got, want := rr.Status(), safehttp.StatusForbidden; got != want {
+			t.Errorf("rr.Status() got: %v want: %v", got, want)
+		}
+		want := `<h1>Error: Forbidden</h1>
+<p>forbidden in Before</p>
+`
+		if diff := cmp.Diff(want, b.String()); diff != "" {
+			t.Errorf("response body diff (-want,+got): \n%s\ngot %q, want %q", diff, b.String(), want)
+		}
+	})
+}