feat: nextrouter pkg to handle nextjs routing rules (#167)

An issue came up last week... our `embed.go` strategy doesn't handle dynamic NextJS-style routes! This is a blocker, because I'm aiming to set up CD on Monday, and the v2 UI makes heavy use of dynamic routing.

As a potential solution, this implements a go pkg `nextrouter` that serves `html` files, but respecting the dynamic routing behavior of NextJS:
- Files that have square brackets - ie `[providers]` provide a single-level dynamic route
- Files that have `[[...` prefix - ie `[[...any]]` - are catch-all routes.
- Files should be preferred over folders (ie, `providers.html` is preferred over `/providers`)
- Fixes the trailing-slash bug we hit in the previous `embed` strategy

This also integrates with `slog.Logger` for tracing, and handles injecting template parameters - a feature we need in v1 and v2 to be able to inject stuff like CSRF tokens.

This implements testing by using an in-memory file-system, so that we can exercise all of these cases.

In addition, this adjust V2's `embed.go` strategy to use `nextrouter`, which simplifies that file considerably. I'm tempted to factor out the `secureheaders` logic into a separate package, too.

If this works OK, it could be used for V1 too (although that scenario is more complex due to our hybrid-routing strategy). Based on our FE variety meeting, there's always a chance we could move away from NextJS in v1 - if that's the case, this router will still work and be more tested than our previous strategy (it just won't make use of dynamic routing). So I figured this was worth doing to make sure we can make forward progress in V2.

coderd/coderd.go

@@ -126,7 +126,7 @@ func New(options *Options) http.Handler {
 			})
 		})
 	})
-	r.NotFound(site.Handler().ServeHTTP)
+	r.NotFound(site.Handler(options.Logger).ServeHTTP)
 	return r
 }
 

go.mod

