Repository: google/go-safeweb (go)

Issue:
Hide the Dispatcher from ServeMuxConfig
Dispatcher should be passed on construction and be immutable (to prevent users from being able to change it).

Pull request description:
ServeMuxConfig needs to be created using NewServeMuxConfig from now on
Linters like https://github.com/bazelbuild/rules_go/blob/master/go/nogo.rst can ban function calls, but not necessarily composite literals / zero value variables of a given type.

In order to make `ServeMuxConfig` creation restricted, it's easier to just hide it behind a function call.

Other benefits:
- prevents changing the `Dispatcher` at runtime


Fixes #269.

Patch:
diff --git a/safehttp/defaults/defaults.go b/safehttp/defaults/defaults.go
index e9f03f5..703bb0d 100644
--- a/safehttp/defaults/defaults.go
+++ b/safehttp/defaults/defaults.go
@@ -41,7 +41,7 @@ func ServeMuxConfig(hosts []string, xsrfKey string) (*safehttp.ServeMuxConfig, e
 		return nil, errors.New("xsrfKey cannot be empty")
 	}
 
-	var c safehttp.ServeMuxConfig
+	c := safehttp.NewServeMuxConfig(nil)
 	// TODO(clap): add a report group once we support reporting.
 	c.Intercept(coop.Default(""))
 	// TODO(clap): add a report-uri once we support reporting.
@@ -52,5 +52,5 @@ func ServeMuxConfig(hosts []string, xsrfKey string) (*safehttp.ServeMuxConfig, e
 	c.Intercept(hsts.Default())
 	c.Intercept(staticheaders.Interceptor{})
 	c.Intercept(&xsrfhtml.Interceptor{SecretAppKey: xsrfKey})
-	return &c, nil
+	return c, nil
 }
diff --git a/safehttp/mux.go b/safehttp/mux.go
index 6be5a07..39bb65e 100644
--- a/safehttp/mux.go
+++ b/safehttp/mux.go
@@ -111,11 +111,20 @@ 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
 }
 
+// NewServeMuxConfig crates a ServeMuxConfig with the provided Dispatcher. If
+// the provided Dispatcher is nil, the DefaultDispatcher is used.
+func NewServeMuxConfig(disp Dispatcher) *ServeMuxConfig {
+	if disp == nil {
+		disp = &DefaultDispatcher{}
+	}
+	return &ServeMuxConfig{dispatcher: disp}
+}
+
 type handlerRegistration struct {
 	pattern string
 	method  string
@@ -154,9 +163,8 @@ func (s *ServeMuxConfig) Mux() *ServeMux {
 		log.Println("Warning: creating safehttp.Mux in dev mode. This configuration is not valid for production use")
 	}
 
-	dispatcher := s.Dispatcher
-	if dispatcher == nil {
-		dispatcher = DefaultDispatcher{}
+	if s.dispatcher == nil {
+		panic("Use NewServeMuxConfig instead of creating ServeMuxConfig using a composite literal.")
 	}
 	// pattern -> method -> handlerConfig
 	handlers := map[string]map[string]handlerConfig{}
@@ -170,7 +178,7 @@ func (s *ServeMuxConfig) Mux() *ServeMux {
 		}
 		handlers[hr.pattern][hr.method] =
 			handlerConfig{
-				Dispatcher:   dispatcher,
+				Dispatcher:   s.dispatcher,
 				Handler:      hr.handler,
 				Interceptors: configureInterceptors(s.interceptors, hr.cfgs),
 			}
@@ -202,7 +210,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)),
 	}


Test patch:
diff --git a/safehttp/fileserver_test.go b/safehttp/fileserver_test.go
index df7cd05..de19439 100644
--- a/safehttp/fileserver_test.go
+++ b/safehttp/fileserver_test.go
@@ -58,7 +58,7 @@ func TestFileServer(t *testing.T) {
 		},
 	}
 
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/", safehttp.MethodGet, safehttp.FileServer(tmpDir))
 	m := mb.Mux()
 
