Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update routing #670

Merged
merged 4 commits into from
Jan 26, 2024
Merged

Update routing #670

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd8e52d
Update routing
CannonLock Jan 17, 2024
7512fc4
Clean path and Serve 404 file
CannonLock Jan 26, 2024
c9c07a7
Fix extra whitespace
haoming29 Jan 26, 2024
af80bf9
Reorder web asset redirect handler
haoming29 Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion web_ui/ui.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,19 @@ func configureWebResource(engine *gin.Engine) error {
engine.GET("/view/*path", func(ctx *gin.Context) {
path := ctx.Param("path")

// If the path is a directory indicate that we are looking for the index.html file
if strings.HasSuffix(path, "/") {
path += "index.html"
}

// 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+"/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be http.StatusMovedPermanently instead. As Go http server did in here

Copy link
Contributor

@haoming29 haoming29 Jan 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus, for the path parameter, we might want to clean it first to prevent directory traversal attack:

safePath := path.Clean("/" + pathParam)

return
}
}

db := authDB.Load()
user, err := GetUser(ctx)

Expand Down Expand Up @@ -137,12 +146,20 @@ func configureWebResource(engine *gin.Engine) error {

// If just one server is enabled, redirect to that server
if len(config.GetEnabledServerString(true)) == 1 && path == "/index.html" {
ctx.Redirect(http.StatusFound, "/view/"+config.GetEnabledServerString(true)[0]+"/index.html")
ctx.Redirect(http.StatusFound, "/view/"+config.GetEnabledServerString(true)[0]+"/")
return
}

filePath := "frontend/out" + path
file, _ := webAssets.ReadFile(filePath)

// If the file is not found, return 404
if file == nil {
ctx.AbortWithStatus(http.StatusNotFound)
return
}

// If the file is found, return the file
ctx.Data(
http.StatusOK,
mime.TypeByExtension(filePath),
Expand Down
Loading