@@ -28,6 +28,7 @@ require (
 	github.com/pion/logging v0.2.2
 	github.com/pion/transport v0.13.0
 	github.com/pion/webrtc/v3 v3.1.23
+	github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef
 	github.com/quasilyte/go-ruleguard/dsl v0.3.16
 	github.com/spf13/cobra v1.3.0
 	github.com/stretchr/testify v1.7.0

go.sum

@@ -1088,6 +1088,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
 github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU=
 github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
 github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
+github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef h1:NKxTG6GVGbfMXc2mIk+KphcH6hagbVXhcFkbTgYleTI=
+github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef/go.mod h1:tcaRap0jS3eifrEEllL6ZMd9dg8IlDpi2S1oARrQ+NI=
 github.com/quasilyte/go-ruleguard/dsl v0.3.16 h1:yJtIpd4oyNS+/c/gKqxNwoGO9+lPOsy1A4BzKjJRcrI=
 github.com/quasilyte/go-ruleguard/dsl v0.3.16/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
 github.com/remyoudompheng/bigfft v0.0.0-20190728182440-6a916e37a237/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=

site/embed.go

@@ -1,21 +1,18 @@
 package site
 
 import (
-	"bytes"
 	"embed"
 	"fmt"
-	"io"
 	"io/fs"
 	"net/http"
-	"path"
-	"path/filepath"
 	"strings"
-	"text/template" // html/template escapes some nonces
-	"time"
 
 	"github.com/justinas/nosurf"
 	"github.com/unrolled/secure"
-	"golang.org/x/xerrors"
+
+	"cdr.dev/slog"
+
+	"github.com/coder/coder/site/nextrouter"
 )
 
 // The `embed` package ignores recursively including directories
@@ -27,53 +24,33 @@ import (
 var site embed.FS
 
 // Handler returns an HTTP handler for serving the static site.
-func Handler() http.Handler {
+func Handler(logger slog.Logger) http.Handler {
 	filesystem, err := fs.Sub(site, "out")
 	if err != nil {
 		// This can't happen... Go would throw a compilation error.
 		panic(err)
 	}
 
-	// html files are handled by a text/template. Non-html files
-	// are served by the default file server.
-	files, err := htmlFiles(filesystem)
-	if err != nil {
-		panic(xerrors.Errorf("Failed to return handler for static files. Html files failed to load: %w", err))
+	// Render CSP and CSRF in the served pages
+	templateFunc := func(r *http.Request) interface{} {
+		return htmlState{
+			// Nonce is the CSP nonce for the given request (if there is one present)
+			CSP: cspState{Nonce: secure.CSPNonce(r.Context())},
+			// Token is the CSRF token for the given request
+			CSRF: csrfState{Token: nosurf.Token(r)},
+		}
 	}
 
-	return secureHeaders(&handler{
-		fs:        filesystem,
-		htmlFiles: files,
-		h:         http.FileServer(http.FS(filesystem)), // All other non-html static files
+	nextRouterHandler, err := nextrouter.Handler(filesystem, &nextrouter.Options{
+		Logger:           logger,
+		TemplateDataFunc: templateFunc,
 	})
-}
-
-type handler struct {
-	fs fs.FS
-	// htmlFiles is the text/template for all *.html files.
-	// This is needed to support Content Security Policy headers.
-	// Due to material UI, we are forced to use a nonce to allow inline
-	// scripts, and that nonce is passed through a template.
-	// We only do this for html files to reduce the amount of in memory caching
-	// of duplicate files as `fs`.
-	htmlFiles *htmlTemplates
-	h         http.Handler
-}
-
-// filePath returns the filepath of the requested file.
-func (*handler) filePath(p string) string {
-	if !strings.HasPrefix(p, "/") {
-		p = "/" + p
-	}
-	return strings.TrimPrefix(path.Clean(p), "/")
-}
-
-func (h *handler) exists(filePath string) bool {
-	f, err := h.fs.Open(filePath)
-	if err == nil {
-		_ = f.Close()
+	if err != nil {
+		// There was an error setting up our file system handler.
+		// This likely means a problem with our embedded file system.
+		panic(err)
 	}
-	return err == nil
+	return secureHeaders(nextRouterHandler)
 }
 
 type htmlState struct {
@@ -89,80 +66,6 @@ type csrfState struct {
 	Token string
 }
 
-func (h *handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
-	// reqFile is the static file requested
-	reqFile := h.filePath(r.URL.Path)
-	state := htmlState{
-		// Nonce is the CSP nonce for the given request (if there is one present)
-		CSP: cspState{Nonce: secure.CSPNonce(r.Context())},
-		// Token is the CSRF token for the given request
-		CSRF: csrfState{Token: nosurf.Token(r)},
-	}
-
-	// First check if it's a file we have in our templates
-	if h.serveHTML(rw, r, reqFile, state) {
-		return
-	}
-
-	// If the original file path exists we serve it.
-	if h.exists(reqFile) {
-		h.h.ServeHTTP(rw, r)
-		return
-	}
-
-	// Serve the file assuming it's an html file
-	// This matches paths like `/app/terminal.html`
-	r.URL.Path = strings.TrimSuffix(r.URL.Path, "/")
-	r.URL.Path += ".html"
-
-	reqFile = h.filePath(r.URL.Path)
-	// All html files should be served by the htmlFile templates
-	if h.serveHTML(rw, r, reqFile, state) {
-		return
-	}
-
-	// If we don't have the file... we should redirect to `/`
-	// for our single-page-app.
-	r.URL.Path = "/"
-	if h.serveHTML(rw, r, "", state) {
-		return
-	}
-
-	// This will send a correct 404
-	h.h.ServeHTTP(rw, r)
-}
-
-func (h *handler) serveHTML(rw http.ResponseWriter, r *http.Request, reqPath string, state htmlState) bool {
-	if data, err := h.htmlFiles.renderWithState(reqPath, state); err == nil {
-		if reqPath == "" {
-			// Pass "index.html" to the ServeContent so the ServeContent sets the right content headers.
-			reqPath = "index.html"
-		}
-		http.ServeContent(rw, r, reqPath, time.Time{}, bytes.NewReader(data))
-		return true
-	}
-	return false
-}
-
-type htmlTemplates struct {
-	tpls *template.Template
-}
-
-// renderWithState will render the file using the given nonce if the file exists
-// as a template. If it does not, it will return an error.
-func (t *htmlTemplates) renderWithState(filePath string, state htmlState) ([]byte, error) {
-	var buf bytes.Buffer
-	if filePath == "" {
-		filePath = "index.html"
-	}
-	err := t.tpls.ExecuteTemplate(&buf, filePath, state)
-	if err != nil {
-		return nil, err
-	}
-
-	return buf.Bytes(), nil
-}
-
 // cspDirectives is a map of all csp fetch directives to their values.
 // Each directive is a set of values that is joined by a space (' ').
 // All directives are semi-colon separated as a single string for the csp header.
@@ -264,52 +167,3 @@ func secureHeaders(next http.Handler) http.Handler {
 		ReferrerPolicy: "no-referrer",
 	}).Handler(next)
 }
-
-// htmlFiles recursively walks the file system passed finding all *.html files.
-// The template returned has all html files parsed.
-func htmlFiles(files fs.FS) (*htmlTemplates, error) {
-	// root is the collection of html templates. All templates are named by their pathing.
-	// So './404.html' is named '404.html'. './subdir/index.html' is 'subdir/index.html'
-	root := template.New("")
-
-	rootPath := "."
-	err := fs.WalkDir(files, rootPath, func(path string, dirEntry fs.DirEntry, err error) error {
-		if err != nil {
-			return err
-		}
-
-		if dirEntry.IsDir() {
-			return nil
-		}
-
-		if filepath.Ext(dirEntry.Name()) != ".html" {
-			return nil
-		}
-
-		file, err := files.Open(path)
-		if err != nil {
-			return err
-		}
-
-		data, err := io.ReadAll(file)
-		if err != nil {
-			return err
-		}
-
-		tPath := strings.TrimPrefix(path, rootPath+string(filepath.Separator))
-		_, err = root.New(tPath).Parse(string(data))
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
-
-	if err != nil {
-		return nil, err
-	}
-
-	return &htmlTemplates{
-		tpls: root,
-	}, nil
-}

site/embed_test.go

@@ -9,13 +9,15 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"cdr.dev/slog"
+
 	"github.com/coder/coder/site"
 )
 
 func TestIndexPageRenders(t *testing.T) {
 	t.Parallel()
 
-	srv := httptest.NewServer(site.Handler())
+	srv := httptest.NewServer(site.Handler(slog.Logger{}))
 
 	req, err := http.NewRequestWithContext(context.Background(), "GET", srv.URL, nil)
 	require.NoError(t, err)