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.

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

Fix panic in storageHandler #27446

Merged
merged 3 commits into from
Oct 6, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
Fix panic in storageHandler
storageHandler() is written as a middleware but is used as an endpoint
handler, and thus `next` is actually `nil`, which causes a null pointer
dereference when a request URL does not match the pattern (where it
calls `next.ServerHTTP()`).

Example CURL command to trigger the panic:

```
curl -I "http:https://yourhost/gitea//avatars/a"
```

Fixes #27409
  • Loading branch information
sryze committed Oct 5, 2023
commit 3e3ae4a9dee56f9b15fd77930a389f372d4489fe
99 changes: 49 additions & 50 deletions routers/web/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,81 +19,80 @@ import (
"code.gitea.io/gitea/modules/web/routing"
)

func storageHandler(storageSetting *setting.Storage, prefix string, objStore storage.ObjectStorage) func(next http.Handler) http.Handler {
func storageHandler(storageSetting *setting.Storage, prefix string, objStore storage.ObjectStorage) http.HandlerFunc {
prefix = strings.Trim(prefix, "/")
funcInfo := routing.GetFuncInfo(storageHandler, prefix)
return func(next http.Handler) http.Handler {
if storageSetting.MinioConfig.ServeDirect {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.Method != "GET" && req.Method != "HEAD" {
next.ServeHTTP(w, req)
return
}

if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") {
next.ServeHTTP(w, req)
return
}
routing.UpdateFuncInfo(req.Context(), funcInfo)

rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.PathJoinRelX(rPath)

u, err := objStore.URL(rPath, path.Base(rPath))
if err != nil {
if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
log.Warn("Unable to find %s %s", prefix, rPath)
http.Error(w, "file not found", http.StatusNotFound)
return
}
log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath), http.StatusInternalServerError)
return
}

http.Redirect(w, req, u.String(), http.StatusTemporaryRedirect)
})
}

if storageSetting.MinioConfig.ServeDirect {
return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.Method != "GET" && req.Method != "HEAD" {
next.ServeHTTP(w, req)
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
}

if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") {
next.ServeHTTP(w, req)
http.Error(w, "file not found", http.StatusNotFound)
return
}
routing.UpdateFuncInfo(req.Context(), funcInfo)

rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.PathJoinRelX(rPath)
if rPath == "" || rPath == "." {
http.Error(w, "file not found", http.StatusNotFound)
return
}

fi, err := objStore.Stat(rPath)
u, err := objStore.URL(rPath, path.Base(rPath))
if err != nil {
if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
log.Warn("Unable to find %s %s", prefix, rPath)
http.Error(w, "file not found", http.StatusNotFound)
return
}
log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath), http.StatusInternalServerError)
return
}

fr, err := objStore.Open(rPath)
if err != nil {
log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
return
}
defer fr.Close()
httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), fi.ModTime(), fr)
http.Redirect(w, req, u.String(), http.StatusTemporaryRedirect)
})
}

return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
if req.Method != "GET" && req.Method != "HEAD" {
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
return
}

if !strings.HasPrefix(req.URL.Path, "/"+prefix+"/") {
http.Error(w, "file not found", http.StatusNotFound)
return
}
routing.UpdateFuncInfo(req.Context(), funcInfo)

rPath := strings.TrimPrefix(req.URL.Path, "/"+prefix+"/")
rPath = util.PathJoinRelX(rPath)
if rPath == "" || rPath == "." {
http.Error(w, "file not found", http.StatusNotFound)
return
}

fi, err := objStore.Stat(rPath)
if err != nil {
if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) {
log.Warn("Unable to find %s %s", prefix, rPath)
http.Error(w, "file not found", http.StatusNotFound)
return
}
log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
return
}

fr, err := objStore.Open(rPath)
if err != nil {
log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err)
http.Error(w, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath), http.StatusInternalServerError)
return
}
defer fr.Close()
httpcache.ServeContentWithCacheControl(w, req, path.Base(rPath), fi.ModTime(), fr)
})
}