Skip to content

Commit 9e8ee58

Browse files
sirozhaclaude
andcommitted
fix(resources,flowfiles): propagate zip Close() error instead of swallowing it
The four Zip* builders deferred zw.Close() with an unnamed return, dropping the error from the central-directory flush. After the resources download moved to direct socket streaming, a dropped connection during that final flush could return a truncated archive under HTTP 200. Capture Close() via a named return so streamZipArchive logs and aborts the partial response. Regression tests in both packages drive the Close() error path with a failing writer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent b8151c9 commit 9e8ee58

4 files changed

Lines changed: 56 additions & 8 deletions

File tree

backend/pkg/flowfiles/files.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,9 +430,13 @@ func ExtractTar(r io.Reader, destDir string) error {
430430
return nil
431431
}
432432

433-
func ZipDirectory(w io.Writer, dirPath string) error {
433+
func ZipDirectory(w io.Writer, dirPath string) (err error) {
434434
zw := zip.NewWriter(w)
435-
defer zw.Close()
435+
defer func() {
436+
if cerr := zw.Close(); err == nil {
437+
err = cerr
438+
}
439+
}()
436440

437441
return filepath.WalkDir(dirPath, func(entryPath string, d fs.DirEntry, err error) error {
438442
if err != nil {
@@ -469,9 +473,13 @@ func ZipDirectory(w io.Writer, dirPath string) error {
469473
//
470474
// The caller is responsible for deduplicating relPaths before calling this
471475
// function; no internal deduplication of ZIP entry names is performed.
472-
func ZipRelativePaths(w io.Writer, baseDir string, relPaths []string) error {
476+
func ZipRelativePaths(w io.Writer, baseDir string, relPaths []string) (err error) {
473477
zw := zip.NewWriter(w)
474-
defer zw.Close()
478+
defer func() {
479+
if cerr := zw.Close(); err == nil {
480+
err = cerr
481+
}
482+
}()
475483

476484
for _, relPath := range relPaths {
477485
localPath := filepath.Join(baseDir, filepath.FromSlash(relPath))

backend/pkg/flowfiles/files_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"archive/tar"
55
"archive/zip"
66
"bytes"
7+
"errors"
78
"io"
89
"mime/multipart"
910
"net/http/httptest"
@@ -17,6 +18,21 @@ import (
1718
"github.com/stretchr/testify/require"
1819
)
1920

21+
type errWriter struct{}
22+
23+
func (errWriter) Write([]byte) (int, error) { return 0, errors.New("write failed") }
24+
25+
// Empty inputs route every write through zip.Writer.Close()'s central-directory
26+
// flush, so a failing writer here exercises exactly the Close() error path that
27+
// must surface instead of being dropped as a success.
28+
func TestZipDirectoryPropagatesCloseError(t *testing.T) {
29+
require.Error(t, ZipDirectory(errWriter{}, t.TempDir()))
30+
}
31+
32+
func TestZipRelativePathsPropagatesCloseError(t *testing.T) {
33+
require.Error(t, ZipRelativePaths(errWriter{}, t.TempDir(), nil))
34+
}
35+
2036
type tarTestEntry struct {
2137
name string
2238
typeflag byte

backend/pkg/resources/resources.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,9 +322,13 @@ func CommitBlob(dataDir, hash, tmpPath string) error {
322322

323323
// ZipResources writes a ZIP archive to w containing all entries in files.
324324
// Each ZipEntry maps a .blob file on disk to a path inside the archive.
325-
func ZipResources(w io.Writer, entries []ZipEntry) error {
325+
func ZipResources(w io.Writer, entries []ZipEntry) (err error) {
326326
zw := zip.NewWriter(w)
327-
defer zw.Close()
327+
defer func() {
328+
if cerr := zw.Close(); err == nil {
329+
err = cerr
330+
}
331+
}()
328332

329333
for _, e := range entries {
330334
info, err := os.Lstat(e.BlobPath)
@@ -367,9 +371,13 @@ func ZipResources(w io.Writer, entries []ZipEntry) error {
367371
// ZipDirectory is kept for symmetry with flowfiles and wraps ZipResources by
368372
// walking a real FS directory. In the resources service, use ZipResources
369373
// instead since files are stored as blobs, not in a directory structure.
370-
func ZipDirectory(w io.Writer, dirPath string) error {
374+
func ZipDirectory(w io.Writer, dirPath string) (err error) {
371375
zw := zip.NewWriter(w)
372-
defer zw.Close()
376+
defer func() {
377+
if cerr := zw.Close(); err == nil {
378+
err = cerr
379+
}
380+
}()
373381

374382
return filepath.WalkDir(dirPath, func(entryPath string, d fs.DirEntry, err error) error {
375383
if err != nil {

backend/pkg/resources/resources_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"bytes"
66
"crypto/md5"
77
"encoding/hex"
8+
"errors"
89
"io"
910
"os"
1011
"path/filepath"
@@ -15,6 +16,21 @@ import (
1516
"github.com/stretchr/testify/require"
1617
)
1718

19+
type errWriter struct{}
20+
21+
func (errWriter) Write([]byte) (int, error) { return 0, errors.New("write failed") }
22+
23+
// With no entries, the only bytes written to w happen during zip.Writer.Close()'s
24+
// final central-directory flush, so a failing writer here exercises exactly the
25+
// Close() error path that must surface instead of being dropped as a success.
26+
func TestZipResourcesPropagatesCloseError(t *testing.T) {
27+
require.Error(t, ZipResources(errWriter{}, nil))
28+
}
29+
30+
func TestZipDirectoryPropagatesCloseError(t *testing.T) {
31+
require.Error(t, ZipDirectory(errWriter{}, t.TempDir()))
32+
}
33+
1834
func TestResourceDirsAndBlobPath(t *testing.T) {
1935
assert.Equal(t, "/data/resources", ResourcesDir("/data"))
2036
assert.Equal(t, "/data/resources/900150983cd24fb0d6963f7d28e17f72.blob",

0 commit comments

Comments
 (0)