Update routing #670
Update routing #670
Conversation
CannonLock
commented
Jan 17, 2024
CannonLock
commented
- /path -> 301 Redirect -> /path/index.html if exists
- /path that doesn't exists returns 404
- Closes Forward /path to /path/ if /path/index.html exists #653
- /path -> 301 Redirect -> /path/index.html if exists - /path that doesn't exists returns 404
web_ui/ui.go
Outdated
| // If path doesn't have extension, is not a directory, and has a index file, redirect to index file | ||
| if !strings.Contains(path, ".") && !strings.HasSuffix(path, "/") { | ||
| if _, err := webAssets.ReadFile("frontend/out" + path + "/index.html"); err == nil { | ||
| ctx.Redirect(http.StatusFound, "/view/"+path+"/") |
There was a problem hiding this comment.
This should be http.StatusMovedPermanently instead. As Go http server did in here
There was a problem hiding this comment.
I'm actually a bit confused here. I thought we should not redirect user to index.html if their path has / suffix, like /view/login/ but we should append index.html to the user path internally to find the correct file whenever there's only a / ?
For requests without a trailing slash, and is not a file name, like /view/login, we want to permanently redirect them to /view/login/.
For requests with a valid file name (that webAssets.ReadFile gives a positive answer), we can serve the file directly.
This way, the path is cleaner and more readable for users but we are still able to serve *index.html file which won't hurt SEO.
There was a problem hiding this comment.
Plus, for the path parameter, we might want to clean it first to prevent directory traversal attack:
safePath := path.Clean("/" + pathParam)
|
LGTM with the latest commit. I fixed a format issue and reordered the "direct-to-single-server" logic so that when visiting registry from |
