From 6876fd6074655382ac4e1d81b0bfa573b4119d09 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 18 Oct 2020 02:29:06 +0100 Subject: [PATCH 1/3] When handling errors in storageHandler check underlying error (#13178) 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 --- modules/storage/minio.go | 2 +- routers/routes/routes.go | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/modules/storage/minio.go b/modules/storage/minio.go index eb43fa96ee040..88416c3522de4 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 diff --git a/routers/routes/routes.go b/routers/routes/routes.go index a09e53efc1f89..0ca172faecb63 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -7,6 +7,8 @@ package routes import ( "bytes" "encoding/gob" + "errors" + "fmt" "io" "net/http" "path" @@ -125,7 +127,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 +160,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() From 07b7a936f2a63489f94116bade04a9f7ca7d77b7 Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 17 Oct 2020 21:54:28 -0400 Subject: [PATCH 2/3] add missing convertMinioErr func --- modules/storage/minio.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 88416c3522de4..b65af8ddbdef9 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -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) From 9d03ca30ddc3752f0634ddb78f08bf30b6f50efa Mon Sep 17 00:00:00 2001 From: Matti R Date: Sat, 17 Oct 2020 21:59:58 -0400 Subject: [PATCH 3/3] add missing import --- routers/routes/routes.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/routes/routes.go b/routers/routes/routes.go index 0ca172faecb63..135c6b56a8ba8 100644 --- a/routers/routes/routes.go +++ b/routers/routes/routes.go @@ -11,6 +11,7 @@ import ( "fmt" "io" "net/http" + "os" "path" "strings" "text/template"