coop: rename some unclear variables, add some docs.

safehttp/plugins/coop/coop.go

@@ -41,6 +41,7 @@ type Policy struct {
 	ReportOnly bool
 }
 
+// String serializes the policy. The returned value can be used as a header value.
 func (p Policy) String() string {
 	if p.ReportingGroup == "" {
 		return string(p.Mode)
@@ -50,16 +51,16 @@ func (p Policy) String() string {
 
 // NewInterceptor constructs an interceptor that applies the given policies.
 func NewInterceptor(policies ...Policy) Interceptor {
-	var ro []string
+	var rep []string
 	var enf []string
 	for _, p := range policies {
 		if p.ReportOnly {
-			ro = append(ro, p.String())
+			rep = append(rep, p.String())
 		} else {
 			enf = append(enf, p.String())
 		}
 	}
-	return Interceptor{ro: ro, enf: enf}
+	return Interceptor{rep: rep, enf: enf}
 }
 
 // Default returns a same-origin enforcing interceptor with the given (potentially empty) report group.
@@ -69,20 +70,22 @@ func Default(reportGroup string) Interceptor {
 
 // Interceptor is the interceptor for COOP.
 type Interceptor struct {
-	ro  []string
+	rep []string
 	enf []string
 }
 
+// Before claims and sets the Report-Only and Enforcement headers for COOP.
 func (it Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
 	if cfg != nil {
 		// We got an override, run its Before phase instead.
 		return Interceptor(cfg.(Overrider)).Before(w, r, nil)
 	}
 	w.Header().Claim("Cross-Origin-Opener-Policy")(it.enf)
-	w.Header().Claim("Cross-Origin-Opener-Policy-Report-Only")(it.ro)
+	w.Header().Claim("Cross-Origin-Opener-Policy-Report-Only")(it.rep)
 	return safehttp.NotWritten()
 }
 
+// Commit does nothing.
 func (it Interceptor) Commit(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg safehttp.InterceptorConfig) safehttp.Result {
 	return safehttp.NotWritten()
 }
@@ -95,6 +98,7 @@ func Override(policies ...Policy) Overrider {
 	return Overrider(NewInterceptor(policies...))
 }
 
+// Match recognizes just this package Interceptor.
 func (p Overrider) Match(i safehttp.Interceptor) bool {
 	_, ok := i.(Interceptor)
 	return ok

safehttp/plugins/coop/coop_test.go

@@ -15,7 +15,6 @@
 package coop
 
 import (
-	"net/http"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -25,7 +24,7 @@ import (
 
 func TestBefore(t *testing.T) {
 	type want struct {
-		enf, ro []string
+		enf, rep []string
 	}
 	var tests = []struct {
 		name                 string
@@ -62,10 +61,10 @@ func TestBefore(t *testing.T) {
 			}),
 			want: want{
 				enf: []string{`same-origin-allow-popups; report-to "coop-ap"`},
-				ro:  []string{`same-origin; report-to "coop-so"`},
+				rep: []string{`same-origin; report-to "coop-so"`},
 			},
 			wantOverridden: want{
-				ro: []string{`same-origin; report-to "coop-so"`},
+				rep: []string{`same-origin; report-to "coop-so"`},
 			},
 		},
 		{
@@ -84,35 +83,42 @@ func TestBefore(t *testing.T) {
 			}),
 			want: want{
 				enf: []string{`same-origin-allow-popups; report-to "coop-ap"`},
-				ro:  []string{`same-origin; report-to "coop-so"`, `unsafe-none; report-to "coop-un"`},
+				rep: []string{`same-origin; report-to "coop-so"`, `unsafe-none; report-to "coop-un"`},
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			check := func(h http.Header, w want) {
+			check := func(rr *safehttptest.ResponseRecorder, w want) {
 				t.Helper()
-				enf, ro := h.Values("Cross-Origin-Opener-Policy"), h.Values("Cross-Origin-Opener-Policy-Report-Only")
+				h := rr.Header()
+				enf, rep := h.Values("Cross-Origin-Opener-Policy"), h.Values("Cross-Origin-Opener-Policy-Report-Only")
 				if diff := cmp.Diff(w.enf, enf); diff != "" {
 					t.Errorf("Enforced COOP -want +got:\n%s", diff)
 				}
-				if diff := cmp.Diff(w.ro, ro); diff != "" {
+				if diff := cmp.Diff(w.rep, rep); diff != "" {
 					t.Errorf("Report Only COOP -want +got:\n%s", diff)
 				}
+				if rr.Status() != safehttp.StatusOK {
+					t.Errorf("Status: got %v want: %v", rr.Status(), safehttp.StatusOK)
+				}
+				if rr.Body() != "" {
+					t.Errorf("Got body: %q, didn't want one", rr.Body())
+				}
 			}
 			// Non overridden
 			{
 				rr := safehttptest.NewResponseRecorder()
 				req := safehttptest.NewRequest(safehttp.MethodGet, "/", nil)
 				tt.interceptor.Before(rr.ResponseWriter, req, nil)
-				check(rr.Header(), tt.want)
+				check(rr, tt.want)
 			}
 			// Overridden
 			{
 				rr := safehttptest.NewResponseRecorder()
 				req := safehttptest.NewRequest(safehttp.MethodGet, "/", nil)
 				tt.interceptor.Before(rr.ResponseWriter, req, tt.overrider)
-				check(rr.Header(), tt.wantOverridden)
+				check(rr, tt.wantOverridden)
 			}
 		})
 	}