diff options
| -rw-r--r-- | hotline/file_transfer.go | 42 | ||||
| -rw-r--r-- | hotline/file_transfer_handlers_test.go | 27 |
2 files changed, 59 insertions, 10 deletions
diff --git a/hotline/file_transfer.go b/hotline/file_transfer.go index 1526e28..685d0d5 100644 --- a/hotline/file_transfer.go +++ b/hotline/file_transfer.go @@ -330,18 +330,22 @@ func UploadHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileTransfe rLogger.Debug("File upload started", "dstFile", fullPath) - rForkWriter := io.Discard - iForkWriter := io.Discard + var rForkWriter, iForkWriter io.Writer = io.Discard, io.Discard + var forkClosers []io.Closer if preserveForks { - rForkWriter, err = f.rsrcForkWriter() + rFork, err := f.rsrcForkWriter() if err != nil { return err } + rForkWriter = rFork + forkClosers = append(forkClosers, rFork) - iForkWriter, err = f.InfoForkWriter() + iFork, err := f.InfoForkWriter() if err != nil { return err } + iForkWriter = iFork + forkClosers = append(forkClosers, iFork) } if err := receiveFile(rwc, file, rForkWriter, iForkWriter, fileTransfer.bytesSentCounter); err != nil { @@ -349,10 +353,17 @@ func UploadHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileTransfe return fmt.Errorf("receive file: %v", err) } - // Close the file before attempting to rename it. + // Close the data fork and the resource/info fork writers before renaming. + // Closing the fork writers is required for backends that only commit on + // Close (e.g. an object store); leaving them open also leaks descriptors. if err := file.Close(); err != nil { return fmt.Errorf("close file: %v", err) } + for _, c := range forkClosers { + if err := c.Close(); err != nil { + return fmt.Errorf("close fork: %v", err) + } + } // Rename the temporary upload file to the final file name. if err := fileStore.Rename(fullPath+".incomplete", fullPath); err != nil { @@ -653,27 +664,38 @@ func UploadFolderHandler(rwc io.ReadWriter, fullPath string, fileTransfer *FileT return err } - rForkWriter := io.Discard - iForkWriter := io.Discard + var rForkWriter, iForkWriter io.Writer = io.Discard, io.Discard + var forkClosers []io.Closer if preserveForks { - iForkWriter, err = hlFile.InfoForkWriter() + iFork, err := hlFile.InfoForkWriter() if err != nil { return err } + iForkWriter = iFork + forkClosers = append(forkClosers, iFork) - rForkWriter, err = hlFile.rsrcForkWriter() + rFork, err := hlFile.rsrcForkWriter() if err != nil { return err } + rForkWriter = rFork + forkClosers = append(forkClosers, rFork) } if err := receiveFile(rwc, incWriter, rForkWriter, iForkWriter, fileTransfer.bytesSentCounter); err != nil { return err } - // Close the file before attempting to rename it. + // Close the data fork and the resource/info fork writers before + // renaming. Closing the fork writers is required for backends + // that only commit on Close and avoids leaking descriptors. if err := incWriter.Close(); err != nil { return fmt.Errorf("close file: %v", err) } + for _, c := range forkClosers { + if err := c.Close(); err != nil { + return fmt.Errorf("close fork: %v", err) + } + } // Rename the temporary upload file to the final file name. if err := fileStore.Rename(filePath+".incomplete", filePath); err != nil { diff --git a/hotline/file_transfer_handlers_test.go b/hotline/file_transfer_handlers_test.go index 2af95cc..06bc041 100644 --- a/hotline/file_transfer_handlers_test.go +++ b/hotline/file_transfer_handlers_test.go @@ -154,6 +154,33 @@ func TestUploadHandler(t *testing.T) { assert.True(t, os.IsNotExist(err)) }) + t.Run("commits fork data on a backend that only persists on Close", func(t *testing.T) { + // MemFileStore's writers buffer and only commit on Close. This guards + // against regressing the fork writers being left unclosed, which would + // silently drop the resource and info forks on such backends. + fileStore := NewMemFileStore() + dst := "/files/note.txt" + + data := []byte("body") + rsrc := []byte("rsrc") + + ft := &FileTransfer{bytesSentCounter: &WriteCounter{}} + rwc := readWriter{r: bytes.NewReader(flatFileBytes("note.txt", data, rsrc))} + + require.NoError(t, UploadHandler(rwc, dst, ft, fileStore, logger, true)) + + got, err := fileStore.ReadFile(dst) + require.NoError(t, err) + assert.Equal(t, data, got) + + gotRsrc, err := fileStore.ReadFile("/files/" + fmt.Sprintf(RsrcForkNameTemplate, "note.txt")) + require.NoError(t, err, "resource fork must be committed") + assert.Equal(t, rsrc, gotRsrc) + + _, err = fileStore.ReadFile("/files/" + fmt.Sprintf(InfoForkNameTemplate, "note.txt")) + require.NoError(t, err, "info fork must be committed") + }) + t.Run("refuses to overwrite an existing file", func(t *testing.T) { dir := t.TempDir() fileStore := &OSFileStore{} |