From fa25f03885e68990af694fcc394539152126b1c8 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 15:04:14 +0530 Subject: [PATCH 1/8] add test case negative and positive for `SPA` filepath.Abs err on windows --- mux_httpserver_test.go | 158 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go index 907ab91d..8faf75bd 100644 --- a/mux_httpserver_test.go +++ b/mux_httpserver_test.go @@ -8,9 +8,92 @@ import ( "io/ioutil" "net/http" "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" "testing" ) +// spaHandler implements the http.Handler interface, so we can use it +// to respond to HTTP requests. The path to the static directory and +// path to the index file within that static directory are used to +// serve the SPA in the given static directory. +type spaHandler struct { + staticPath string + indexPath string +} + +// FilepathAbsServeHTTP inspects the URL path to locate a file within the static dir +// on the SPA handler. If a file is found, it will be served. If not, the +// file located at the index path on the SPA handler will be served. This +// is suitable behavior for serving an SPA (single page application). +// This is a negative test case where `filepath.Abs` will return path value like `D:\` +// if our route is `/`. As per docs: Abs returns an absolute representation of path. If the path is not absolute it will be joined with the current working directory to turn it into an absolute path. The absolute path name for a given file is not guaranteed to be unique. Abs calls Clean on the result. +func (h spaHandler) FilepathAbsServeHTTP(w http.ResponseWriter, r *http.Request) { + // get the absolute path to prevent directory traversal + path, err := filepath.Abs(r.URL.Path) + if err != nil { + // if we failed to get the absolute path respond with a 400 bad request + // and stop + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + // prepend the path with the path to the static directory + // path = filepath.Join(h.staticPath, path) + + // check whether a file exists at the given path + _, err = os.Stat(path) + + if os.IsNotExist(err) { + // file does not exist, serve index.html + http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath)) + return + } else if err != nil { + // if we got an error (that wasn't that the file doesn't exist) stating the + // file, return a 500 internal server error and stop + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // otherwise, use http.FileServer to serve the static dir + http.FileServer(http.Dir(h.staticPath)).ServeHTTP(w, r) +} + +// ServeHTTP inspects the URL path to locate a file within the static dir +// on the SPA handler. If a file is found, it will be served. If not, the +// file located at the index path on the SPA handler will be served. This +// is suitable behavior for serving an SPA (single page application). +func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + // get the absolute path to prevent directory traversal + var err error + path := filepath.Join(h.staticPath, r.URL.Path) + if err != nil { + // if we failed to get the absolute path respond with a 400 bad request + // and stop + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + // check whether a file exists at the given path + _, err = os.Stat(path) + + if os.IsNotExist(err) { + // file does not exist, serve index.html + http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath)) + return + } else if err != nil { + // if we got an error (that wasn't that the file doesn't exist) stating the + // file, return a 500 internal server error and stop + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // otherwise, use http.FileServer to serve the static dir + http.FileServer(http.Dir(h.staticPath)).ServeHTTP(w, r) +} + func TestSchemeMatchers(t *testing.T) { router := NewRouter() router.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) { @@ -48,3 +131,78 @@ func TestSchemeMatchers(t *testing.T) { assertResponseBody(t, s, "hello https world") }) } + +func TestServeHttpFilepathAbs(t *testing.T) { + // create a diretory name `build` + os.Mkdir("build", 0700) + + // create a file `index.html` and write below content + htmlContent := []byte(`helloworld`) + err := os.WriteFile("./build/index.html", htmlContent, 0700) + if err != nil { + t.Fatal(err) + } + + // make new request + req, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. + rr := httptest.NewRecorder() + spa := spaHandler{staticPath: "./build", indexPath: "index.html"} + spa.FilepathAbsServeHTTP(rr, req) + + status := rr.Code + if runtime.GOOS != "windows" && status != http.StatusOK { + t.Errorf("handler returned wrong status code: got %v want %v", + status, http.StatusOK) + } else if runtime.GOOS == "windows" && rr.Code != http.StatusInternalServerError { + t.Errorf("handler returned wrong status code in case of windows: got %v want %v", + status, http.StatusOK) + } + + // Check the response body is what we expect. + if runtime.GOOS != "windows" && rr.Body.String() != string(htmlContent) { + t.Errorf("handler returned unexpected body: got %v want %v", + rr.Body.String(), string(htmlContent)) + } else if runtime.GOOS == "windows" && strings.Contains(rr.Body.String(), "syntax is incorrect.") { + t.Errorf("handler returned unexpected body in case of windows: got %v want %v", + rr.Body.String(), string(htmlContent)) + } +} + +func TestServeHttpFilepathJoin(t *testing.T) { + // create a diretory name `build` + os.Mkdir("build", 0700) + + // create a file `index.html` and write below content + htmlContent := []byte(`helloworld`) + err := os.WriteFile("./build/index.html", htmlContent, 0700) + if err != nil { + t.Fatal(err) + } + + // make new request + req, err := http.NewRequest("GET", "/", nil) + if err != nil { + t.Fatal(err) + } + + // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. + rr := httptest.NewRecorder() + spa := spaHandler{staticPath: "./build", indexPath: "index.html"} + spa.ServeHTTP(rr, req) + + if status := rr.Code; status != http.StatusOK { + t.Errorf("handler returned wrong status code: got %v want %v", + status, http.StatusOK) + } + + // Check the response body is what we expect. + if rr.Body.String() != string(htmlContent) { + t.Errorf("handler returned unexpected body: got %v want %v", + rr.Body.String(), string(htmlContent)) + } +} From eee21669a12a642e6ece7ba5f4172acc9f6dd44b Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 23:23:31 +0530 Subject: [PATCH 2/8] fix comments wrap-lines and uncomment code --- mux_httpserver_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go index 8faf75bd..95cccf3e 100644 --- a/mux_httpserver_test.go +++ b/mux_httpserver_test.go @@ -29,7 +29,10 @@ type spaHandler struct { // file located at the index path on the SPA handler will be served. This // is suitable behavior for serving an SPA (single page application). // This is a negative test case where `filepath.Abs` will return path value like `D:\` -// if our route is `/`. As per docs: Abs returns an absolute representation of path. If the path is not absolute it will be joined with the current working directory to turn it into an absolute path. The absolute path name for a given file is not guaranteed to be unique. Abs calls Clean on the result. +// if our route is `/`. As per docs: Abs returns an absolute representation of path. +// If the path is not absolute it will be joined with the current working directory to turn +// it into an absolute path. The absolute path name for a given file is not guaranteed to +// be unique. Abs calls Clean on the result. func (h spaHandler) FilepathAbsServeHTTP(w http.ResponseWriter, r *http.Request) { // get the absolute path to prevent directory traversal path, err := filepath.Abs(r.URL.Path) @@ -41,7 +44,7 @@ func (h spaHandler) FilepathAbsServeHTTP(w http.ResponseWriter, r *http.Request) } // prepend the path with the path to the static directory - // path = filepath.Join(h.staticPath, path) + path = filepath.Join(h.staticPath, path) // check whether a file exists at the given path _, err = os.Stat(path) @@ -66,8 +69,8 @@ func (h spaHandler) FilepathAbsServeHTTP(w http.ResponseWriter, r *http.Request) // file located at the index path on the SPA handler will be served. This // is suitable behavior for serving an SPA (single page application). func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // get the absolute path to prevent directory traversal var err error + // internally calls path.Clean path to prevent directory traversal path := filepath.Join(h.staticPath, r.URL.Path) if err != nil { // if we failed to get the absolute path respond with a 400 bad request From 4a15a9b1d48421c8f47514995acf315d29e02ea5 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 23:37:06 +0530 Subject: [PATCH 3/8] fix condition of test case to check if not contain substring --- mux_httpserver_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go index 95cccf3e..662b3337 100644 --- a/mux_httpserver_test.go +++ b/mux_httpserver_test.go @@ -170,7 +170,7 @@ func TestServeHttpFilepathAbs(t *testing.T) { if runtime.GOOS != "windows" && rr.Body.String() != string(htmlContent) { t.Errorf("handler returned unexpected body: got %v want %v", rr.Body.String(), string(htmlContent)) - } else if runtime.GOOS == "windows" && strings.Contains(rr.Body.String(), "syntax is incorrect.") { + } else if runtime.GOOS == "windows" && !strings.Contains(rr.Body.String(), "syntax is incorrect.") { t.Errorf("handler returned unexpected body in case of windows: got %v want %v", rr.Body.String(), string(htmlContent)) } From e5c7656e774499ed62ea29f5328c050f4fe095a7 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 23:40:15 +0530 Subject: [PATCH 4/8] fix `README.md` `Single Page Application` section to avoid err in windows --- README.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index c7e6872b..8e8b49ad 100644 --- a/README.md +++ b/README.md @@ -251,8 +251,9 @@ type spaHandler struct { // file located at the index path on the SPA handler will be served. This // is suitable behavior for serving an SPA (single page application). func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - // get the absolute path to prevent directory traversal - path, err := filepath.Abs(r.URL.Path) + var err error + // Join internally call path.Clean to prevent directory traversal + path = filepath.Join(h.staticPath, path) if err != nil { // if we failed to get the absolute path respond with a 400 bad request // and stop @@ -260,9 +261,6 @@ func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // prepend the path with the path to the static directory - path = filepath.Join(h.staticPath, path) - // check whether a file exists at the given path _, err = os.Stat(path) if os.IsNotExist(err) { From d0860538d0f5086f614a1f7b0f996774db50dbe6 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 23:44:16 +0530 Subject: [PATCH 5/8] fix indentation `README.md` --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8e8b49ad..121868df 100644 --- a/README.md +++ b/README.md @@ -251,7 +251,7 @@ type spaHandler struct { // file located at the index path on the SPA handler will be served. This // is suitable behavior for serving an SPA (single page application). func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - var err error + var err error // Join internally call path.Clean to prevent directory traversal path = filepath.Join(h.staticPath, path) if err != nil { From 36aa408612796078ba8a4870f45c4b693a041c43 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 1 Jun 2022 23:56:49 +0530 Subject: [PATCH 6/8] replace WriteFile package to fix test case for older go versions --- mux_httpserver_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go index 662b3337..4867415c 100644 --- a/mux_httpserver_test.go +++ b/mux_httpserver_test.go @@ -141,7 +141,7 @@ func TestServeHttpFilepathAbs(t *testing.T) { // create a file `index.html` and write below content htmlContent := []byte(`helloworld`) - err := os.WriteFile("./build/index.html", htmlContent, 0700) + err := ioutil.WriteFile("./build/index.html", htmlContent, 0700) if err != nil { t.Fatal(err) } @@ -182,7 +182,7 @@ func TestServeHttpFilepathJoin(t *testing.T) { // create a file `index.html` and write below content htmlContent := []byte(`helloworld`) - err := os.WriteFile("./build/index.html", htmlContent, 0700) + err := ioutil.WriteFile("./build/index.html", htmlContent, 0700) if err != nil { t.Fatal(err) } From d308dd34c56e9c7f5095538b44d5dbfee3b4be28 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Fri, 3 Jun 2022 21:01:57 +0530 Subject: [PATCH 7/8] remove err condtion after filepath.Join statement --- README.md | 9 +-- mux_httpserver_test.go | 161 ----------------------------------------- 2 files changed, 1 insertion(+), 169 deletions(-) diff --git a/README.md b/README.md index 121868df..f9d09c31 100644 --- a/README.md +++ b/README.md @@ -251,18 +251,11 @@ type spaHandler struct { // file located at the index path on the SPA handler will be served. This // is suitable behavior for serving an SPA (single page application). func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - var err error // Join internally call path.Clean to prevent directory traversal path = filepath.Join(h.staticPath, path) - if err != nil { - // if we failed to get the absolute path respond with a 400 bad request - // and stop - http.Error(w, err.Error(), http.StatusBadRequest) - return - } // check whether a file exists at the given path - _, err = os.Stat(path) + _, err := os.Stat(path) if os.IsNotExist(err) { // file does not exist, serve index.html http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath)) diff --git a/mux_httpserver_test.go b/mux_httpserver_test.go index 4867415c..907ab91d 100644 --- a/mux_httpserver_test.go +++ b/mux_httpserver_test.go @@ -8,95 +8,9 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "os" - "path/filepath" - "runtime" - "strings" "testing" ) -// spaHandler implements the http.Handler interface, so we can use it -// to respond to HTTP requests. The path to the static directory and -// path to the index file within that static directory are used to -// serve the SPA in the given static directory. -type spaHandler struct { - staticPath string - indexPath string -} - -// FilepathAbsServeHTTP inspects the URL path to locate a file within the static dir -// on the SPA handler. If a file is found, it will be served. If not, the -// file located at the index path on the SPA handler will be served. This -// is suitable behavior for serving an SPA (single page application). -// This is a negative test case where `filepath.Abs` will return path value like `D:\` -// if our route is `/`. As per docs: Abs returns an absolute representation of path. -// If the path is not absolute it will be joined with the current working directory to turn -// it into an absolute path. The absolute path name for a given file is not guaranteed to -// be unique. Abs calls Clean on the result. -func (h spaHandler) FilepathAbsServeHTTP(w http.ResponseWriter, r *http.Request) { - // get the absolute path to prevent directory traversal - path, err := filepath.Abs(r.URL.Path) - if err != nil { - // if we failed to get the absolute path respond with a 400 bad request - // and stop - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - - // prepend the path with the path to the static directory - path = filepath.Join(h.staticPath, path) - - // check whether a file exists at the given path - _, err = os.Stat(path) - - if os.IsNotExist(err) { - // file does not exist, serve index.html - http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath)) - return - } else if err != nil { - // if we got an error (that wasn't that the file doesn't exist) stating the - // file, return a 500 internal server error and stop - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - // otherwise, use http.FileServer to serve the static dir - http.FileServer(http.Dir(h.staticPath)).ServeHTTP(w, r) -} - -// ServeHTTP inspects the URL path to locate a file within the static dir -// on the SPA handler. If a file is found, it will be served. If not, the -// file located at the index path on the SPA handler will be served. This -// is suitable behavior for serving an SPA (single page application). -func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { - var err error - // internally calls path.Clean path to prevent directory traversal - path := filepath.Join(h.staticPath, r.URL.Path) - if err != nil { - // if we failed to get the absolute path respond with a 400 bad request - // and stop - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - - // check whether a file exists at the given path - _, err = os.Stat(path) - - if os.IsNotExist(err) { - // file does not exist, serve index.html - http.ServeFile(w, r, filepath.Join(h.staticPath, h.indexPath)) - return - } else if err != nil { - // if we got an error (that wasn't that the file doesn't exist) stating the - // file, return a 500 internal server error and stop - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - // otherwise, use http.FileServer to serve the static dir - http.FileServer(http.Dir(h.staticPath)).ServeHTTP(w, r) -} - func TestSchemeMatchers(t *testing.T) { router := NewRouter() router.HandleFunc("/", func(rw http.ResponseWriter, r *http.Request) { @@ -134,78 +48,3 @@ func TestSchemeMatchers(t *testing.T) { assertResponseBody(t, s, "hello https world") }) } - -func TestServeHttpFilepathAbs(t *testing.T) { - // create a diretory name `build` - os.Mkdir("build", 0700) - - // create a file `index.html` and write below content - htmlContent := []byte(`helloworld`) - err := ioutil.WriteFile("./build/index.html", htmlContent, 0700) - if err != nil { - t.Fatal(err) - } - - // make new request - req, err := http.NewRequest("GET", "/", nil) - if err != nil { - t.Fatal(err) - } - - // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. - rr := httptest.NewRecorder() - spa := spaHandler{staticPath: "./build", indexPath: "index.html"} - spa.FilepathAbsServeHTTP(rr, req) - - status := rr.Code - if runtime.GOOS != "windows" && status != http.StatusOK { - t.Errorf("handler returned wrong status code: got %v want %v", - status, http.StatusOK) - } else if runtime.GOOS == "windows" && rr.Code != http.StatusInternalServerError { - t.Errorf("handler returned wrong status code in case of windows: got %v want %v", - status, http.StatusOK) - } - - // Check the response body is what we expect. - if runtime.GOOS != "windows" && rr.Body.String() != string(htmlContent) { - t.Errorf("handler returned unexpected body: got %v want %v", - rr.Body.String(), string(htmlContent)) - } else if runtime.GOOS == "windows" && !strings.Contains(rr.Body.String(), "syntax is incorrect.") { - t.Errorf("handler returned unexpected body in case of windows: got %v want %v", - rr.Body.String(), string(htmlContent)) - } -} - -func TestServeHttpFilepathJoin(t *testing.T) { - // create a diretory name `build` - os.Mkdir("build", 0700) - - // create a file `index.html` and write below content - htmlContent := []byte(`helloworld`) - err := ioutil.WriteFile("./build/index.html", htmlContent, 0700) - if err != nil { - t.Fatal(err) - } - - // make new request - req, err := http.NewRequest("GET", "/", nil) - if err != nil { - t.Fatal(err) - } - - // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. - rr := httptest.NewRecorder() - spa := spaHandler{staticPath: "./build", indexPath: "index.html"} - spa.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - t.Errorf("handler returned wrong status code: got %v want %v", - status, http.StatusOK) - } - - // Check the response body is what we expect. - if rr.Body.String() != string(htmlContent) { - t.Errorf("handler returned unexpected body: got %v want %v", - rr.Body.String(), string(htmlContent)) - } -} From a3df0f4e4ba0646aa3dc070105abc44d272f5039 Mon Sep 17 00:00:00 2001 From: Mustaque Ahmed Date: Wed, 10 Aug 2022 22:09:42 +0530 Subject: [PATCH 8/8] fix `path` var initialisation --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f9d09c31..0daef68b 100644 --- a/README.md +++ b/README.md @@ -252,7 +252,7 @@ type spaHandler struct { // is suitable behavior for serving an SPA (single page application). func (h spaHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Join internally call path.Clean to prevent directory traversal - path = filepath.Join(h.staticPath, path) + path := filepath.Join(h.staticPath, path) // check whether a file exists at the given path _, err := os.Stat(path)