-
Notifications
You must be signed in to change notification settings - Fork 880
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
* fix: correct mimetype is returned
fix: when request /index.html, base path was not modified
fix: wrong 'err' variable used when log.Errorf("Failed to stat file or dir %s: %v"...) was printed
change: when a file is not found, 404 is return (before: the index.html was returned)
add: tests for static files serving
Signed-off-by: nitram509 <[email protected]>
* fix sonar cloud complains about invalid HTML
See https://sonarcloud.io/project/issues?resolved=false&types=BUG&pullRequest=2303&id=argoproj_argo-rollouts&open=AYO5y8lxtb83AIZrmShZ
Signed-off-by: nitram509 <[email protected]>
* fix send index.html when page not found, because client side React UI router
Signed-off-by: nitram509 <[email protected]>
* make variable private (feedback from review)
Signed-off-by: nitram509 <[email protected]>
Signed-off-by: nitram509 <[email protected]>- Loading branch information
Showing
5 changed files
with
227 additions
and
113 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "embed" | ||
| "errors" | ||
| "io/fs" | ||
| "mime" | ||
| "net/http" | ||
| "path" | ||
| "regexp" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| log "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| var ( | ||
| //go:embed static/* | ||
| static embed.FS //nolint | ||
| staticBasePath = "static" | ||
| indexHtmlFile = staticBasePath + "/index.html" | ||
| ) | ||
|
|
||
| const ( | ||
| ContentType = "Content-Type" | ||
| ContentLength = "Content-Length" | ||
| ) | ||
|
|
||
| func (s *ArgoRolloutsServer) staticFileHttpHandler(w http.ResponseWriter, r *http.Request) { | ||
| requestedURI := path.Clean(r.RequestURI) | ||
| rootPath := path.Clean("/" + s.Options.RootPath) | ||
|
|
||
| if requestedURI == "/" { | ||
| http.Redirect(w, r, rootPath+"/", http.StatusFound) | ||
| return | ||
| } | ||
|
|
||
| //If the rootPath is not in the prefix 404 | ||
| if !strings.HasPrefix(requestedURI, rootPath) { | ||
| http.NotFound(w, r) | ||
| return | ||
| } | ||
|
|
||
| embedPath := path.Join(staticBasePath, strings.TrimPrefix(requestedURI, rootPath)) | ||
|
|
||
| //If the rootPath is the requestedURI, serve index.html | ||
| if requestedURI == rootPath { | ||
| embedPath = indexHtmlFile | ||
| } | ||
|
|
||
| fileBytes, err := static.ReadFile(embedPath) | ||
| if err != nil { | ||
| if fileNotExistsOrIsDirectoryError(err) { | ||
| // send index.html, because UI will use path based router in React | ||
| fileBytes, _ = static.ReadFile(indexHtmlFile) | ||
| embedPath = indexHtmlFile | ||
| } else { | ||
| log.Errorf("Error reading file %s: %v", embedPath, err) | ||
| w.WriteHeader(http.StatusInternalServerError) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| if embedPath == indexHtmlFile { | ||
| fileBytes = withRootPath(fileBytes, s.Options.RootPath) | ||
| } | ||
|
|
||
| w.Header().Set(ContentType, determineMimeType(embedPath)) | ||
| w.Header().Set(ContentLength, strconv.Itoa(len(fileBytes))) | ||
| w.WriteHeader(http.StatusOK) | ||
| _, err = w.Write(fileBytes) | ||
| if err != nil { | ||
| log.Errorf("Error writing response: %v", err) | ||
| } | ||
| } | ||
|
|
||
| func fileNotExistsOrIsDirectoryError(err error) bool { | ||
| if errors.Is(err, fs.ErrNotExist) { | ||
| return true | ||
| } | ||
| pathErr, isPathError := err.(*fs.PathError) | ||
| return isPathError && strings.Contains(pathErr.Error(), "is a directory") | ||
| } | ||
|
|
||
| func determineMimeType(fileName string) string { | ||
| idx := strings.LastIndex(fileName, ".") | ||
| if idx >= 0 { | ||
| mimeType := mime.TypeByExtension(fileName[idx:]) | ||
| if len(mimeType) > 0 { | ||
| return mimeType | ||
| } | ||
| } | ||
| return "text/plain" | ||
| } | ||
|
|
||
| var re = regexp.MustCompile(`<base href=".*".*/>`) | ||
|
|
||
| func withRootPath(fileContent []byte, rootpath string) []byte { | ||
| var temp = re.ReplaceAllString(string(fileContent), `<base href="`+path.Clean("/"+rootpath)+`/" />`) | ||
| return []byte(temp) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| package server | ||
|
|
||
| import ( | ||
| "embed" | ||
| "io" | ||
| "mime" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/tj/assert" | ||
| ) | ||
|
|
||
| const TestRootPath = "/test-root" | ||
|
|
||
| var ( | ||
| //go:embed static_test/* | ||
| staticTestData embed.FS //nolint | ||
| mockServer ArgoRolloutsServer | ||
| ) | ||
|
|
||
| func init() { | ||
| static = staticTestData | ||
| staticBasePath = "static_test" | ||
| indexHtmlFile = staticBasePath + "/index.html" | ||
| mockServer = mockArgoRolloutServer() | ||
| } | ||
|
|
||
| func TestIndexHtmlIsServed(t *testing.T) { | ||
| tests := []struct { | ||
| requestPath string | ||
| }{ | ||
| {TestRootPath + "/"}, | ||
| {TestRootPath + "/index.html"}, | ||
| {TestRootPath + "/nonsense/../index.html"}, | ||
| {TestRootPath + "/test-dir/test.css"}, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.requestPath, func(t *testing.T) { | ||
| req := httptest.NewRequest(http.MethodGet, test.requestPath, nil) | ||
| w := httptest.NewRecorder() | ||
| mockServer.staticFileHttpHandler(w, req) | ||
| res := w.Result() | ||
| defer res.Body.Close() | ||
| data, err := io.ReadAll(res.Body) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, res.StatusCode, http.StatusOK) | ||
| if strings.HasSuffix(test.requestPath, ".css") { | ||
| assert.Equal(t, res.Header.Get(ContentType), mime.TypeByExtension(".css")) | ||
| assert.Contains(t, string(data), "empty by intent") | ||
| } else { | ||
| assert.Equal(t, res.Header.Get(ContentType), mime.TypeByExtension(".html")) | ||
| assert.Contains(t, string(data), "<title>index-title</title>") | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestWhenFileNotFoundSendIndexPageForUiReactRouter(t *testing.T) { | ||
| req := httptest.NewRequest(http.MethodGet, TestRootPath+"/namespace-default", nil) | ||
| w := httptest.NewRecorder() | ||
| mockServer.staticFileHttpHandler(w, req) | ||
| res := w.Result() | ||
| defer res.Body.Close() | ||
| data, err := io.ReadAll(res.Body) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, res.StatusCode, http.StatusOK) | ||
| assert.Contains(t, string(data), "<title>index-title</title>") | ||
| } | ||
|
|
||
| func TestSlashWillBeRedirectedToRootPath(t *testing.T) { | ||
| req := httptest.NewRequest(http.MethodGet, "/", nil) | ||
| w := httptest.NewRecorder() | ||
| mockServer.staticFileHttpHandler(w, req) | ||
| res := w.Result() | ||
| defer res.Body.Close() | ||
| _, err := io.ReadAll(res.Body) | ||
| assert.NoError(t, err) | ||
| assert.Equal(t, res.StatusCode, http.StatusFound) | ||
| assert.Contains(t, res.Header.Get("Location"), TestRootPath) | ||
| } | ||
|
|
||
| func TestInvalidFilesOrHackingAttemptReturn404(t *testing.T) { | ||
| tests := []struct { | ||
| requestPath string | ||
| }{ | ||
| {"/index.html"}, // should fail, because not prefixed with Option.RootPath | ||
| {"/etc/passwd"}, | ||
| {TestRootPath + "/../etc/passwd"}, | ||
| {TestRootPath + "/../../etc/passwd"}, | ||
| {TestRootPath + "/../../../etc/passwd"}, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.requestPath, func(t *testing.T) { | ||
| req := httptest.NewRequest(http.MethodGet, test.requestPath, nil) | ||
| w := httptest.NewRecorder() | ||
| mockServer.staticFileHttpHandler(w, req) | ||
| res := w.Result() | ||
| defer res.Body.Close() | ||
| assert.Equal(t, res.StatusCode, http.StatusNotFound) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func mockArgoRolloutServer() ArgoRolloutsServer { | ||
| s := ArgoRolloutsServer{ | ||
| Options: ServerOptions{ | ||
| RootPath: TestRootPath, | ||
| }, | ||
| } | ||
| return s | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <title>index-title</title> | ||
| </head> | ||
| <body> | ||
| index-body | ||
| </body> | ||
| </html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| /* empty by intent */ |