From aca13f941ccfec998eed506c256a0d72b26e64dd Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 18 Oct 2020 10:52:03 -0400 Subject: [PATCH] When handling errors in storageHandler check underlying error (#13178) (#13193) Unfortunately there was a mistake in #13164 which fails to handle os.PathError wrapping an os.ErrNotExist Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick Co-authored-by: zeripath --- modules/storage/minio.go | 22 +++++++++++++++++++++- routers/routes/routes.go | 19 +++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/modules/storage/minio.go b/modules/storage/minio.go index eb43fa96ee..b65af8ddbd 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -30,7 +30,7 @@ type minioObject struct { func (m *minioObject) Stat() (os.FileInfo, error) { oi, err := m.Object.Stat() if err != nil { - return nil, err + return nil, convertMinioErr(err) } return &minioFileInfo{oi}, nil @@ -58,6 +58,26 @@ type MinioStorage struct { basePath string } +func convertMinioErr(err error) error { + if err == nil { + return nil + } + errResp, ok := err.(minio.ErrorResponse) + if !ok { + return err + } + + // Convert two responses to standard analogues + switch errResp.Code { + case "NoSuchKey": + return os.ErrNotExist + case "AccessDenied": + return os.ErrPermission + } + + return err +} + // NewMinioStorage returns a minio storage func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) { configInterface, err := toConfig(MinioStorageConfig{}, cfg) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index a09e53efc1..135c6b56a8 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -7,8 +7,11 @@ package routes import ( "bytes" "encoding/gob" + "errors" + "fmt" "io" "net/http" + "os" "path" "strings" "text/template" @@ -125,7 +128,13 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor rPath := strings.TrimPrefix(req.RequestURI, "/"+prefix) u, err := objStore.URL(rPath, path.Base(rPath)) if err != nil { - ctx.Error(500, err.Error()) + if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { + log.Warn("Unable to find %s %s", prefix, rPath) + ctx.Error(404, "file not found") + return + } + log.Error("Error whilst getting URL for %s %s. Error: %v", prefix, rPath, err) + ctx.Error(500, fmt.Sprintf("Error whilst getting URL for %s %s", prefix, rPath)) return } http.Redirect( @@ -152,7 +161,13 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor //If we have matched and access to release or issue fr, err := objStore.Open(rPath) if err != nil { - ctx.Error(500, err.Error()) + if os.IsNotExist(err) || errors.Is(err, os.ErrNotExist) { + log.Warn("Unable to find %s %s", prefix, rPath) + ctx.Error(404, "file not found") + return + } + log.Error("Error whilst opening %s %s. Error: %v", prefix, rPath, err) + ctx.Error(500, fmt.Sprintf("Error whilst opening %s %s", prefix, rPath)) return } defer fr.Close()