diff --git a/safehttp/flight_test.go b/safehttp/flight_test.go
index ed0a8c4..4a85b0e 100644
--- a/safehttp/flight_test.go
+++ b/safehttp/flight_test.go
@@ -59,7 +59,7 @@ func TestFlightInterceptorPanic(t *testing.T) {
 	}
 	for _, tc := range tests {
 		t.Run(tc.desc, func(t *testing.T) {
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 			mb.Intercept(tc.interceptor)
 			mb.Handle("/search", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 				// IMPORTANT: We are setting the header here and expecting to be
@@ -94,7 +94,7 @@ func TestFlightInterceptorPanic(t *testing.T) {
 }
 
 func TestFlightHandlerPanic(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/search", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 		// IMPORTANT: We are setting the header here and expecting to be
 		// cleared if a panic occurs.
@@ -136,7 +136,7 @@ func TestFlightDoubleWritePanics(t *testing.T) {
 	for firstWriteName, firstWrite := range writeFuncs {
 		for secondWriteName, secondWrite := range writeFuncs {
 			t.Run(fmt.Sprintf("%s->%s", firstWriteName, secondWriteName), func(t *testing.T) {
-				mb := &safehttp.ServeMuxConfig{}
+				mb := safehttp.NewServeMuxConfig(nil)
 				mb.Handle("/search", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 					firstWrite(w, r)
 					secondWrite(w, r) // this should panic
diff --git a/safehttp/mux_test.go b/safehttp/mux_test.go
index d4d0f28..c29037b 100644
--- a/safehttp/mux_test.go
+++ b/safehttp/mux_test.go
@@ -56,7 +56,7 @@ func TestMuxOneHandlerOneRequest(t *testing.T) {
 
 	for _, tt := range test {
 		t.Run(tt.name, func(t *testing.T) {
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 
 			h := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 				return w.Write(safehtml.HTMLEscaped("<h1>Hello World!</h1>"))
@@ -118,7 +118,7 @@ func TestMuxServeTwoHandlers(t *testing.T) {
 		},
 	}
 
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/bar", safehttp.MethodGet, tests[0].hf)
 	mb.Handle("/bar", safehttp.MethodPost, tests[1].hf)
 	mux := mb.Mux()
@@ -165,7 +165,7 @@ func TestMuxRegisterCorrectHandlerAllPaths(t *testing.T) {
 		},
 	}
 
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/get", safehttp.MethodGet, tests[0].hf)
 	mb.Handle("/get2", safehttp.MethodGet, tests[1].hf)
 	mux := mb.Mux()
@@ -181,7 +181,7 @@ func TestMuxRegisterCorrectHandlerAllPaths(t *testing.T) {
 }
 
 func TestMuxHandleSameMethodTwice(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 
 	registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 		return w.Write(safehtml.HTMLEscaped("<h1>Hello World!</h1>"))
@@ -272,7 +272,7 @@ func TestMuxInterceptors(t *testing.T) {
 		{
 			name: "Install ServeMux Interceptor before handler registration",
 			mux: func() *safehttp.ServeMux {
-				mb := &safehttp.ServeMuxConfig{}
+				mb := safehttp.NewServeMuxConfig(nil)
 				mb.Intercept(setHeaderInterceptor{
 					name:  "Foo",
 					value: "bar",
@@ -294,7 +294,7 @@ func TestMuxInterceptors(t *testing.T) {
 		{
 			name: "Install Interrupting Interceptor",
 			mux: func() *safehttp.ServeMux {
-				mb := &safehttp.ServeMuxConfig{}
+				mb := safehttp.NewServeMuxConfig(nil)
 				mb.Intercept(internalErrorInterceptor{})
 
 				registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -314,7 +314,7 @@ func TestMuxInterceptors(t *testing.T) {
 		{
 			name: "Handler Communication With ServeMux Interceptor",
 			mux: func() *safehttp.ServeMux {
-				mb := &safehttp.ServeMuxConfig{}
+				mb := safehttp.NewServeMuxConfig(nil)
 				mb.Intercept(&claimHeaderInterceptor{headerToClaim: "Foo"})
 
 				registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -335,7 +335,7 @@ func TestMuxInterceptors(t *testing.T) {
 		{
 			name: "Commit phase sets header",
 			mux: func() *safehttp.ServeMux {
-				mb := &safehttp.ServeMuxConfig{}
+				mb := safehttp.NewServeMuxConfig(nil)
 				mb.Intercept(committerInterceptor{})
 
 				registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -450,7 +450,7 @@ func TestMuxInterceptorConfigs(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 			mb.Intercept(setHeaderConfigInterceptor{})
 
 			registeredHandler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -529,7 +529,7 @@ func (interceptorThree) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inc
 }
 
 func TestMuxDeterministicInterceptorOrder(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Intercept(interceptorOne{})
 	mb.Intercept(interceptorTwo{})
 	mb.Intercept(interceptorThree{})
@@ -567,7 +567,7 @@ func TestMuxDeterministicInterceptorOrder(t *testing.T) {
 }
 
 func TestMuxHandlerReturnsNotWritten(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	h := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 		return safehttp.NotWritten()
 	})
diff --git a/safehttp/plugins/hostcheck/hostcheck_test.go b/safehttp/plugins/hostcheck/hostcheck_test.go
index 5aec398..dcc753f 100644
--- a/safehttp/plugins/hostcheck/hostcheck_test.go
+++ b/safehttp/plugins/hostcheck/hostcheck_test.go
@@ -44,7 +44,7 @@ func TestInterceptor(t *testing.T) {
 
 	for _, tt := range test {
 		t.Run(tt.name, func(t *testing.T) {
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 			mb.Intercept(hostcheck.New("foo.com"))
 
 			h := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
diff --git a/safehttp/server_test.go b/safehttp/server_test.go
index 748f4ea..be6d9d7 100644
--- a/safehttp/server_test.go
+++ b/safehttp/server_test.go
@@ -30,7 +30,7 @@ func TestServer(t *testing.T) {
 	l := requesttesting.NewFakeListener()
 	defer l.Close()
 
-	var cfg ServeMuxConfig
+	cfg := NewServeMuxConfig(nil)
 	cfg.Handle("/", "GET", HandlerFunc(func(w ResponseWriter, r *IncomingRequest) Result {
 		return w.Write(safehtml.HTMLEscaped("response"))
 	}))
diff --git a/tests/integration/csp/csp_test.go b/tests/integration/csp/csp_test.go
index a447c9c..a7f6987 100644
--- a/tests/integration/csp/csp_test.go
+++ b/tests/integration/csp/csp_test.go
@@ -25,7 +25,7 @@ import (
 )
 
 func TestServeMuxInstallCSP(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	it := csp.Default("")
 	mb.Intercept(&it)
 
diff --git a/tests/integration/errors/errors_test.go b/tests/integration/errors/errors_test.go
index 3b23583..8797182 100644
--- a/tests/integration/errors/errors_test.go
+++ b/tests/integration/errors/errors_test.go
@@ -52,7 +52,7 @@ func (m myDispatcher) Error(rw http.ResponseWriter, resp safehttp.ErrorResponse)
 // 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 := safehttp.NewServeMuxConfig(myDispatcher{})
 
 	mb.Handle("/compute", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
 		qs, err := r.URL.Query()
@@ -121,7 +121,7 @@ func (it interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Incom
 }
 
 func TestCustomErrorsInBefore(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{Dispatcher: myDispatcher{}}
+	mb := safehttp.NewServeMuxConfig(myDispatcher{})
 	mb.Intercept(interceptor{errBefore: true})
 
 	mb.Handle("/compute", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
diff --git a/tests/integration/header/header_test.go b/tests/integration/header/header_test.go
index 466dbe6..445e4a3 100644
--- a/tests/integration/header/header_test.go
+++ b/tests/integration/header/header_test.go
@@ -27,7 +27,7 @@ import (
 )
 
 func TestAccessIncomingHeaders(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/", safehttp.MethodGet, safehttp.HandlerFunc(func(rw safehttp.ResponseWriter, ir *safehttp.IncomingRequest) safehttp.Result {
 		if got, want := ir.Header.Get("A"), "B"; got != want {
 			t.Errorf(`ir.Header.Get("A") got: %v want: %v`, got, want)
@@ -49,7 +49,7 @@ func TestAccessIncomingHeaders(t *testing.T) {
 }
 
 func TestChangingResponseHeaders(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/", safehttp.MethodGet, safehttp.HandlerFunc(func(rw safehttp.ResponseWriter, ir *safehttp.IncomingRequest) safehttp.Result {
 		rw.Header().Set("pIZZA", "Pasta")
 		return rw.Write(safehtml.HTMLEscaped("hello"))
diff --git a/tests/integration/hsts/hsts_test.go b/tests/integration/hsts/hsts_test.go
index 2697726..144e26e 100644
--- a/tests/integration/hsts/hsts_test.go
+++ b/tests/integration/hsts/hsts_test.go
@@ -26,7 +26,7 @@ import (
 )
 
 func TestHSTSServeMuxInstall(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 
 	mb.Intercept(hsts.Default())
 	handler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -58,7 +58,7 @@ func TestHSTSServeMuxInstall(t *testing.T) {
 }
 
 func TestHSTSOnErrors(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 
 	mb.Intercept(hsts.Default())
 	handler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
diff --git a/tests/integration/mux/mux_test.go b/tests/integration/mux/mux_test.go
index 338760e..e968a95 100644
--- a/tests/integration/mux/mux_test.go
+++ b/tests/integration/mux/mux_test.go
@@ -71,7 +71,7 @@ func TestMuxDefaultDispatcher(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 			mb.Handle("/pizza", safehttp.MethodGet, tt.handler)
 
 			rw := httptest.NewRecorder()
@@ -127,7 +127,7 @@ func TestMuxDefaultDispatcherUnsafeResponses(t *testing.T) {
 			// error-handling in the ResponseWriter has been fixed.
 			t.Skip()
 
-			mb := &safehttp.ServeMuxConfig{}
+			mb := safehttp.NewServeMuxConfig(nil)
 			mb.Handle("/pizza", safehttp.MethodGet, tt.handler)
 			mux := mb.Mux()
 
diff --git a/tests/integration/safehtml/safehtml_test.go b/tests/integration/safehtml/safehtml_test.go
index c46f71a..4a3fcca 100644
--- a/tests/integration/safehtml/safehtml_test.go
+++ b/tests/integration/safehtml/safehtml_test.go
@@ -24,7 +24,7 @@ import (
 )
 
 func TestHandleRequestWrite(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/", safehttp.MethodGet, safehttp.HandlerFunc(func(rw safehttp.ResponseWriter, ir *safehttp.IncomingRequest) safehttp.Result {
 		return rw.Write(safehtml.HTMLEscaped("<h1>Escaped, so not really a heading</h1>"))
 	}))
@@ -43,7 +43,7 @@ func TestHandleRequestWrite(t *testing.T) {
 }
 
 func TestHandleRequestWriteTemplate(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	mb.Handle("/", safehttp.MethodGet, safehttp.HandlerFunc(func(rw safehttp.ResponseWriter, ir *safehttp.IncomingRequest) safehttp.Result {
 		return safehttp.ExecuteTemplate(rw, template.Must(template.New("name").Parse("<h1>{{ . }}</h1>")), "This is an actual heading, though.")
 	}))
diff --git a/tests/integration/staticheaders/staticheaders_test.go b/tests/integration/staticheaders/staticheaders_test.go
index b730e9f..6e5fbd4 100644
--- a/tests/integration/staticheaders/staticheaders_test.go
+++ b/tests/integration/staticheaders/staticheaders_test.go
@@ -26,7 +26,7 @@ import (
 )
 
 func TestServeMuxInstallStaticHeaders(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 
 	mb.Intercept(staticheaders.Interceptor{})
 	handler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
@@ -59,7 +59,7 @@ func TestServeMuxInstallStaticHeaders(t *testing.T) {
 }
 
 func TestStaticHeadersOnError(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 
 	mb.Intercept(staticheaders.Interceptor{})
 	handler := safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
diff --git a/tests/integration/xsrf/xsrf_test.go b/tests/integration/xsrf/xsrf_test.go
index 7617b29..02177c5 100644
--- a/tests/integration/xsrf/xsrf_test.go
+++ b/tests/integration/xsrf/xsrf_test.go
@@ -25,7 +25,7 @@ import (
 )
 
 func TestServeMuxInstallXSRF(t *testing.T) {
-	mb := &safehttp.ServeMuxConfig{}
+	mb := safehttp.NewServeMuxConfig(nil)
 	it := xsrfhtml.Interceptor{SecretAppKey: "testSecretAppKey"}
 	mb.Intercept(&it)