Repository: google/go-safeweb (go)

Issue:
Provide a StripPrefix utility
Without it static file serving is practically useless: users can only serve static files from the root or mimic the site structure in the static files.

For example if a FileServer is registered on "/static" and a request comes from "/static/foo.css" the FileServer will not lookup for a file called "/static/foo.css" but for one called "static/static/foo.css".

While this is workable it's definitely not a good UX.


Pull request description:
Provide a StripPrefix utility
Notable difference from the `net/http.StripPrefix`: it panics when a prefix cannot be matched. This is clearly a server configuration error.

In order to reduce complexity, the `IncomingRequest.URL` field is no longer there and was replaced by a method. `IncomingRequest` used to contain `req` and `URL` fields, the latter referencing the former. Removing the `URL` field removed the cross-referencing.

Fixes #316.

Patch:
diff --git a/cmd/bancheck/doc.go b/cmd/bancheck/doc.go
deleted file mode 100644
index e2a6fea..0000000
--- a/cmd/bancheck/doc.go
+++ /dev/null
@@ -1,100 +0,0 @@
-// 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 main contains the CLI used for detecting risky APIs.
-// See https://pkg.go.dev/github.com/google/go-safeweb/safehttp#hdr-Restricting_Risky_APIs
-// for a high level overview.
-//
-// Overview
-//
-// Bancheck is a program that allows you to define risky APIs and check for their usage.
-// It can be used as part of the CI/CD pipeline to avoid common pitfalls and prevent
-// potentially vulnerable code from being deployed. Under the hood it uses the go/analysis
-// package https://pkg.go.dev/golang.org/x/tools/go/analysis which provides all the tools that
-// are needed for static code analysis. The tool resolves fully qualified function
-// and import names and checks them against a config file that defines risky APIs.
-//
-// Usage
-//
-// Apart from the standard https://pkg.go.dev/golang.org/x/tools/go/analysis#Analyzer flags
-// the command requires a config flag where a list of config files should be provided.
-// You can find a sample usage below.
-//
-// Config
-//
-// Config lets you specify which APIs should be banned, explain why they are risky to use
-// and allow a list of packages for which the check should be skipped.
-// The structure of a config can be found in go-safeweb/cmd/bancheck/config/config.go.
-//
-// Note: It is possible to have colliding config files e.g. one config file bans a usage of
-// an API but another one exempts it. The tool applies checks from each config file separately
-// i.e. one warning will still be returned.
-//
-// Example config:
-//  {
-// 		"functions": [
-// 			{
-// 				"name": "fmt.Printf",
-// 				"msg": "Banned by team A"
-// 			}
-// 		],
-// 		"imports": [
-// 			{
-// 				"name": "fmt",
-// 				"msg": "Banned by team A",
-//				"exemptions": [
-//					{
-//						"justification": "#yolo",
-//						"allowedPkg": "main"
-//					}
-//				]
-// 			}
-// 		]
-//  }
-//
-// Example
-//
-// The example below shows a simple use case where "fmt" package and "fmt.Printf" function were banned
-// by two separate teams.
-//
-// main.go
-//  package main
-//
-//  import "fmt"
-//
-//  func main() {
-//  	fmt.Printf("Hello")
-//  }
-//
-// config.json
-//  {
-//  	"functions": [
-// 			{
-// 				"name": "fmt.Printf",
-// 	   			"msg": "Banned by team A"
-// 	  		}
-// 	 	],
-//   	"imports": [
-// 	  		{
-// 	   			"name": "fmt",
-// 	   			"msg": "Banned by team B"
-// 	  		}
-// 	 	],
-//  }
-//
-// CLI usage
-//  $ ./bancheck -configs config.json main.go
-//  /go-safeweb/cmd/bancheck/test/main.go:3:8: Banned API found "fmt". Additional info: Banned by team B
-//  /go-safeweb/cmd/bancheck/test/main.go:6:6: Banned API found "fmt.Printf". Additional info: Banned by team A
-package main
diff --git a/examples/echo/echo.go b/examples/echo/echo.go
index ebd69fc..23e4fbb 100644
--- a/examples/echo/echo.go
+++ b/examples/echo/echo.go
@@ -49,7 +49,7 @@ func main() {
 }
 
 func echo(w safehttp.ResponseWriter, req *safehttp.IncomingRequest) safehttp.Result {
-	q, err := req.URL.Query()
+	q, err := req.URL().Query()
 	if err != nil {
 		return w.WriteError(safehttp.StatusBadRequest)
 	}
@@ -72,7 +72,7 @@ func uptime(w safehttp.ResponseWriter, req *safehttp.IncomingRequest) safehttp.R
 	x.Uptime = time.Now().Sub(start)
 
 	// Easter egg handling.
-	q, err := req.URL.Query()
+	q, err := req.URL().Query()
 	if err != nil {
 		return w.WriteError(safehttp.StatusBadRequest)
 	}
diff --git a/safehttp/doc.go b/safehttp/doc.go
index d294fe4..706e4b3 100644
--- a/safehttp/doc.go
+++ b/safehttp/doc.go
@@ -145,12 +145,5 @@
 //
 // Restricting Risky APIs
 //
-// Some APIs are easy-to-misuse in a security sensitive context. We choose to
-// restrict these and require a security review for their usage in order to
-// prevent vulnerabilities.
-//
-// cmd/bancheck allows you to restrict APIs and check for their usage as part
-// of the CI/CD pipeline and prevent potentially vulnerable code from being
-// deployed. For detailed usage instructions, please see:
-// https://pkg.go.dev/github.com/google/go-safeweb/cmd/bancheck
+// TODO
 package safehttp
diff --git a/safehttp/fileserver.go b/safehttp/fileserver.go
index ed96cfe..e942e87 100644
--- a/safehttp/fileserver.go
+++ b/safehttp/fileserver.go
@@ -107,7 +107,7 @@ func (fsrw *fileServerResponseWriter) WriteHeader(statusCode int) {
 	}
 
 	fsrw.result = fsrw.flight.Write(FileServerResponse{
-		Path:        fsrw.flight.req.URL.Path(),
+		Path:        fsrw.flight.req.URL().Path(),
 		contentType: ct,
 	})
 }
