Prevent panics with missing storage (#13164)
* The `.Use` of storageHandler before setting up the template renderer causes a panic if there is an error to log. * The error passed to `ctx.Error` in that case may contain sensitive information and should not be rendered to the end user. We should instead log the error and render a simple error message. * There is no handling of missing avatars and this needs a 404. Minio errors need to be mapped to standard golang errors such as os.ErrNotExist. * There is no logging when storage is set up. Related #13159 Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
parent
cb171dbd56
commit
91f2afdb54
4 changed files with 64 additions and 19 deletions
|
@ -11,6 +11,7 @@ import (
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/log"
|
||||||
"code.gitea.io/gitea/modules/util"
|
"code.gitea.io/gitea/modules/util"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -40,6 +41,7 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
|
||||||
}
|
}
|
||||||
config := configInterface.(LocalStorageConfig)
|
config := configInterface.(LocalStorageConfig)
|
||||||
|
|
||||||
|
log.Info("Creating new Local Storage at %s", config.Path)
|
||||||
if err := os.MkdirAll(config.Path, os.ModePerm); err != nil {
|
if err := os.MkdirAll(config.Path, os.ModePerm); err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
|
@ -13,6 +13,8 @@ import (
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/log"
|
||||||
|
|
||||||
"github.com/minio/minio-go/v7"
|
"github.com/minio/minio-go/v7"
|
||||||
"github.com/minio/minio-go/v7/pkg/credentials"
|
"github.com/minio/minio-go/v7/pkg/credentials"
|
||||||
)
|
)
|
||||||
|
@ -58,20 +60,42 @@ type MinioStorage struct {
|
||||||
basePath string
|
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
|
// NewMinioStorage returns a minio storage
|
||||||
func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
|
func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error) {
|
||||||
configInterface, err := toConfig(MinioStorageConfig{}, cfg)
|
configInterface, err := toConfig(MinioStorageConfig{}, cfg)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
config := configInterface.(MinioStorageConfig)
|
config := configInterface.(MinioStorageConfig)
|
||||||
|
|
||||||
|
log.Info("Creating Minio storage at %s:%s with base path %s", config.Endpoint, config.Bucket, config.BasePath)
|
||||||
|
|
||||||
minioClient, err := minio.New(config.Endpoint, &minio.Options{
|
minioClient, err := minio.New(config.Endpoint, &minio.Options{
|
||||||
Creds: credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""),
|
Creds: credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, ""),
|
||||||
Secure: config.UseSSL,
|
Secure: config.UseSSL,
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{
|
if err := minioClient.MakeBucket(ctx, config.Bucket, minio.MakeBucketOptions{
|
||||||
|
@ -80,7 +104,7 @@ func NewMinioStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
|
||||||
// Check to see if we already own this bucket (which happens if you run this twice)
|
// Check to see if we already own this bucket (which happens if you run this twice)
|
||||||
exists, errBucketExists := minioClient.BucketExists(ctx, config.Bucket)
|
exists, errBucketExists := minioClient.BucketExists(ctx, config.Bucket)
|
||||||
if !exists || errBucketExists != nil {
|
if !exists || errBucketExists != nil {
|
||||||
return nil, err
|
return nil, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -101,7 +125,7 @@ func (m *MinioStorage) Open(path string) (Object, error) {
|
||||||
var opts = minio.GetObjectOptions{}
|
var opts = minio.GetObjectOptions{}
|
||||||
object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts)
|
object, err := m.client.GetObject(m.ctx, m.bucket, m.buildMinioPath(path), opts)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, err
|
return nil, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
return &minioObject{object}, nil
|
return &minioObject{object}, nil
|
||||||
}
|
}
|
||||||
|
@ -117,7 +141,7 @@ func (m *MinioStorage) Save(path string, r io.Reader) (int64, error) {
|
||||||
minio.PutObjectOptions{ContentType: "application/octet-stream"},
|
minio.PutObjectOptions{ContentType: "application/octet-stream"},
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return 0, err
|
return 0, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
return uploadInfo.Size, nil
|
return uploadInfo.Size, nil
|
||||||
}
|
}
|
||||||
|
@ -159,19 +183,16 @@ func (m *MinioStorage) Stat(path string) (os.FileInfo, error) {
|
||||||
minio.StatObjectOptions{},
|
minio.StatObjectOptions{},
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if errResp, ok := err.(minio.ErrorResponse); ok {
|
return nil, convertMinioErr(err)
|
||||||
if errResp.Code == "NoSuchKey" {
|
|
||||||
return nil, os.ErrNotExist
|
|
||||||
}
|
|
||||||
}
|
|
||||||
return nil, err
|
|
||||||
}
|
}
|
||||||
return &minioFileInfo{info}, nil
|
return &minioFileInfo{info}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Delete delete a file
|
// Delete delete a file
|
||||||
func (m *MinioStorage) Delete(path string) error {
|
func (m *MinioStorage) Delete(path string) error {
|
||||||
return m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{})
|
err := m.client.RemoveObject(m.ctx, m.bucket, m.buildMinioPath(path), minio.RemoveObjectOptions{})
|
||||||
|
|
||||||
|
return convertMinioErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
|
// URL gets the redirect URL to a file. The presigned link is valid for 5 minutes.
|
||||||
|
@ -179,7 +200,8 @@ func (m *MinioStorage) URL(path, name string) (*url.URL, error) {
|
||||||
reqParams := make(url.Values)
|
reqParams := make(url.Values)
|
||||||
// TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we?
|
// TODO it may be good to embed images with 'inline' like ServeData does, but we don't want to have to read the file, do we?
|
||||||
reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"")
|
reqParams.Set("response-content-disposition", "attachment; filename=\""+quoteEscaper.Replace(name)+"\"")
|
||||||
return m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)
|
u, err := m.client.PresignedGetObject(m.ctx, m.bucket, m.buildMinioPath(path), 5*time.Minute, reqParams)
|
||||||
|
return u, convertMinioErr(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// IterateObjects iterates across the objects in the miniostorage
|
// IterateObjects iterates across the objects in the miniostorage
|
||||||
|
@ -193,13 +215,13 @@ func (m *MinioStorage) IterateObjects(fn func(path string, obj Object) error) er
|
||||||
}) {
|
}) {
|
||||||
object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts)
|
object, err := m.client.GetObject(lobjectCtx, m.bucket, mObjInfo.Key, opts)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return convertMinioErr(err)
|
||||||
}
|
}
|
||||||
if err := func(object *minio.Object, fn func(path string, obj Object) error) error {
|
if err := func(object *minio.Object, fn func(path string, obj Object) error) error {
|
||||||
defer object.Close()
|
defer object.Close()
|
||||||
return fn(strings.TrimPrefix(m.basePath, mObjInfo.Key), &minioObject{object})
|
return fn(strings.TrimPrefix(m.basePath, mObjInfo.Key), &minioObject{object})
|
||||||
}(object, fn); err != nil {
|
}(object, fn); err != nil {
|
||||||
return err
|
return convertMinioErr(err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
|
|
|
@ -12,6 +12,7 @@ import (
|
||||||
"net/url"
|
"net/url"
|
||||||
"os"
|
"os"
|
||||||
|
|
||||||
|
"code.gitea.io/gitea/modules/log"
|
||||||
"code.gitea.io/gitea/modules/setting"
|
"code.gitea.io/gitea/modules/setting"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
@ -141,21 +142,25 @@ func NewStorage(typStr string, cfg interface{}) (ObjectStorage, error) {
|
||||||
}
|
}
|
||||||
|
|
||||||
func initAvatars() (err error) {
|
func initAvatars() (err error) {
|
||||||
|
log.Info("Initialising Avatar storage with type: %s", setting.Avatar.Storage.Type)
|
||||||
Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage)
|
Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
func initAttachments() (err error) {
|
func initAttachments() (err error) {
|
||||||
|
log.Info("Initialising Attachment storage with type: %s", setting.Attachment.Storage.Type)
|
||||||
Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage)
|
Attachments, err = NewStorage(setting.Attachment.Storage.Type, setting.Attachment.Storage)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
func initLFS() (err error) {
|
func initLFS() (err error) {
|
||||||
|
log.Info("Initialising LFS storage with type: %s", setting.LFS.Storage.Type)
|
||||||
LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage)
|
LFS, err = NewStorage(setting.LFS.Storage.Type, setting.LFS.Storage)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
func initRepoAvatars() (err error) {
|
func initRepoAvatars() (err error) {
|
||||||
|
log.Info("Initialising Repository Avatar storage with type: %s", setting.RepoAvatar.Storage.Type)
|
||||||
RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, setting.RepoAvatar.Storage)
|
RepoAvatars, err = NewStorage(setting.RepoAvatar.Storage.Type, setting.RepoAvatar.Storage)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
|
@ -7,8 +7,10 @@ package routes
|
||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"encoding/gob"
|
"encoding/gob"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"os"
|
||||||
"path"
|
"path"
|
||||||
"strings"
|
"strings"
|
||||||
"text/template"
|
"text/template"
|
||||||
|
@ -125,7 +127,13 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
|
||||||
rPath := strings.TrimPrefix(req.RequestURI, "/"+prefix)
|
rPath := strings.TrimPrefix(req.RequestURI, "/"+prefix)
|
||||||
u, err := objStore.URL(rPath, path.Base(rPath))
|
u, err := objStore.URL(rPath, path.Base(rPath))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ctx.Error(500, err.Error())
|
if 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
|
return
|
||||||
}
|
}
|
||||||
http.Redirect(
|
http.Redirect(
|
||||||
|
@ -152,14 +160,21 @@ func storageHandler(storageSetting setting.Storage, prefix string, objStore stor
|
||||||
//If we have matched and access to release or issue
|
//If we have matched and access to release or issue
|
||||||
fr, err := objStore.Open(rPath)
|
fr, err := objStore.Open(rPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ctx.Error(500, err.Error())
|
if 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
|
return
|
||||||
}
|
}
|
||||||
defer fr.Close()
|
defer fr.Close()
|
||||||
|
|
||||||
_, err = io.Copy(ctx.Resp, fr)
|
_, err = io.Copy(ctx.Resp, fr)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
ctx.Error(500, err.Error())
|
log.Error("Error whilst rendering %s %s. Error: %v", prefix, rPath, err)
|
||||||
|
ctx.Error(500, fmt.Sprintf("Error whilst rendering %s %s", prefix, rPath))
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -208,10 +223,11 @@ func NewMacaron() *macaron.Macaron {
|
||||||
},
|
},
|
||||||
))
|
))
|
||||||
|
|
||||||
|
m.Use(templates.HTMLRenderer())
|
||||||
|
|
||||||
m.Use(storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
|
m.Use(storageHandler(setting.Avatar.Storage, "avatars", storage.Avatars))
|
||||||
m.Use(storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
|
m.Use(storageHandler(setting.RepoAvatar.Storage, "repo-avatars", storage.RepoAvatars))
|
||||||
|
|
||||||
m.Use(templates.HTMLRenderer())
|
|
||||||
mailer.InitMailRender(templates.Mailer())
|
mailer.InitMailRender(templates.Mailer())
|
||||||
|
|
||||||
localeNames, err := options.Dir("locale")
|
localeNames, err := options.Dir("locale")
|
||||||
|
|
Reference in a new issue