Repository: gobuffalo/buffalo
Pull request #1639 (2019-03-29 15:32:43)

Calling error handlers when the url is wrong on the fileserve
This pr calls the error handler when the fileserver cannot parse the url that's being asked for, it also changes the http code to be 400 given the HTTP specification says:

> The request could not be understood by the server due to malformed syntax. The client SHOULD NOT repeat the request without modifications.

And i feel it would be more adequate for this case.

paganotoni: @markbates added tests for this case and also changed `events.LoadPlugins` to `buffalo.LoadPlugins` because some tests were failing.

Patch:
diff --git a/buffalo/cmd/root.go b/buffalo/cmd/root.go
index 26db4eac..f5b9711c 100644
--- a/buffalo/cmd/root.go
+++ b/buffalo/cmd/root.go
@@ -6,7 +6,7 @@ import (
 	"os"
 	"strings"
 
-	"github.com/gobuffalo/events"
+	"github.com/gobuffalo/buffalo"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -19,7 +19,7 @@ var RootCmd = &cobra.Command{
 	Use:           "buffalo",
 	Short:         "Build Buffalo applications with ease",
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
-		if err := events.LoadPlugins(); err != nil {
+		if err := buffalo.LoadPlugins(); err != nil {
 			return err
 		}
 		isFreeCommand := false
diff --git a/route_mappings.go b/route_mappings.go
index af19458d..be363e8b 100644
--- a/route_mappings.go
+++ b/route_mappings.go
@@ -94,7 +94,7 @@ func (a *App) ServeFiles(p string, root http.FileSystem) {
 	path := path.Join(a.Prefix, p)
 	a.filepaths = append(a.filepaths, path)
 
-	h := stripAsset(path, a.fileServer(root))
+	h := stripAsset(path, a.fileServer(root), a)
 	a.router.PathPrefix(path).Handler(h)
 }
 
@@ -316,7 +316,7 @@ func (a *App) buildRouteName(p string) string {
 	return name.VarCase(underscore)
 }
 
-func stripAsset(path string, h http.Handler) http.Handler {
+func stripAsset(path string, h http.Handler, a *App) http.Handler {
 	if path == "" {
 		return h
 	}
@@ -325,12 +325,14 @@ func stripAsset(path string, h http.Handler) http.Handler {
 		up := r.URL.Path
 		up = strings.TrimPrefix(up, path)
 		up = strings.TrimSuffix(up, "/")
+
 		u, err := url.Parse(up)
 		if err != nil {
-			w.WriteHeader(500)
-			w.Write([]byte(err.Error()))
+			eh := a.ErrorHandlers.Get(400)
+			eh(400, err, a.newContext(RouteInfo{}, w, r))
 			return
 		}
+
 		r.URL = u
 		h.ServeHTTP(w, r)
 	})


Test patch:
diff --git a/router_test.go b/router_test.go
index 89a19354..b2cb601e 100644
--- a/router_test.go
+++ b/router_test.go
@@ -359,6 +359,24 @@ func Test_Router_ServeFiles(t *testing.T) {
 	r.Equal(res.Header().Get("Cache-Control"), "max-age=3600")
 }
 
+func Test_Router_InvalidURL(t *testing.T) {
+	r := require.New(t)
+
+	box := packd.NewMemoryBox()
+	box.AddString("foo.png", "foo")
+	a := New(Options{})
+	a.ServeFiles("/", box)
+
+	w := httptest.New(a)
+	s := "/%25%7dn2zq0%3cscript%3ealert(1)%3c\\/script%3evea7f"
+
+	request, _ := http.NewRequest("GET", s, nil)
+	response := httptest.NewRecorder()
+
+	w.ServeHTTP(response, request)
+	r.Equal(http.StatusBadRequest, response.Code, "(400) BadRequest response is expected")
+}
+
 type WebResource struct {
 	BaseResource
 }
@@ -654,17 +672,17 @@ func Test_ResourceOnResource(t *testing.T) {
 func Test_buildRouteName(t *testing.T) {
 	r := require.New(t)
 	cases := map[string]string{
-		"/":                                          "root",
-		"/users":                                     "users",
-		"/users/new":                                 "newUsers",
-		"/users/{user_id}":                           "user",
-		"/users/{user_id}/children":                  "userChildren",
-		"/users/{user_id}/children/{child_id}":       "userChild",
-		"/users/{user_id}/children/new":              "newUserChildren",
+		"/":                                    "root",
+		"/users":                               "users",
+		"/users/new":                           "newUsers",
+		"/users/{user_id}":                     "user",
+		"/users/{user_id}/children":            "userChildren",
+		"/users/{user_id}/children/{child_id}": "userChild",
+		"/users/{user_id}/children/new":        "newUserChildren",
 		"/users/{user_id}/children/{child_id}/build": "userChildBuild",
-		"/admin/planes":                              "adminPlanes",
-		"/admin/planes/{plane_id}":                   "adminPlane",
-		"/admin/planes/{plane_id}/edit":              "editAdminPlane",
+		"/admin/planes":                 "adminPlanes",
+		"/admin/planes/{plane_id}":      "adminPlane",
+		"/admin/planes/{plane_id}/edit": "editAdminPlane",
 	}
 
 	a := New(Options{})