diff --git a/safehttp/handler.go b/safehttp/handler.go
index 46c9665..4002c19 100644
--- a/safehttp/handler.go
+++ b/safehttp/handler.go
@@ -31,3 +31,23 @@ type HandlerFunc func(ResponseWriter, *IncomingRequest) Result
 func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *IncomingRequest) Result {
 	return f(w, r)
 }
+
+// StripPrefix returns a handler that serves HTTP requests by removing the given
+// prefix from the request URL's Path (and RawPath if set) and invoking the
+// handler h.
+//
+// StripPrefix handles a request for a path that doesn't begin with prefix by
+// panicking, as this is a server configuration error. The prefix must match
+// exactly (e.g. escaped and unescaped characters are considered different).
+func StripPrefix(prefix string, h Handler) Handler {
+	if prefix == "" {
+		return h
+	}
+	return HandlerFunc(func(rw ResponseWriter, ir *IncomingRequest) Result {
+		ir2, err := ir.WithStrippedURLPrefix(prefix)
+		if err != nil {
+			panic(err)
+		}
+		return h.ServeHTTP(rw, ir2)
+	})
+}
diff --git a/safehttp/incoming_request.go b/safehttp/incoming_request.go
index 1a306af..b625068 100644
--- a/safehttp/incoming_request.go
+++ b/safehttp/incoming_request.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"sync"
 )
