Repository: goadesign/goa
Pull request #2856 (2021-05-25 15:02:03)

Add http.Replace() and use it for file servers
This is a fix of #2839.

I'm glad if you release v3.4.3 after merging this. Thank you.

raphael: Thank you for the fix, I will release v3.4.3 once it is merged. I just have one question: would it make sense to refactor `Replace` and `ReplacePrefix` into a single function with an additional argument to differentiate between the two cases or does that end up being too complicated?
tchssk: I integrated ReplacePrefix into Replace. An additional commit is a fix for `http.FileServer`.
raphael: This is great thank you!

Patch:
diff --git a/http/codegen/server.go b/http/codegen/server.go
index 0eef29d1..32680b21 100644
--- a/http/codegen/server.go
+++ b/http/codegen/server.go
@@ -35,12 +35,13 @@ func serverFile(genpkg string, svc *expr.HTTPServiceExpr) *codegen.File {
 	path := filepath.Join(codegen.Gendir, "http", svcName, "server", "server.go")
 	title := fmt.Sprintf("%s HTTP server", svc.Name())
 	funcs := map[string]interface{}{
-		"join":                func(ss []string, s string) string { return strings.Join(ss, s) },
-		"hasWebSocket":        hasWebSocket,
-		"isWebSocketEndpoint": isWebSocketEndpoint,
-		"viewedServerBody":    viewedServerBody,
-		"mustDecodeRequest":   mustDecodeRequest,
-		"addLeadingSlash":     addLeadingSlash,
+		"join":                    func(ss []string, s string) string { return strings.Join(ss, s) },
+		"hasWebSocket":            hasWebSocket,
+		"isWebSocketEndpoint":     isWebSocketEndpoint,
+		"viewedServerBody":        viewedServerBody,
+		"mustDecodeRequest":       mustDecodeRequest,
+		"addLeadingSlash":         addLeadingSlash,
+		"removeTrailingIndexHTML": removeTrailingIndexHTML,
 	}
 	sections := []*codegen.SectionTemplate{
 		codegen.Header(title, "server", []*codegen.ImportSpec{
@@ -246,6 +247,13 @@ func addLeadingSlash(s string) string {
 	return "/" + s
 }
 
+func removeTrailingIndexHTML(s string) string {
+	if strings.HasSuffix(s, "/index.html") {
+		return strings.TrimSuffix(s, "index.html")
+	}
+	return s
+}
+
 func mapQueryDecodeData(dt expr.DataType, varName string, inc int) map[string]interface{} {
 	return map[string]interface{}{
 		"Type":      dt,
@@ -368,9 +376,12 @@ func {{ .MountServer }}(mux goahttp.Muxer, h *{{ .ServerStruct }}) {
 	{{ .MountHandler }}(mux, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			http.Redirect(w, r, "{{ .Redirect.URL }}", {{ .Redirect.StatusCode }})
 		}))
-	 	{{- else }}
-			{{- $filepath := addLeadingSlash .FilePath }}
-	{{ .MountHandler }}(mux, {{ range .RequestPaths }}{{if ne . $filepath }}goahttp.ReplacePrefix("{{ . }}", "{{ $filepath }}", {{ end }}{{ end }}h.{{ .VarName }}){{ range .RequestPaths }}{{ if ne . $filepath }}){{ end}}{{ end }}
+	 	{{- else if .IsDir }}
+			{{- $filepath := addLeadingSlash (removeTrailingIndexHTML .FilePath) }}
+	{{ .MountHandler }}(mux, {{ range .RequestPaths }}{{if ne . $filepath }}goahttp.Replace("{{ . }}", "{{ $filepath }}", {{ end }}{{ end }}h.{{ .VarName }}){{ range .RequestPaths }}{{ if ne . $filepath }}){{ end}}{{ end }}
+		{{- else }}
+			{{- $filepath := addLeadingSlash (removeTrailingIndexHTML .FilePath) }}
+	{{ .MountHandler }}(mux, {{ range .RequestPaths }}{{if ne . $filepath }}goahttp.Replace("", "{{ $filepath }}", {{ end }}{{ end }}h.{{ .VarName }}){{ range .RequestPaths }}{{ if ne . $filepath }}){{ end}}{{ end }}
 		{{- end }}
 	{{- end }}
 }
diff --git a/http/server.go b/http/server.go
index 4b34b6aa..e0453f85 100644
--- a/http/server.go
+++ b/http/server.go
@@ -24,14 +24,21 @@ func (s Servers) Use(m func(http.Handler) http.Handler) {
 	}
 }
 
-// ReplacePrefix returns a handler that serves HTTP requests by replacing the
-// prefix from the request URL's Path (and RawPath if set) and invoking the
-// handler h. The logic is the same as the standard http package StripPrefix
-// function.
-func ReplacePrefix(old, nw string, h http.Handler) http.Handler {
+// Replace returns a handler that serves HTTP requests by replacing the
+// request URL's Path (and RawPath if set) and invoking the handler h.
+// The logic is the same as the standard http package StripPrefix function.
+func Replace(old, nw string, h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		p := strings.Replace(r.URL.Path, old, nw, 1)
-		rp := strings.Replace(r.URL.RawPath, old, nw, 1)
+		var p, rp string
+		if old != "" {
+			p = strings.Replace(r.URL.Path, old, nw, 1)
+			rp = strings.Replace(r.URL.RawPath, old, nw, 1)
+		} else {
+			p = nw
+			if r.URL.RawPath != "" {
+				rp = nw
+			}
+		}
 		if p != r.URL.Path && (r.URL.RawPath == "" || rp != r.URL.RawPath) {
 			r2 := new(http.Request)
 			*r2 = *r


Test patch:
diff --git a/http/codegen/testdata/server_init_functions.go b/http/codegen/testdata/server_init_functions.go
index a36b4856..d2730e03 100644
--- a/http/codegen/testdata/server_init_functions.go
+++ b/http/codegen/testdata/server_init_functions.go
@@ -179,19 +179,19 @@ func New(
 
 var ServerMultipleFilesConstructorCode = `// Mount configures the mux to serve the ServiceFileServer endpoints.
 func Mount(mux goahttp.Muxer, h *Server) {
-	MountPathToFileJSON(mux, goahttp.ReplacePrefix("/file.json", "/path/to/file.json", h.PathToFileJSON))
-	MountPathToFileJSON2(mux, goahttp.ReplacePrefix("/", "/path/to/file.json", h.PathToFileJSON2))
+	MountPathToFileJSON(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON))
+	MountPathToFileJSON2(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON2))
 	MountFileJSON(mux, h.FileJSON)
-	MountPathToFolder(mux, goahttp.ReplacePrefix("/", "/path/to/folder", h.PathToFolder))
+	MountPathToFolder(mux, goahttp.Replace("/", "/path/to/folder", h.PathToFolder))
 }
 `
 
 var ServerMultipleFilesWithPrefixPathConstructorCode = `// Mount configures the mux to serve the ServiceFileServer endpoints.
 func Mount(mux goahttp.Muxer, h *Server) {
-	MountPathToFileJSON(mux, goahttp.ReplacePrefix("/server_file_server/file.json", "/path/to/file.json", h.PathToFileJSON))
-	MountPathToFileJSON2(mux, goahttp.ReplacePrefix("/server_file_server", "/path/to/file.json", h.PathToFileJSON2))
-	MountFileJSON(mux, goahttp.ReplacePrefix("/server_file_server/file.json", "/file.json", h.FileJSON))
-	MountPathToFolder(mux, goahttp.ReplacePrefix("/server_file_server", "/path/to/folder", h.PathToFolder))
+	MountPathToFileJSON(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON))
+	MountPathToFileJSON2(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON2))
+	MountFileJSON(mux, goahttp.Replace("", "/file.json", h.FileJSON))
+	MountPathToFolder(mux, goahttp.Replace("/server_file_server", "/path/to/folder", h.PathToFolder))
 }
 `
 
@@ -200,9 +200,9 @@ func Mount(mux goahttp.Muxer, h *Server) {
 	MountPathToFileJSON(mux, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		http.Redirect(w, r, "/redirect/dest", http.StatusMovedPermanently)
 	}))
-	MountPathToFileJSON2(mux, goahttp.ReplacePrefix("/", "/path/to/file.json", h.PathToFileJSON2))
+	MountPathToFileJSON2(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON2))
 	MountFileJSON(mux, h.FileJSON)
-	MountPathToFolder(mux, goahttp.ReplacePrefix("/", "/path/to/folder", h.PathToFolder))
+	MountPathToFolder(mux, goahttp.Replace("/", "/path/to/folder", h.PathToFolder))
 }
 `
 
diff --git a/http/server_test.go b/http/server_test.go
new file mode 100644
index 00000000..dacf99cf
--- /dev/null
+++ b/http/server_test.go
@@ -0,0 +1,63 @@
+package http
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestReplace(t *testing.T) {
+	var (
+		h = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("X-Path", r.URL.Path)
+			w.Header().Set("X-RawPath", r.URL.RawPath)
+		})
+	)
+	cases := []struct {
+		old     string
+		nw      string
+		reqPath string
+		path    string // If empty we want a 404.
+		rawPath string
+	}{
+		{"/foo/bar", "", "/foo/bar/qux", "/qux", ""},
+		{"/foo/bar", "", "/foo/bar%2Fqux", "/qux", "%2Fqux"},
+		{"/foo/bar", "", "/foo%2Fbar/qux", "", ""}, // Escaped prefix does not match.
+		{"/foo/bar", "", "/bar", "", ""},           // No prefix match.
+		{"/foo/bar", "/baz", "/foo/bar/qux", "/baz/qux", ""},
+		{"/foo/bar", "/baz", "/foo/bar%2Fqux", "/baz/qux", "/baz%2Fqux"},
+		{"/foo/bar", "/baz", "/foo%2Fbar/qux", "", ""}, // Escaped prefix does not match.
+		{"/foo/bar", "/baz", "/bar", "", ""},           // No prefix match.
+		{"", "/baz/baz/baz", "/foo/bar/qux", "/baz/baz/baz", ""},
+		{"", "/baz/baz/baz", "/foo/bar%2Fqux", "/baz/baz/baz", "/baz/baz/baz"},
+		{"", "/baz/baz/baz", "/foo%2Fbar/qux", "/baz/baz/baz", "/baz/baz/baz"},
+		{"", "/baz/baz/baz", "/bar", "/baz/baz/baz", ""},
+	}
+	for _, tc := range cases {
+		t.Run(tc.reqPath, func(t *testing.T) {
+			ts := httptest.NewServer(Replace(tc.old, tc.nw, h))
+			defer ts.Close()
+			c := ts.Client()
+			res, err := c.Get(ts.URL + tc.reqPath)
+			if err != nil {
+				t.Fatal(err)
+			}
+			res.Body.Close()
+			if tc.path == "" {
+				if res.StatusCode != http.StatusNotFound {
+					t.Errorf("got %q, want 404 Not Found", res.Status)
+				}
+				return
+			}
+			if res.StatusCode != http.StatusOK {
+				t.Fatalf("got %q, want 200 OK", res.Status)
+			}
+			if g, w := res.Header.Get("X-Path"), tc.path; g != w {
+				t.Errorf("got Path %q, want %q", g, w)
+			}
+			if g, w := res.Header.Get("X-RawPath"), tc.rawPath; g != w {
+				t.Errorf("got RawPath %q, want %q", g, w)
+			}
+		})
+	}
+}