Repository: google/go-safeweb
Pull request #92 (2020-08-17 11:36:39)

Use the Interceptor name instead of Plugin in the HSTS plugin


Patch:
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
index ef979bb..91b9008 100644
--- a/safehttp/plugins/hsts/hsts.go
+++ b/safehttp/plugins/hsts/hsts.go
@@ -23,9 +23,9 @@ import (
 	"github.com/google/go-safeweb/safehttp"
 )
 
-// Plugin implements automatic HSTS functionality.
+// Interceptor implements automatic HSTS functionality.
 // See https://tools.ietf.org/html/rfc6797 for more info.
-type Plugin struct {
+type Interceptor struct {
 	// MaxAge is the duration that the browser should remember
 	// that a site is only to be accessed using HTTPS. MaxAge
 	// must be positive. It will be rounded to seconds before use.
@@ -52,25 +52,25 @@ type Plugin struct {
 	BehindProxy bool
 }
 
-// NewPlugin creates a new HSTS plugin with safe defaults.
+// Default creates a new HSTS interceptor with safe defaults.
 // These safe defaults are:
 //  - max-age set to two years.
 //  - includeSubDomains is enabled.
 //  - preload is disabled.
-func NewPlugin() Plugin {
-	return Plugin{MaxAge: 63072000 * time.Second} // two years in seconds
+func Default() Interceptor {
+	return Interceptor{MaxAge: 63072000 * time.Second} // two years in seconds
 }
 
 // Before should be executed before the request is sent to the handler.
 // The function redirects HTTP requests to HTTPS. When HTTPS traffic
 // is received the Strict-Transport-Security header is applied to the
 // response.
-func (p Plugin) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-	if p.MaxAge < 0 {
+func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+	if it.MaxAge < 0 {
 		return w.ServerError(safehttp.StatusInternalServerError)
 	}
 
-	if !p.BehindProxy && r.TLS == nil {
+	if !it.BehindProxy && r.TLS == nil {
 		u, err := url.Parse(r.URL.String())
 		if err != nil {
 			return w.ServerError(safehttp.StatusInternalServerError)
@@ -81,19 +81,17 @@ func (p Plugin) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) s
 
 	var value strings.Builder
 	value.WriteString("max-age=")
-	value.WriteString(strconv.FormatInt(int64(p.MaxAge.Seconds()), 10))
-	if !p.DisableIncludeSubDomains {
+	value.WriteString(strconv.FormatInt(int64(it.MaxAge.Seconds()), 10))
+	if !it.DisableIncludeSubDomains {
 		value.WriteString("; includeSubDomains")
 	}
-	if p.Preload {
+	if it.Preload {
 		value.WriteString("; preload")
 	}
-	h := w.Header()
-	if err := h.Set("Strict-Transport-Security", value.String()); err != nil {
-		return w.ServerError(safehttp.StatusInternalServerError)
-	}
-	if _, err := h.Claim("Strict-Transport-Security"); err != nil {
+	set, err := w.Header().Claim("Strict-Transport-Security")
+	if err != nil {
 		return w.ServerError(safehttp.StatusInternalServerError)
 	}
+	set([]string{value.String()})
 	return safehttp.Result{}
 }
diff --git a/safehttp/response_writer.go b/safehttp/response_writer.go
index 48f42f4..aec52aa 100644
--- a/safehttp/response_writer.go
+++ b/safehttp/response_writer.go
@@ -23,9 +23,8 @@ type ResponseWriter struct {
 	d  Dispatcher
 	rw http.ResponseWriter
 
-	// Having this field unexported is essential for
-	// security. Otherwise one can easily overwrite
-	// the struct bypassing all our safety guarantees.
+	// Having this field unexported is essential for security. Otherwise one can
+	// easily overwrite the struct bypassing all our safety guarantees.
 	header       Header
 	muxInterceps map[string]Interceptor
 }


Test patch:
diff --git a/safehttp/plugins/hsts/hsts_test.go b/safehttp/plugins/hsts/hsts_test.go
index 0c62afd..f120acc 100644
--- a/safehttp/plugins/hsts/hsts_test.go
+++ b/safehttp/plugins/hsts/hsts_test.go
@@ -27,27 +27,27 @@ import (
 func TestHSTS(t *testing.T) {
 	var test = []struct {
 		name        string
-		plugin      hsts.Plugin
+		interceptor hsts.Interceptor
 		req         *safehttp.IncomingRequest
 		wantStatus  int
 		wantHeaders map[string][]string
 		wantBody    string
 	}{
 		{
-			name:       "HTTPS",
-			plugin:     hsts.NewPlugin(),
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 200,
+			name:        "HTTPS",
+			interceptor: hsts.Default(),
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				"Strict-Transport-Security": {"max-age=63072000; includeSubDomains"}, // 63072000 seconds is two years
 			},
 			wantBody: "",
 		},
 		{
-			name:       "HTTP",
-			plugin:     hsts.NewPlugin(),
-			req:        safehttptest.NewRequest("GET", "http://localhost/", nil),
-			wantStatus: 301,
+			name:        "HTTP",
+			interceptor: hsts.Default(),
+			req:         safehttptest.NewRequest("GET", "http://localhost/", nil),
+			wantStatus:  301,
 			wantHeaders: map[string][]string{
 				"Content-Type": {"text/html; charset=utf-8"},
 				"Location":     {"https://localhost/"},
@@ -55,10 +55,10 @@ func TestHSTS(t *testing.T) {
 			wantBody: "<a href=\"https://localhost/\">Moved Permanently</a>.\n\n",
 		},
 		{
-			name:       "HTTP behind proxy",
-			plugin:     hsts.Plugin{BehindProxy: true},
-			req:        safehttptest.NewRequest("GET", "http://localhost/", nil),
-			wantStatus: 200,
+			name:        "HTTP behind proxy",
+			interceptor: hsts.Interceptor{BehindProxy: true},
+			req:         safehttptest.NewRequest("GET", "http://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				// max-age=0 tells the browser to expire the HSTS protection.
 				"Strict-Transport-Security": {"max-age=0; includeSubDomains"},
@@ -66,10 +66,10 @@ func TestHSTS(t *testing.T) {
 			wantBody: "",
 		},
 		{
-			name:       "Preload",
-			plugin:     hsts.Plugin{Preload: true, DisableIncludeSubDomains: true},
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 200,
+			name:        "Preload",
+			interceptor: hsts.Interceptor{Preload: true, DisableIncludeSubDomains: true},
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				// max-age=0 tells the browser to expire the HSTS protection.
 				"Strict-Transport-Security": {"max-age=0; preload"},
@@ -77,10 +77,10 @@ func TestHSTS(t *testing.T) {
 			wantBody: "",
 		},
 		{
-			name:       "Preload and IncludeSubDomains",
-			plugin:     hsts.Plugin{Preload: true},
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 200,
+			name:        "Preload and IncludeSubDomains",
+			interceptor: hsts.Interceptor{Preload: true},
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				// max-age=0 tells the browser to expire the HSTS protection.
 				"Strict-Transport-Security": {"max-age=0; includeSubDomains; preload"},
@@ -88,10 +88,10 @@ func TestHSTS(t *testing.T) {
 			wantBody: "",
 		},
 		{
-			name:       "No preload and no includeSubDomains",
-			plugin:     hsts.Plugin{DisableIncludeSubDomains: true},
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 200,
+			name:        "No preload and no includeSubDomains",
+			interceptor: hsts.Interceptor{DisableIncludeSubDomains: true},
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				// max-age=0 tells the browser to expire the HSTS protection.
 				"Strict-Transport-Security": {"max-age=0"},
@@ -99,20 +99,20 @@ func TestHSTS(t *testing.T) {
 			wantBody: "",
 		},
 		{
-			name:       "Custom maxage",
-			plugin:     hsts.Plugin{MaxAge: 3600 * time.Second},
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 200,
+			name:        "Custom maxage",
+			interceptor: hsts.Interceptor{MaxAge: 3600 * time.Second},
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  200,
 			wantHeaders: map[string][]string{
 				"Strict-Transport-Security": {"max-age=3600; includeSubDomains"}, // 3600 seconds is 1 hour
 			},
 			wantBody: "",
 		},
 		{
-			name:       "Negative maxage",
-			plugin:     hsts.Plugin{MaxAge: -1 * time.Second},
-			req:        safehttptest.NewRequest("GET", "https://localhost/", nil),
-			wantStatus: 500,
+			name:        "Negative maxage",
+			interceptor: hsts.Interceptor{MaxAge: -1 * time.Second},
+			req:         safehttptest.NewRequest("GET", "https://localhost/", nil),
+			wantStatus:  500,
 			wantHeaders: map[string][]string{
 				"Content-Type":           {"text/plain; charset=utf-8"},
 				"X-Content-Type-Options": {"nosniff"},
@@ -125,7 +125,7 @@ func TestHSTS(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			rr := safehttptest.NewResponseRecorder()
 
-			tt.plugin.Before(rr.ResponseWriter, tt.req)
+			tt.interceptor.Before(rr.ResponseWriter, tt.req)
 
 			if rr.Status() != tt.wantStatus {
 				t.Errorf("status code got: %v want: %v", rr.Status(), tt.wantStatus)
@@ -143,7 +143,7 @@ func TestHSTS(t *testing.T) {
 }
 
 func TestStrictTransportSecurityAlreadyImmutable(t *testing.T) {
-	p := hsts.NewPlugin()
+	p := hsts.Default()
 
 	req := safehttptest.NewRequest("GET", "https://localhost/", nil)
 
diff --git a/tests/integration/hsts/hsts_test.go b/tests/integration/hsts/hsts_test.go
index 1fa51df..b358f89 100644
--- a/tests/integration/hsts/hsts_test.go
+++ b/tests/integration/hsts/hsts_test.go
@@ -73,7 +73,7 @@ func TestHSTSServeMuxInstall(t *testing.T) {
 	})
 	mux.Handle("/asdf", safehttp.MethodGet, handler)
 
-	mux.Install("hsts", hsts.NewPlugin())
+	mux.Install("hsts", hsts.Default())
 
 	b := strings.Builder{}
 	rr := newResponseRecorder(&b)