From 0b2297a2e8b20e00e8bee209309467839ffffe66 Mon Sep 17 00:00:00 2001 From: Daan Sprenkels Date: Sat, 30 May 2020 17:49:44 +0200 Subject: [PATCH 1/2] Add a test for issue #60 --- handlers_test.go | 16 ++++++++++++++-- router.go | 12 ++++++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/handlers_test.go b/handlers_test.go index 5f07184..8f9a37c 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -57,7 +57,7 @@ func createTemporaryRouter(t *testing.T) (*mux.Router, *rushlink) { fs: fileStore, rootURL: rootURL, } - return CreateMainRouter(&rl), &rl + return CreateMainRouter(&rl, true), &rl } // checkStatusCode checks whether the status code from a recorded response is equal @@ -145,5 +145,17 @@ func TestIssue53(t *testing.T) { } return nil }) - +} + +func TestIssue60(t *testing.T) { + srv, _ := createTemporaryRouter(t) + + // Put a URL with a fragment identifier into the database. + req, err := http.NewRequest("GET", "/css/nonexistent_file.css", nil) + if err != nil { + t.Fatal(err) + } + rr := httptest.NewRecorder() + srv.ServeHTTP(rr, req) + checkStatusCode(t, rr, http.StatusNotFound) } diff --git a/router.go b/router.go index 9def677..d695c12 100644 --- a/router.go +++ b/router.go @@ -33,6 +33,7 @@ func (rl *rushlink) recoveryMiddleware(next http.Handler) http.Handler { defer func() { defer func() { if err := recover(); err != nil { + w.WriteHeader(500) log.Printf("error: panic while recovering from another panic: %v\n", err) debug.PrintStack() fmt.Fprintf(w, "internal server error: %v\n", err) @@ -40,6 +41,7 @@ func (rl *rushlink) recoveryMiddleware(next http.Handler) http.Handler { }() if err := recover(); err != nil { + w.WriteHeader(500) log.Printf("error: %v\n", err) debug.PrintStack() rl.renderInternalServerError(w, r, err) @@ -80,10 +82,12 @@ func (w *statusResponseWriter) WriteHeader(statusCode int) { } // CreateMainRouter creates the main Gorilla router for the application. -func CreateMainRouter(rl *rushlink) *mux.Router { +func CreateMainRouter(rl *rushlink, debug bool) *mux.Router { router := mux.NewRouter() - router.Use(rl.recoveryMiddleware) - router.Use(rl.metricsMiddleware) + if !debug { + router.Use(rl.recoveryMiddleware) + router.Use(rl.metricsMiddleware) + } router.HandleFunc("/{path:img/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") router.HandleFunc("/{path:css/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") router.HandleFunc("/{path:js/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") @@ -119,7 +123,7 @@ func StartMainServer(addr string, db *db.Database, fs *db.FileStore, rawRootURL } srv := &http.Server{ - Handler: CreateMainRouter(&rl), + Handler: CreateMainRouter(&rl, false), Addr: addr, WriteTimeout: 15 * time.Second, ReadTimeout: 15 * time.Second, -- 2.46.0 From d37222f82aea1188500085d69797a0f7fd06b45a Mon Sep 17 00:00:00 2001 From: Daan Sprenkels Date: Sat, 30 May 2020 17:53:46 +0200 Subject: [PATCH 2/2] Fix error handling in renderStatic Fixes #60 --- handlers_test.go | 4 ++-- router.go | 18 ++++++++++++------ views.go | 10 +++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/handlers_test.go b/handlers_test.go index 8f9a37c..529c2b2 100644 --- a/handlers_test.go +++ b/handlers_test.go @@ -57,7 +57,7 @@ func createTemporaryRouter(t *testing.T) (*mux.Router, *rushlink) { fs: fileStore, rootURL: rootURL, } - return CreateMainRouter(&rl, true), &rl + return CreateMainRouter(&rl), &rl } // checkStatusCode checks whether the status code from a recorded response is equal @@ -150,7 +150,7 @@ func TestIssue53(t *testing.T) { func TestIssue60(t *testing.T) { srv, _ := createTemporaryRouter(t) - // Put a URL with a fragment identifier into the database. + // Request a nonexistent static file req, err := http.NewRequest("GET", "/css/nonexistent_file.css", nil) if err != nil { t.Fatal(err) diff --git a/router.go b/router.go index d695c12..8a89a43 100644 --- a/router.go +++ b/router.go @@ -82,12 +82,13 @@ func (w *statusResponseWriter) WriteHeader(statusCode int) { } // CreateMainRouter creates the main Gorilla router for the application. -func CreateMainRouter(rl *rushlink, debug bool) *mux.Router { +// +// This function will not populate the router with an error-recovery and +// metrics-reporting middleware. If these middleware are required, then the +// caller should encapsulate this router inside of another router and register +// the middlewares on the encapsulating router. +func CreateMainRouter(rl *rushlink) *mux.Router { router := mux.NewRouter() - if !debug { - router.Use(rl.recoveryMiddleware) - router.Use(rl.metricsMiddleware) - } router.HandleFunc("/{path:img/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") router.HandleFunc("/{path:css/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") router.HandleFunc("/{path:js/"+staticFilenameExpr+"}", rl.staticGetHandler).Methods("GET", "HEAD") @@ -122,8 +123,13 @@ func StartMainServer(addr string, db *db.Database, fs *db.FileStore, rawRootURL rootURL: rootURL, } + router := mux.NewRouter() + router.Use(rl.metricsMiddleware) + router.Use(rl.recoveryMiddleware) + router.Handle("/", CreateMainRouter(&rl)) + srv := &http.Server{ - Handler: CreateMainRouter(&rl, false), + Handler: router, Addr: addr, WriteTimeout: 15 * time.Second, ReadTimeout: 15 * time.Second, diff --git a/views.go b/views.go index ba82c98..7b4476f 100644 --- a/views.go +++ b/views.go @@ -88,7 +88,7 @@ func mapExtend(m map[string]interface{}, key string, value interface{}) { func (rl *rushlink) renderStatic(w http.ResponseWriter, r *http.Request, path string) { var modTime time.Time - if info, err := AssetInfo(path); err != nil { + if info, err := AssetInfo(path); err == nil { modTime = info.ModTime() } contents, err := Asset(path) @@ -118,7 +118,12 @@ func (rl *rushlink) render(w http.ResponseWriter, r *http.Request, status int, t err = fmt.Errorf("'%v' not in textTemplates", tmplName) break } - err = tmpl.Execute(w, data) + if status != 0 { + w.WriteHeader(status) + } + if r.Method != "HEAD" { + err = tmpl.Execute(w, data) + } case "text/html": w.Header().Set("Content-Type", "text/html") tmpl := htmlTemplates[tmplName] @@ -142,7 +147,6 @@ func (rl *rushlink) render(w http.ResponseWriter, r *http.Request, status int, t if status != 0 { w.WriteHeader(status) } - if r.Method != "HEAD" { err = tmpl.Execute(w, data) } -- 2.46.0