diff --git a/changelog/unreleased/thumbnail-create-and-rename.md b/changelog/unreleased/thumbnail-create-and-rename.md new file mode 100644 index 00000000000..f349b49c3ab --- /dev/null +++ b/changelog/unreleased/thumbnail-create-and-rename.md @@ -0,0 +1,8 @@ +Bugfix: Fix possible race condition when a thumbnails is stored in the FS + +A race condition could cause the thumbnail service to return a thumbnail +with 0 bytes or with partial content. In order to fix this, the service will +create a temporary file with the contents and then rename that file to its +final location. + +https://github.com/owncloud/ocis/pull/10693 diff --git a/services/thumbnails/pkg/thumbnail/storage/filesystem.go b/services/thumbnails/pkg/thumbnail/storage/filesystem.go index 0188fc8b438..8f2f561fb0e 100644 --- a/services/thumbnails/pkg/thumbnail/storage/filesystem.go +++ b/services/thumbnails/pkg/thumbnail/storage/filesystem.go @@ -62,14 +62,32 @@ func (s FileSystem) Put(key string, img []byte) error { } if _, err := os.Stat(imgPath); os.IsNotExist(err) { - f, err := os.Create(imgPath) + f, err := os.CreateTemp(dir, "tmpthumb") if err != nil { - return errors.Wrapf(err, "could not create file \"%s\"", key) + return errors.Wrapf(err, "could not create temporary file for \"%s\"", key) } defer f.Close() - if _, err = f.Write(img); err != nil { - return errors.Wrapf(err, "could not write to file \"%s\"", key) + // if there was a problem writing, remove the temporary file + if _, writeErr := f.Write(img); writeErr != nil { + if remErr := os.Remove(f.Name()); remErr != nil { + return errors.Wrapf(remErr, "could not cleanup temporary file for \"%s\"", key) + } + return errors.Wrapf(writeErr, "could not write to temporary file for \"%s\"", key) + } + + // if there wasn't a problem, ensure the data is written into disk + if synErr := f.Sync(); synErr != nil { + return errors.Wrapf(synErr, "could not sync temporary file data into the disk for \"%s\"", key) + } + + // rename the temporary file to the final file + if renErr := os.Rename(f.Name(), imgPath); renErr != nil { + // if we couldn't rename, remove the temporary file + if remErr := os.Remove(f.Name()); remErr != nil { + return errors.Wrapf(remErr, "rename failed and could not cleanup temporary file for \"%s\"", key) + } + return errors.Wrapf(renErr, "could not rename temporary file to \"%s\"", key) } }