Skip to content
Merged
Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ mistaken for a safe patch upgrade.

## [Unreleased]

### Fixed

- `distribution`: `NewArchiveReader` no longer leaks an open file when a
`.tar.gz` holds no readable tar header.

## [0.10.0] - 2026-09-18

### Added
Expand Down
4 changes: 4 additions & 0 deletions distribution/internal/archiver/archive_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ func NewArchiveReader(fqn string) (ArchiveReader, error) {
r := tar.NewReader(gzr)
err = tarReadCheck(r)
if err != nil {
cerr := f.Close()
if cerr != nil {
log.Printf("error closing file: %v", cerr)
}
return nil, err
}
return &tarReader{fqn, f}, nil
Expand Down
48 changes: 48 additions & 0 deletions distribution/internal/archiver/archive_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"compress/gzip"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/posit-dev/go-python-packaging/distribution/internal/archiver"
Expand Down Expand Up @@ -78,3 +79,50 @@ func TestTarReader_ReadFile_FirstEntry(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, pkgInfoContents, string(got))
}

// countOpenFDs counts this process's open file descriptors via /dev/fd,
// which works on both macOS and Linux (CI is ubuntu). It reads names only
// (not os.ReadDir, which also Lstats each entry): on macOS /dev/fd entries
// can close between listing and stat, so Lstat there is flaky.
func countOpenFDs(t *testing.T) int {
t.Helper()
f, err := os.Open("/dev/fd")
require.NoError(t, err)
defer func() {
require.NoError(t, f.Close())
}()
names, err := f.Readdirnames(-1)
require.NoError(t, err)
return len(names)
}

// TestNewArchiveReader_TarReadCheckFailure_DoesNotLeakFile is a regression
// test for a bug where NewArchiveReader opened f, then returned early on a
// tarReadCheck error without closing it.
func TestNewArchiveReader_TarReadCheckFailure_DoesNotLeakFile(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("counting descriptors via /dev/fd is not supported on Windows")
}

dir := t.TempDir()
path := filepath.Join(dir, "broken.tar.gz")

// A valid gzip stream with no tar data at all, so gzip.NewReader
// succeeds but tarReadCheck's r.Next() fails (io.EOF).
f, err := os.Create(path)
require.NoError(t, err)
gzw := gzip.NewWriter(f)
require.NoError(t, gzw.Close())
require.NoError(t, f.Close())

before := countOpenFDs(t)

// Run several times so a per-call leak accumulates well above noise.
for i := 0; i < 5; i++ {
_, err := archiver.NewArchiveReader(path)
require.Error(t, err)
}

after := countOpenFDs(t)
assert.Equal(t, before, after, "NewArchiveReader must not leak an open file when tarReadCheck fails")
}
Loading
Loading