Repository: google/go-safeweb (go)

Issue:
Make sure that no two given configs apply to the same interceptor
This is a TODO expressed here:
https://github.com/google/go-safeweb/blob/d29986caf7ba2bd243f3ca698413cff9fa91bc92/safehttp/mux.go#L225-L233

We should either panic there, or require configs to be somehow mergeable.

Pull request:
Add support for wrapped interceptor, detect config clashes.
Fixes #302, #307

Patch:
diff --git a/safehttp/interceptor.go b/safehttp/interceptor.go
index 1ad71e2..2116853 100644
--- a/safehttp/interceptor.go
+++ b/safehttp/interceptor.go
@@ -32,10 +32,21 @@ type Interceptor interface {
 	Commit(w ResponseHeadersWriter, r *IncomingRequest, resp Response, cfg InterceptorConfig)
 }
 
+// WrappedInterceptor is a wrapped interceptor.
+// This can be used to decorate an already existing interceptor and still allow
+// its native configurations to match with it.
+type WrappedInterceptor interface {
+	Interceptor
+	// Unwrap returns the wrapped interceptor.
+	Unwrap() Interceptor
+}
+
 // InterceptorConfig is a configuration of an interceptor.
 type InterceptorConfig interface {
 	// Match checks whether this InterceptorConfig is meant to be applied to the
 	// given Interceptor.
+	// If the Interceptor implements WrappedInterceptor the framework will take care of calling
+	// Match for all the nested interceptors.
 	Match(Interceptor) bool
 }
 
diff --git a/safehttp/mux.go b/safehttp/mux.go
index 1fde0fd..a5a31ad 100644
--- a/safehttp/mux.go
+++ b/safehttp/mux.go
@@ -147,8 +147,7 @@ type handlerRegistration struct {
 // InterceptorConfigs can be passed in order to modify the behavior of the
 // interceptors on a registered handler. Passing an InterceptorConfig whose
 // corresponding Interceptor was not installed will produce no effect. If
-// multiple configurations are passed for the same Interceptor, only the first
-// one will take effect.
+// multiple configurations are passed for the same Interceptor, Mux will panic.
 func (s *ServeMuxConfig) Handle(pattern string, method string, h Handler, cfgs ...InterceptorConfig) {
 	s.handlers = append(s.handlers, handlerRegistration{
 		pattern: pattern,
@@ -222,14 +221,32 @@ func (s *ServeMuxConfig) Mux() *ServeMux {
 func configureInterceptors(interceptors []Interceptor, cfgs []InterceptorConfig) []configuredInterceptor {
 	var its []configuredInterceptor
 	for _, it := range interceptors {
-		var cfg InterceptorConfig
+		var matches []InterceptorConfig
 		for _, c := range cfgs {
 			if c.Match(it) {
-				// TODO: there should be a validation check that there is at
-				// most one config per interceptor.
-				cfg = c
-				break
+				matches = append(matches, c)
+			}
+			wrapped, ok := it.(WrappedInterceptor)
+			for ok {
+				inner := wrapped.Unwrap()
+				if c.Match(inner) {
+					matches = append(matches, c)
+				}
+				wrapped, ok = inner.(WrappedInterceptor)
+			}
+		}
+
+		if len(matches) > 1 {
+			msg := fmt.Sprintf("multiple configurations specified for interceptor %T: ", it)
+			for _, match := range matches {
+				msg += fmt.Sprintf("%#v", match)
 			}
+			panic(msg)
+		}
+
+		var cfg InterceptorConfig
+		if len(matches) == 1 {
+			cfg = matches[0]
 		}
 		its = append(its, configuredInterceptor{interceptor: it, config: cfg})
 	}


Test patch:
diff --git a/safehttp/mux_test.go b/safehttp/mux_test.go
index 2a14dba..db725da 100644
--- a/safehttp/mux_test.go
+++ b/safehttp/mux_test.go
@@ -413,6 +413,22 @@ func (p setHeaderConfigInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *
 
 type noInterceptorConfig struct{}
 
+type wrappedInterceptor struct {
+	w safehttp.Interceptor
+}
+
+func (wi wrappedInterceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
+	return wi.w.Before(w, r, cfg)
+}
+
+func (wi wrappedInterceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) {
+	wi.w.Commit(w, r, resp, cfg)
+}
+
+func (wi wrappedInterceptor) Unwrap() safehttp.Interceptor {
+	return wi.w
+}
+
 func (noInterceptorConfig) Match(i safehttp.Interceptor) bool {
 	return false
 }
@@ -420,15 +436,17 @@ func (noInterceptorConfig) Match(i safehttp.Interceptor) bool {
 func TestMuxInterceptorConfigs(t *testing.T) {
 	tests := []struct {
 		name        string
+		interceptor safehttp.Interceptor
 		config      safehttp.InterceptorConfig
 		wantStatus  safehttp.StatusCode
 		wantHeaders map[string][]string
 		wantBody    string
 	}{
 		{
-			name:       "SetHeaderInterceptor with config",
-			config:     setHeaderConfig{name: "Foo", value: "Bar"},
-			wantStatus: safehttp.StatusOK,
+			name:        "SetHeaderInterceptor with config",
+			interceptor: setHeaderConfigInterceptor{},
+			config:      setHeaderConfig{name: "Foo", value: "Bar"},
+			wantStatus:  safehttp.StatusOK,
 			wantHeaders: map[string][]string{
 				"Content-Type": {"text/html; charset=utf-8"},
 				"Commit-Foo":   {"Bar"},
@@ -437,9 +455,22 @@ func TestMuxInterceptorConfigs(t *testing.T) {
 			wantBody: "&lt;h1&gt;Hello World!&lt;/h1&gt;",
 		},
 		{
-			name:       "SetHeaderInterceptor with mismatching config",
-			config:     noInterceptorConfig{},
-			wantStatus: safehttp.StatusOK,
+			name:        "Wrapped SetHeaderInterceptor with config",
+			interceptor: wrappedInterceptor{w: setHeaderConfigInterceptor{}},
+			config:      setHeaderConfig{name: "Foo", value: "Bar"},
+			wantStatus:  safehttp.StatusOK,
+			wantHeaders: map[string][]string{
+				"Content-Type": {"text/html; charset=utf-8"},
+				"Commit-Foo":   {"Bar"},
+				"Foo":          {"Bar"},
+			},
+			wantBody: "&lt;h1&gt;Hello World!&lt;/h1&gt;",
+		},
+		{
+			name:        "SetHeaderInterceptor with mismatching config",
+			interceptor: setHeaderConfigInterceptor{},
+			config:      noInterceptorConfig{},
+			wantStatus:  safehttp.StatusOK,
 			wantHeaders: map[string][]string{
 				"Content-Type": {"text/html; charset=utf-8"},
 				"Pizza":        {"Hawaii"},
@@ -452,7 +483,7 @@ func TestMuxInterceptorConfigs(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mb := safehttp.NewServeMuxConfig(nil)
-			mb.Intercept(setHeaderConfigInterceptor{})
+			mb.Intercept(tt.interceptor)
 
 			registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 				return w.Write(safehtml.HTMLEscaped("<h1>Hello World!</h1>"))