Repository: goadesign/goa
Pull request #3588 (2024-09-10 05:46:48)

Refactor file server implementation
So that it doesn't require introducing a `Replace` function. The new implementation is more robust and properly handles cases where the file path is deep and differs from the mount path.


Patch:
diff --git a/http/codegen/server.go b/http/codegen/server.go
index 4c9e2b23..bbaf5ca4 100644
--- a/http/codegen/server.go
+++ b/http/codegen/server.go
@@ -2,6 +2,7 @@ package codegen
 
 import (
 	"fmt"
+	"path"
 	"path/filepath"
 	"reflect"
 	"strings"
@@ -32,16 +33,16 @@ func ServerFiles(genpkg string, root *expr.RootExpr) []*codegen.File {
 func serverFile(genpkg string, svc *expr.HTTPServiceExpr) *codegen.File {
 	data := HTTPServices.Get(svc.Name())
 	svcName := data.Service.PathName
-	path := filepath.Join(codegen.Gendir, "http", svcName, "server", "server.go")
+	fpath := filepath.Join(codegen.Gendir, "http", svcName, "server", "server.go")
 	title := fmt.Sprintf("%s HTTP server", svc.Name())
 	funcs := map[string]any{
-		"join":                    strings.Join,
-		"hasWebSocket":            hasWebSocket,
-		"isWebSocketEndpoint":     isWebSocketEndpoint,
-		"viewedServerBody":        viewedServerBody,
-		"mustDecodeRequest":       mustDecodeRequest,
-		"addLeadingSlash":         addLeadingSlash,
-		"removeTrailingIndexHTML": removeTrailingIndexHTML,
+		"join":                strings.Join,
+		"hasWebSocket":        hasWebSocket,
+		"isWebSocketEndpoint": isWebSocketEndpoint,
+		"viewedServerBody":    viewedServerBody,
+		"mustDecodeRequest":   mustDecodeRequest,
+		"addLeadingSlash":     addLeadingSlash,
+		"dir":                 path.Dir,
 	}
 	imports := []*codegen.ImportSpec{
 		{Path: "bufio"},
@@ -86,11 +87,14 @@ func serverFile(genpkg string, svc *expr.HTTPServiceExpr) *codegen.File {
 		sections = append(sections, &codegen.SectionTemplate{Name: "server-handler", Source: readTemplate("server_handler"), Data: e})
 		sections = append(sections, &codegen.SectionTemplate{Name: "server-handler-init", Source: readTemplate("server_handler_init"), FuncMap: funcs, Data: e})
 	}
+	if len(data.FileServers) > 0 {
+		sections = append(sections, &codegen.SectionTemplate{Name: "append-fs", Source: readTemplate("append_fs"), FuncMap: funcs, Data: data})
+	}
 	for _, s := range data.FileServers {
 		sections = append(sections, &codegen.SectionTemplate{Name: "server-files", Source: readTemplate("file_server"), FuncMap: funcs, Data: s})
 	}
 
-	return &codegen.File{Path: path, SectionTemplates: sections}
+	return &codegen.File{Path: fpath, SectionTemplates: sections}
 }
 
 // serverEncodeDecodeFile returns the file defining the HTTP server encoding and
@@ -252,13 +256,6 @@ 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]any {
 	return map[string]any{
 		"Type":      dt,
diff --git a/http/codegen/templates/append_fs.go.tpl b/http/codegen/templates/append_fs.go.tpl
new file mode 100644
index 00000000..08233822
--- /dev/null
+++ b/http/codegen/templates/append_fs.go.tpl
@@ -0,0 +1,18 @@
+// appendFS is a custom implementation of fs.FS that appends a specified prefix
+// to the file paths before delegating the Open call to the underlying fs.FS.
+type appendFS struct {
+	prefix string
+	fs     http.FileSystem
+}
+
+// Open opens the named file, appending the prefix to the file path before
+// passing it to the underlying fs.FS.
+func (s appendFS) Open(name string) (http.File, error) {
+	return s.fs.Open(path.Join(s.prefix, name))
+}
+
+// appendPrefix returns a new fs.FS that appends the specified prefix to file paths
+// before delegating to the provided embed.FS.
+func appendPrefix(fsys http.FileSystem, prefix string) http.FileSystem {
+	return appendFS{prefix: prefix, fs: fsys}
+}
\ No newline at end of file
diff --git a/http/codegen/templates/server_init.go.tpl b/http/codegen/templates/server_init.go.tpl
index c7b74284..a80ce6c7 100644
--- a/http/codegen/templates/server_init.go.tpl
+++ b/http/codegen/templates/server_init.go.tpl
@@ -28,6 +28,13 @@ func {{ .ServerInit }}(
 	if {{ .ArgName }} == nil {
 		{{ .ArgName }} = http.Dir(".")
 	}
+		{{- $prefix := addLeadingSlash .FilePath }}
+		{{- if not .IsDir }}
+			{{- $prefix = dir $prefix }}
+		{{- end }}
+		{{- if ne $prefix "/" }}
+	{{ .ArgName }} = appendPrefix({{ .ArgName }}, "{{ $prefix }}")
+		{{- end }}
 	{{- end }}
 	return &{{ .ServerStruct }}{
 		Mounts: []*{{ .MountPointStruct }}{
@@ -39,7 +46,7 @@ func {{ .ServerInit }}(
 			{{- range .FileServers }}
 				{{- $filepath := .FilePath }}
 				{{- range .RequestPaths }}
-			{"{{ $filepath }}", "GET", "{{ . }}"},
+			{"Serve {{ $filepath }}", "GET", "{{ . }}"},
 				{{- end }}
 			{{- end }}
 		},
diff --git a/http/codegen/templates/server_mount.go.tpl b/http/codegen/templates/server_mount.go.tpl
index e9c7520f..01b4fc29 100644
--- a/http/codegen/templates/server_mount.go.tpl
+++ b/http/codegen/templates/server_mount.go.tpl
@@ -8,12 +8,21 @@ 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 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 }}
+			{{- $mountHandler := .MountHandler }}
+			{{- $varName := .VarName }}
+			{{- $isDir := .IsDir }}
+			{{- range .RequestPaths }}
+				{{- $stripped := addLeadingSlash . }}
+				{{- if not $isDir }}
+					{{- $stripped = (dir $stripped) }}
+				{{- end }}
+				{{- if eq $stripped "/" }}
+	{{ $mountHandler }}(mux, h.{{ $varName }}) 
+				{{- else }}
+	{{ $mountHandler }}(mux, http.StripPrefix("{{ $stripped }}", h.{{ $varName }}))
+				{{- end }}
+			{{- end }}
 		{{- end }}
 	{{- end }}
 }
diff --git a/http/server.go b/http/server.go
index 396928e7..2bfc843f 100644
--- a/http/server.go
+++ b/http/server.go
@@ -2,8 +2,6 @@ package http
 
 import (
 	"net/http"
-	"net/url"
-	"strings"
 )
 
 type (
@@ -38,32 +36,3 @@ func (s Servers) Mount(mux Muxer) {
 		m.Mount(mux)
 	}
 }
-
-// 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) {
-		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
-			r2.URL = new(url.URL)
-			*r2.URL = *r.URL
-			r2.URL.Path = p
-			r2.URL.RawPath = rp
-			h.ServeHTTP(w, r2)
-		} else {
-			http.NotFound(w, r)
-		}
-	})
-}