@@ -33,9 +34,6 @@ type IncomingRequest struct {
 	// TLS is set just like this TLS field of the net/http.Request. For more information
 	// see https://pkg.go.dev/net/http?tab=doc#Request.
 	TLS *tls.ConnectionState
-	// URL specifies the URL that is parsed from the Request-Line. For most requests,
-	// only URL.Path() will return a non-empty result. (See RFC 7230, Section 5.3)
-	URL *URL
 	req *http.Request
 
 	// The fields below are kept as pointers to allow cloning through
@@ -56,7 +54,6 @@ func NewIncomingRequest(req *http.Request) *IncomingRequest {
 		req:                req,
 		Header:             NewHeader(req.Header),
 		TLS:                req.TLS,
-		URL:                &URL{url: req.URL},
 		postParseOnce:      &sync.Once{},
 		multipartParseOnce: &sync.Once{},
 	}
@@ -177,3 +174,39 @@ func (r *IncomingRequest) WithContext(ctx context.Context) *IncomingRequest {
 	r2.req = r2.req.WithContext(ctx)
 	return r2
 }
+
+// URL specifies the URL that is parsed from the Request-Line. For most requests,
+// only URL.Path() will return a non-empty result. (See RFC 7230, Section 5.3)
+func (r *IncomingRequest) URL() *URL {
+	return &URL{url: r.req.URL}
+}
+
+// WithStrippedURLPrefix returns a shallow copy of the request with its URL
+// stripped of a prefix. The prefix has to match exactly (e.g. escaped and
+// unescaped characters are considered different).
+func (r *IncomingRequest) WithStrippedURLPrefix(prefix string) (*IncomingRequest, error) {
+	req := rawRequest(r)
+	if !strings.HasPrefix(req.URL.Path, prefix) {
+		return nil, fmt.Errorf("Path %q doesn't have prefix %q", req.URL.Path, prefix)
+	}
+	if req.URL.RawPath != "" && !strings.HasPrefix(req.URL.RawPath, prefix) {
+		return nil, fmt.Errorf("RawPath %q doesn't have prefix %q", req.URL.RawPath, prefix)
+	}
+
+	req2 := new(http.Request)
+	*req2 = *req
+	req2.URL = new(url.URL)
+	*req2.URL = *req.URL
+	req2.URL.Path = strings.TrimPrefix(req.URL.Path, prefix)
+	req2.URL.RawPath = strings.TrimPrefix(req.URL.RawPath, prefix)
+
+	r2 := new(IncomingRequest)
+	*r2 = *r
+	r2.req = req2
+
+	return r2, nil
+}
+
+func rawRequest(r *IncomingRequest) *http.Request {
+	return r.req
+}
diff --git a/safehttp/internal/internal.go b/safehttp/internal/internal.go
new file mode 100644
index 0000000..9fc4f4e
--- /dev/null
+++ b/safehttp/internal/internal.go
@@ -0,0 +1,22 @@
+// 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 internal contains internal APIs.
+package internal
+
+// RawRequest is a restricted API. See
+// github.com/google/go-safeweb/safehttp/restricted.RawRequest.
+//
+// After safehttp.init(), this becomes a func(*safehttp.IncomingRequest) *http.Request.
+var RawRequest interface{}
diff --git a/safehttp/internals.go b/safehttp/internals.go
new file mode 100644
index 0000000..4d15bc9
--- /dev/null
+++ b/safehttp/internals.go
@@ -0,0 +1,23 @@
+// 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 safehttp
+
+import (
+	"github.com/google/go-safeweb/safehttp/internal"
+)
+
+func init() {
+	internal.RawRequest = rawRequest
+}
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
index ba3c2a0..6b80a93 100644
--- a/safehttp/plugins/hsts/hsts.go
+++ b/safehttp/plugins/hsts/hsts.go
@@ -95,7 +95,7 @@ func (it Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequ
 	}
 
 	if !it.BehindProxy && r.TLS == nil {
-		u, err := url.Parse(r.URL.String())
+		u, err := url.Parse(r.URL().String())
 		if err != nil {
 			return w.WriteError(safehttp.StatusInternalServerError)
 		}
diff --git a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
index d94f503..53a2b48 100644
--- a/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
+++ b/safehttp/plugins/xsrf/xsrfhtml/xsrf.go
@@ -84,7 +84,7 @@ func (it *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingReq
 		return w.WriteError(safehttp.StatusUnauthorized)
 	}
 
-	if ok := xsrftoken.Valid(tok, it.SecretAppKey, cookieID.Value(), r.URL.Host()); !ok {
+	if ok := xsrftoken.Valid(tok, it.SecretAppKey, cookieID.Value(), r.URL().Host()); !ok {
 		return w.WriteError(safehttp.StatusForbidden)
 	}
 
@@ -125,7 +125,7 @@ func (it *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.Inco
 		return
 	}
 
-	tok := xsrftoken.Generate(it.SecretAppKey, cookieID.Value(), r.URL.Host())
+	tok := xsrftoken.Generate(it.SecretAppKey, cookieID.Value(), r.URL().Host())
 	if tmplResp.FuncMap == nil {
 		tmplResp.FuncMap = map[string]interface{}{}
 	}
diff --git a/safehttp/restricted/request.go b/safehttp/restricted/request.go
new file mode 100644
index 0000000..e9e2c00
--- /dev/null
+++ b/safehttp/restricted/request.go
@@ -0,0 +1,33 @@
+// 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 restricted contains restricted APIs. Their usage should be security
+// reviewed. You can use
+// https://github.com/google/go-safeweb/tree/master/cmd/bancheck to enforce
+// this.
+package restricted
+
+import (
+	"net/http"
+
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/internal"
+)
+
+var rawRequest = internal.RawRequest.(func(*safehttp.IncomingRequest) *http.Request)
+
+// RawRequest returns the underlying *http.Request.
+func RawRequest(r *safehttp.IncomingRequest) *http.Request {
+	return rawRequest(r)
+}


Test patch:
diff --git a/safehttp/defaults/defaults_test.go b/safehttp/defaults/defaults_test.go
index 7589b25..4b42d19 100644
--- a/safehttp/defaults/defaults_test.go
+++ b/safehttp/defaults/defaults_test.go
@@ -30,7 +30,7 @@ func TestServeMuxConfig(t *testing.T) {
 		cfg, _ := ServeMuxConfig([]string{"test.host.example"}, "test-xsrf-key")
 		mux := cfg.Mux()
 		mux.Handle("/test", "GET", safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-			form, err := r.URL.Query()
+			form, err := r.URL().Query()
 			if err != nil {
 				t.Errorf("Cannot parse GET form: %v", err)
 			}
diff --git a/safehttp/flightvalues_test.go b/safehttp/flightvalues_test.go
index 3c7f01d..ad6810a 100644
--- a/safehttp/flightvalues_test.go
+++ b/safehttp/flightvalues_test.go
@@ -63,7 +63,7 @@ func SetHeaderSafely(ctx context.Context, level int) {
 }
 
 func handlerInteractingWithTheInterceptor(w safehttp.ResponseWriter, req *safehttp.IncomingRequest) safehttp.Result {
-	f, err := req.URL.Query()
+	f, err := req.URL().Query()
 	if err != nil {
 		panic(err)
 	}
diff --git a/safehttp/handler_test.go b/safehttp/handler_test.go
new file mode 100644
index 0000000..ea7b74d
--- /dev/null
+++ b/safehttp/handler_test.go
@@ -0,0 +1,77 @@
+// 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 safehttp_test
+
+import (
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/safehtml"
+)
+
+var BarHandler = safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+	if !strings.HasPrefix(r.URL().Path(), "/bar") {
+		return w.WriteError(safehttp.StatusBadRequest)
+	}
+	return w.Write(safehtml.HTMLEscaped("Hello!"))
+})
+
+func TestStripPrefix(t *testing.T) {
+	mux := safehttp.NewServeMuxConfig(nil).Mux()
+
+	mux.Handle("/bar", safehttp.MethodGet, BarHandler)
+	mux.Handle("/more/bar", safehttp.MethodGet, safehttp.StripPrefix("/more", BarHandler))
+
+	r := httptest.NewRequest(safehttp.MethodGet, "http://foo.com/bar", nil)
+	rw := httptest.NewRecorder()
+	mux.ServeHTTP(rw, r)
+
+	rStrip := httptest.NewRequest(safehttp.MethodGet, "http://foo.com/more/bar", nil)
+	rwStrip := httptest.NewRecorder()
+	mux.ServeHTTP(rwStrip, rStrip)
+
+	if rwStrip.Code != rw.Code {
+		t.Errorf("Code got %v, want %v", rwStrip.Code, rw.Code)
+	}
+
+	if diff := cmp.Diff(rw.Header(), rwStrip.Header()); diff != "" {
+		t.Errorf("Header() mismatch (-want +got):\n%s", diff)
+	}
+
+	if got := rwStrip.Body.String(); got != rw.Body.String() {
+		t.Errorf("response body: got %q want %q", got, rw.Body.String())
+	}
+}
+
+func TestStripPrefixPanic(t *testing.T) {
+	mux := safehttp.NewServeMuxConfig(nil).Mux()
+
+	mux.Handle("/bar", safehttp.MethodGet, BarHandler)
+	mux.Handle("/more/bar", safehttp.MethodGet, safehttp.StripPrefix("/badprefix", BarHandler))
+
+	r := httptest.NewRequest(safehttp.MethodGet, "http://foo.com/more/bar", nil)
+	rw := httptest.NewRecorder()
+
+	defer func() {
+		if r := recover(); r != nil {
+			return
+		}
+		t.Errorf("expected panic")
+	}()
+	mux.ServeHTTP(rw, r)
+}
diff --git a/safehttp/internal/internal_test.go b/safehttp/internal/internal_test.go
new file mode 100644
index 0000000..5357da8
--- /dev/null
+++ b/safehttp/internal/internal_test.go
@@ -0,0 +1,29 @@
+// 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 internal_test
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/internal"
+)
+
+func TestRawRequest(t *testing.T) {
+	if _, ok := internal.RawRequest.(func(*safehttp.IncomingRequest) *http.Request); !ok {
+		t.Errorf("RawRequest type got %T, want func(*safehttp.IncomingRequest) *http.Request", internal.RawRequest)
+	}
+}
diff --git a/tests/integration/devmode/devmode_test.go b/tests/integration/devmode/devmode_test.go
index 60b8998..8ae551f 100644
--- a/tests/integration/devmode/devmode_test.go
+++ b/tests/integration/devmode/devmode_test.go
@@ -35,7 +35,7 @@ func TestDevMode(t *testing.T) {
 		cfg, _ := defaults.ServeMuxConfig([]string{"test.host.example"}, "test-xsrf-key")
 		mux := cfg.Mux()
 		mux.Handle("/test", "GET", safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-			form, err := r.URL.Query()
+			form, err := r.URL().Query()
 			if err != nil {
 				t.Errorf("Cannot parse GET form: %v", err)
 			}
diff --git a/tests/integration/devmodefreeze/devmodefreeze_test.go b/tests/integration/devmodefreeze/devmodefreeze_test.go
index b79d8d8..801b434 100644
--- a/tests/integration/devmodefreeze/devmodefreeze_test.go
+++ b/tests/integration/devmodefreeze/devmodefreeze_test.go
@@ -32,7 +32,7 @@ func TestDevMode(t *testing.T) {
 		mux := cfg.Mux()
 
 		mux.Handle("/test", "GET", safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-			form, err := r.URL.Query()
+			form, err := r.URL().Query()
 			if err != nil {
 				t.Errorf("Cannot parse GET form: %v", err)
 			}
diff --git a/tests/integration/errors/errors_test.go b/tests/integration/errors/errors_test.go
index 96e54a4..646ecbe 100644
--- a/tests/integration/errors/errors_test.go
+++ b/tests/integration/errors/errors_test.go
@@ -58,7 +58,7 @@ func TestCustomErrors(t *testing.T) {
 	mux := mb.Mux()
 
 	mux.Handle("/compute", safehttp.MethodGet, safehttp.HandlerFunc(func(w safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
-		qs, err := r.URL.Query()
+		qs, err := r.URL().Query()
 		if err != nil {
 			return w.WriteError(safehttp.StatusBadRequest)
 		}