Skip to content

Fix race in local storage (#14888) #14901

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

Merged
merged 2 commits into from
Mar 6, 2021
Merged
Changes from all commits
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
48 changes: 38 additions & 10 deletions modules/storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package storage
import (
"context"
"io"
"io/ioutil"
"net/url"
"os"
"path/filepath"
Expand All @@ -24,13 +25,15 @@ const LocalStorageType Type = "local"

// LocalStorageConfig represents the configuration for a local storage
type LocalStorageConfig struct {
Path string `ini:"PATH"`
Path string `ini:"PATH"`
TemporaryPath string `ini:"TEMPORARY_PATH"`
}

// LocalStorage represents a local files storage
type LocalStorage struct {
ctx context.Context
dir string
ctx context.Context
dir string
tmpdir string
}

// NewLocalStorage returns a local files
Expand All @@ -45,9 +48,14 @@ func NewLocalStorage(ctx context.Context, cfg interface{}) (ObjectStorage, error
return nil, err
}

if config.TemporaryPath == "" {
config.TemporaryPath = config.Path + "/tmp"
}

return &LocalStorage{
ctx: ctx,
dir: config.Path,
ctx: ctx,
dir: config.Path,
tmpdir: config.TemporaryPath,
}, nil
}

Expand All @@ -63,17 +71,37 @@ func (l *LocalStorage) Save(path string, r io.Reader) (int64, error) {
return 0, err
}

// always override
if err := util.Remove(p); err != nil {
// Create a temporary file to save to
if err := os.MkdirAll(l.tmpdir, os.ModePerm); err != nil {
return 0, err
}
tmp, err := ioutil.TempFile(l.tmpdir, "upload-*")
if err != nil {
return 0, err
}
tmpRemoved := false
defer func() {
if !tmpRemoved {
_ = util.Remove(tmp.Name())
}
}()

f, err := os.Create(p)
n, err := io.Copy(tmp, r)
if err != nil {
return 0, err
}
defer f.Close()
return io.Copy(f, r)

if err := tmp.Close(); err != nil {
return 0, err
}

if err := os.Rename(tmp.Name(), p); err != nil {
return 0, err
}

tmpRemoved = true

return n, nil
}

// Stat returns the info of the file
Expand Down