Skip to content

Commit 78bf4c6

Browse files
bryphe-coderbryphe-coder
authored
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.
1 parent 61b3909 commit 78bf4c6

File tree

7 files changed

+640
-169
lines changed

7 files changed

+640
-169
lines changed

‎coderd/coderd.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func New(options *Options) http.Handler {
126126
})
127127
})
128128
})
129-
r.NotFound(site.Handler().ServeHTTP)
129+
r.NotFound(site.Handler(options.Logger).ServeHTTP)
130130
return r
131131
}
132132

‎go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ require (
2828
github.com/pion/logging v0.2.2
2929
github.com/pion/transport v0.13.0
3030
github.com/pion/webrtc/v3 v3.1.23
31+
github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef
3132
github.com/quasilyte/go-ruleguard/dsl v0.3.16
3233
github.com/spf13/cobra v1.3.0
3334
github.com/stretchr/testify v1.7.0

‎go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,6 +1088,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
10881088
github.com/prometheus/procfs v0.2.0/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4OA4YeYWdaU=
10891089
github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
10901090
github.com/prometheus/tsdb v0.7.1/go.mod h1:qhTCs0VvXwvX/y3TZrWD7rabWM+ijKTux40TwIPHuXU=
1091+
github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef h1:NKxTG6GVGbfMXc2mIk+KphcH6hagbVXhcFkbTgYleTI=
1092+
github.com/psanford/memfs v0.0.0-20210214183328-a001468d78ef/go.mod h1:tcaRap0jS3eifrEEllL6ZMd9dg8IlDpi2S1oARrQ+NI=
10911093
github.com/quasilyte/go-ruleguard/dsl v0.3.16 h1:yJtIpd4oyNS+/c/gKqxNwoGO9+lPOsy1A4BzKjJRcrI=
10921094
github.com/quasilyte/go-ruleguard/dsl v0.3.16/go.mod h1:KeCP03KrjuSO0H1kTuZQCWlQPulDV6YMIXmpQss17rU=
10931095
github.com/remyoudompheng/bigfft v0.0.0-20190728182440-6a916e37a237/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=

‎site/embed.go

Lines changed: 21 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,18 @@
11
package site
22

33
import (
4-
"bytes"
54
"embed"
65
"fmt"
7-
"io"
86
"io/fs"
97
"net/http"
10-
"path"
11-
"path/filepath"
128
"strings"
13-
"text/template" // html/template escapes some nonces
14-
"time"
159

1610
"github.com/justinas/nosurf"
1711
"github.com/unrolled/secure"
18-
"golang.org/x/xerrors"
12+
13+
"cdr.dev/slog"
14+
15+
"github.com/coder/coder/site/nextrouter"
1916
)
2017

2118
// The `embed` package ignores recursively including directories
@@ -27,53 +24,33 @@ import (
2724
var site embed.FS
2825

2926
// Handler returns an HTTP handler for serving the static site.
30-
func Handler() http.Handler {
27+
func Handler(logger slog.Logger) http.Handler {
3128
filesystem, err := fs.Sub(site, "out")
3229
if err != nil {
3330
// This can't happen... Go would throw a compilation error.
3431
panic(err)
3532
}
3633

37-
// html files are handled by a text/template. Non-html files
38-
// are served by the default file server.
39-
files, err := htmlFiles(filesystem)
40-
if err != nil {
41-
panic(xerrors.Errorf("Failed to return handler for static files. Html files failed to load: %w", err))
34+
// Render CSP and CSRF in the served pages
35+
templateFunc := func(r *http.Request) interface{} {
36+
return htmlState{
37+
// Nonce is the CSP nonce for the given request (if there is one present)
38+
CSP: cspState{Nonce: secure.CSPNonce(r.Context())},
39+
// Token is the CSRF token for the given request
40+
CSRF: csrfState{Token: nosurf.Token(r)},
41+
}
4242
}
4343

44-
return secureHeaders(&handler{
45-
fs: filesystem,
46-
htmlFiles: files,
47-
h: http.FileServer(http.FS(filesystem)), // All other non-html static files
44+
nextRouterHandler, err := nextrouter.Handler(filesystem, &nextrouter.Options{
45+
Logger: logger,
46+
TemplateDataFunc: templateFunc,
4847
})
49-
}
50-
51-
type handler struct {
52-
fs fs.FS
53-
// htmlFiles is the text/template for all *.html files.
54-
// This is needed to support Content Security Policy headers.
55-
// Due to material UI, we are forced to use a nonce to allow inline
56-
// scripts, and that nonce is passed through a template.
57-
// We only do this for html files to reduce the amount of in memory caching
58-
// of duplicate files as `fs`.
59-
htmlFiles *htmlTemplates
60-
h http.Handler
61-
}
62-
63-
// filePath returns the filepath of the requested file.
64-
func (*handler) filePath(p string) string {
65-
if !strings.HasPrefix(p, "/") {
66-
p = "/" + p
67-
}
68-
return strings.TrimPrefix(path.Clean(p), "/")
69-
}
70-
71-
func (h *handler) exists(filePath string) bool {
72-
f, err := h.fs.Open(filePath)
73-
if err == nil {
74-
_ = f.Close()
48+
if err != nil {
49+
// There was an error setting up our file system handler.
50+
// This likely means a problem with our embedded file system.
51+
panic(err)
7552
}
76-
return err == nil
53+
return secureHeaders(nextRouterHandler)
7754
}
7855

7956
type htmlState struct {
@@ -89,80 +66,6 @@ type csrfState struct {
8966
Token string
9067
}
9168

92-
func (h *handler) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
93-
// reqFile is the static file requested
94-
reqFile := h.filePath(r.URL.Path)
95-
state := htmlState{
96-
// Nonce is the CSP nonce for the given request (if there is one present)
97-
CSP: cspState{Nonce: secure.CSPNonce(r.Context())},
98-
// Token is the CSRF token for the given request
99-
CSRF: csrfState{Token: nosurf.Token(r)},
100-
}
101-
102-
// First check if it's a file we have in our templates
103-
if h.serveHTML(rw, r, reqFile, state) {
104-
return
105-
}
106-
107-
// If the original file path exists we serve it.
108-
if h.exists(reqFile) {
109-
h.h.ServeHTTP(rw, r)
110-
return
111-
}
112-
113-
// Serve the file assuming it's an html file
114-
// This matches paths like `/app/terminal.html`
115-
r.URL.Path = strings.TrimSuffix(r.URL.Path, "/")
116-
r.URL.Path += ".html"
117-
118-
reqFile = h.filePath(r.URL.Path)
119-
// All html files should be served by the htmlFile templates
120-
if h.serveHTML(rw, r, reqFile, state) {
121-
return
122-
}
123-
124-
// If we don't have the file... we should redirect to `/`
125-
// for our single-page-app.
126-
r.URL.Path = "/"
127-
if h.serveHTML(rw, r, "", state) {
128-
return
129-
}
130-
131-
// This will send a correct 404
132-
h.h.ServeHTTP(rw, r)
133-
}
134-
135-
func (h *handler) serveHTML(rw http.ResponseWriter, r *http.Request, reqPath string, state htmlState) bool {
136-
if data, err := h.htmlFiles.renderWithState(reqPath, state); err == nil {
137-
if reqPath == "" {
138-
// Pass "index.html" to the ServeContent so the ServeContent sets the right content headers.
139-
reqPath = "index.html"
140-
}
141-
http.ServeContent(rw, r, reqPath, time.Time{}, bytes.NewReader(data))
142-
return true
143-
}
144-
return false
145-
}
146-
147-
type htmlTemplates struct {
148-
tpls *template.Template
149-
}
150-
151-
// renderWithState will render the file using the given nonce if the file exists
152-
// as a template. If it does not, it will return an error.
153-
func (t *htmlTemplates) renderWithState(filePath string, state htmlState) ([]byte, error) {
154-
var buf bytes.Buffer
155-
if filePath == "" {
156-
filePath = "index.html"
157-
}
158-
err := t.tpls.ExecuteTemplate(&buf, filePath, state)
159-
if err != nil {
160-
return nil, err
161-
}
162-
163-
return buf.Bytes(), nil
164-
}
165-
16669
// cspDirectives is a map of all csp fetch directives to their values.
16770
// Each directive is a set of values that is joined by a space (' ').
16871
// 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 {
264167
ReferrerPolicy: "no-referrer",
265168
}).Handler(next)
266169
}
267-
268-
// htmlFiles recursively walks the file system passed finding all *.html files.
269-
// The template returned has all html files parsed.
270-
func htmlFiles(files fs.FS) (*htmlTemplates, error) {
271-
// root is the collection of html templates. All templates are named by their pathing.
272-
// So './404.html' is named '404.html'. './subdir/index.html' is 'subdir/index.html'
273-
root := template.New("")
274-
275-
rootPath := "."
276-
err := fs.WalkDir(files, rootPath, func(path string, dirEntry fs.DirEntry, err error) error {
277-
if err != nil {
278-
return err
279-
}
280-
281-
if dirEntry.IsDir() {
282-
return nil
283-
}
284-
285-
if filepath.Ext(dirEntry.Name()) != ".html" {
286-
return nil
287-
}
288-
289-
file, err := files.Open(path)
290-
if err != nil {
291-
return err
292-
}
293-
294-
data, err := io.ReadAll(file)
295-
if err != nil {
296-
return err
297-
}
298-
299-
tPath := strings.TrimPrefix(path, rootPath+string(filepath.Separator))
300-
_, err = root.New(tPath).Parse(string(data))
301-
if err != nil {
302-
return err
303-
}
304-
305-
return nil
306-
})
307-
308-
if err != nil {
309-
return nil, err
310-
}
311-
312-
return &htmlTemplates{
313-
tpls: root,
314-
}, nil
315-
}

‎site/embed_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,15 @@ import (
99

1010
"github.com/stretchr/testify/require"
1111

12+
"cdr.dev/slog"
13+
1214
"github.com/coder/coder/site"
1315
)
1416

1517
func TestIndexPageRenders(t *testing.T) {
1618
t.Parallel()
1719

18-
srv := httptest.NewServer(site.Handler())
20+
srv := httptest.NewServer(site.Handler(slog.Logger{}))
1921

2022
req, err := http.NewRequestWithContext(context.Background(), "GET", srv.URL, nil)
2123
require.NoError(t, err)

0 commit comments

Comments
 (0)