Test patch:
diff --git a/http/codegen/testdata/server_init_functions.go b/http/codegen/testdata/server_init_functions.go
index ee3a31ee..2a9e03fd 100644
--- a/http/codegen/testdata/server_init_functions.go
+++ b/http/codegen/testdata/server_init_functions.go
@@ -69,17 +69,20 @@ func New(
 	if fileSystemPathToFile1JSON == nil {
 		fileSystemPathToFile1JSON = http.Dir(".")
 	}
+	fileSystemPathToFile1JSON = appendPrefix(fileSystemPathToFile1JSON, "/path/to")
 	if fileSystemPathToFile2JSON == nil {
 		fileSystemPathToFile2JSON = http.Dir(".")
 	}
+	fileSystemPathToFile2JSON = appendPrefix(fileSystemPathToFile2JSON, "/path/to")
 	if fileSystemPathToFile3JSON == nil {
 		fileSystemPathToFile3JSON = http.Dir(".")
 	}
+	fileSystemPathToFile3JSON = appendPrefix(fileSystemPathToFile3JSON, "/path/to")
 	return &Server{
 		Mounts: []*MountPoint{
-			{"/path/to/file1.json", "GET", "/server_file_server/file1.json"},
-			{"/path/to/file2.json", "GET", "/server_file_server/file2.json"},
-			{"/path/to/file3.json", "GET", "/server_file_server/file3.json"},
+			{"Serve /path/to/file1.json", "GET", "/server_file_server/file1.json"},
+			{"Serve /path/to/file2.json", "GET", "/server_file_server/file2.json"},
+			{"Serve /path/to/file3.json", "GET", "/server_file_server/file3.json"},
 		},
 		PathToFile1JSON: http.FileServer(fileSystemPathToFile1JSON),
 		PathToFile2JSON: http.FileServer(fileSystemPathToFile2JSON),
@@ -107,15 +110,17 @@ func New(
 	if fileSystemPathToFile1JSON == nil {
 		fileSystemPathToFile1JSON = http.Dir(".")
 	}
+	fileSystemPathToFile1JSON = appendPrefix(fileSystemPathToFile1JSON, "/path/to")
 	if fileSystemPathToFile2JSON == nil {
 		fileSystemPathToFile2JSON = http.Dir(".")
 	}
+	fileSystemPathToFile2JSON = appendPrefix(fileSystemPathToFile2JSON, "/path/to")
 	return &Server{
 		Mounts: []*MountPoint{
 			{"MethodMixed1", "GET", "/resources1/{id}"},
 			{"MethodMixed2", "GET", "/resources2/{id}"},
-			{"/path/to/file1.json", "GET", "/file1.json"},
-			{"/path/to/file2.json", "GET", "/file2.json"},
+			{"Serve /path/to/file1.json", "GET", "/file1.json"},
+			{"Serve /path/to/file2.json", "GET", "/file2.json"},
 		},
 		MethodMixed1:    NewMethodMixed1Handler(e.MethodMixed1, mux, decoder, encoder, errhandler, formatter),
 		MethodMixed2:    NewMethodMixed2Handler(e.MethodMixed2, mux, decoder, encoder, errhandler, formatter),
@@ -179,10 +184,10 @@ func New(
 
 var ServerMultipleFilesConstructorCode = `// Mount configures the mux to serve the ServiceFileServer endpoints.
 func Mount(mux goahttp.Muxer, h *Server) {
-	MountPathToFileJSON(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON))
-	MountPathToFileJSON2(mux, goahttp.Replace("", "/path/to/file.json", h.PathToFileJSON2))
+	MountPathToFileJSON(mux, h.PathToFileJSON)
+	MountPathToFileJSON2(mux, h.PathToFileJSON2)
 	MountFileJSON(mux, h.FileJSON)
-	MountPathToFolder(mux, goahttp.Replace("/", "/path/to/folder", h.PathToFolder))
+	MountPathToFolder(mux, h.PathToFolder)
 }
 
 // Mount configures the mux to serve the ServiceFileServer endpoints.
@@ -193,10 +198,10 @@ func (s *Server) Mount(mux goahttp.Muxer) {
 
 var ServerMultipleFilesWithPrefixPathConstructorCode = `// Mount configures the mux to serve the ServiceFileServer endpoints.
 func Mount(mux goahttp.Muxer, h *Server) {
-	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))
+	MountPathToFileJSON(mux, http.StripPrefix("/server_file_server", h.PathToFileJSON))
+	MountPathToFileJSON2(mux, h.PathToFileJSON2)
+	MountFileJSON(mux, http.StripPrefix("/server_file_server", h.FileJSON))
+	MountPathToFolder(mux, http.StripPrefix("/server_file_server", h.PathToFolder))
 }
 
 // Mount configures the mux to serve the ServiceFileServer endpoints.
@@ -210,9 +215,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.Replace("", "/path/to/file.json", h.PathToFileJSON2))
+	MountPathToFileJSON2(mux, h.PathToFileJSON2)
 	MountFileJSON(mux, h.FileJSON)
-	MountPathToFolder(mux, goahttp.Replace("/", "/path/to/folder", h.PathToFolder))
+	MountPathToFolder(mux, h.PathToFolder)
 }
 
 // Mount configures the mux to serve the ServiceFileServer endpoints.
diff --git a/http/server_test.go b/http/server_test.go
deleted file mode 100644
index dacf99cf..00000000
--- a/http/server_test.go
+++ /dev/null
@@ -1,63 +0,0 @@
-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)
-			}
-		})
-	}
-}