fs: Leave decompressor open until archive close (fix #365)

I don't like this solution, but it's all I could think of while
preserving the API.
Anyone using CompressedArchive to Extract files without using archiver.FS
will need to figure it out on their own
if they don't close files before the call to Extract returns.

.github/workflows/macos-latest.yml

@@ -8,16 +8,16 @@ jobs:
 
     strategy:
       matrix:
-        go-version: [1.18]
+        go-version: [1.21]
     runs-on: macos-latest
     steps:
     - name: Install Go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v4
       with:
         go-version: ${{ matrix.go-version }}
 
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: Build
       run: go build cmd/arc/main.go

.github/workflows/ubuntu-latest.yml

@@ -8,16 +8,16 @@ jobs:
 
     strategy:
       matrix:
-        go-version: [1.18]
+        go-version: [1.21]
     runs-on: ubuntu-latest
     steps:
     - name: Install Go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v4
       with:
         go-version: ${{ matrix.go-version }}
 
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: Build
       run: go build cmd/arc/main.go

.github/workflows/windows-latest.yml

@@ -8,16 +8,16 @@ jobs:
 
     strategy:
       matrix:
-        go-version: [1.18]
+        go-version: [1.21]
     runs-on: windows-latest
     steps:
     - name: Install Go
-      uses: actions/setup-go@v2
+      uses: actions/setup-go@v4
       with:
         go-version: ${{ matrix.go-version }}
 
     - name: Checkout code
-      uses: actions/checkout@v2
+      uses: actions/checkout@v3
 
     - name: Build
       run: go build cmd/arc/main.go

formats.go

@@ -253,16 +253,30 @@ func (caf CompressedArchive) ArchiveAsync(ctx context.Context, output io.Writer,
 }
 
 // Extract reads files out of an archive while decompressing the results.
+// If Extract is not called from ArchiveFS.Open, then the FileHandler passed
+// in must close all opened files by the time the Extract walk finishes.
 func (caf CompressedArchive) Extract(ctx context.Context, sourceArchive io.Reader, pathsInArchive []string, handleFile FileHandler) error {
 	if caf.Compression != nil {
 		rc, err := caf.Compression.OpenReader(sourceArchive)
 		if err != nil {
 			return err
 		}
-		defer rc.Close()
+		// I don't like this solution, but we have to close the decompressor.
+		// The problem is that if we simply defer rc.Close(), we potentially
+		// close it before the caller is done using files it opened. Ideally
+		// it should be closed when the sourceArchive is also closed. But since
+		// we don't originate sourceArchive, we can't close it when it closes.
+		// The best I can think of for now is this hack where we tell a type
+		// that supports this to close another reader when itself closes.
+		// See issue #365.
+		if cc, ok := sourceArchive.(compressorCloser); ok {
+			cc.closeCompressor(rc)
+		} else {
+			defer rc.Close()
+		}
 		sourceArchive = rc
 	}
-	return caf.Archival.(Extractor).Extract(ctx, sourceArchive, pathsInArchive, handleFile)
+	return caf.Archival.Extract(ctx, sourceArchive, pathsInArchive, handleFile)
 }
 
 // MatchResult returns true if the format was matched either

fs.go

@@ -329,8 +329,12 @@ func (f ArchiveFS) Open(name string) (fs.File, error) {
 		return nil
 	}
 
-	var inputStream io.Reader = archiveFile
-	if f.Stream != nil {
+	var inputStream io.Reader
+	if f.Stream == nil {
+		// when the archive file is closed, any (soon-to-be) associated decompressor should also be closed; see #365
+		archiveFile = &closeBoth{File: archiveFile}
+		inputStream = archiveFile
+	} else {
 		inputStream = io.NewSectionReader(f.Stream, 0, f.Stream.Size())
 	}
 
@@ -368,13 +372,13 @@ func (f ArchiveFS) Open(name string) (fs.File, error) {
 
 	// implicit files
 	files = fillImplicit(files)
-	file := search(name, files)
-	if file == nil {
+	file, foundFile := search(name, files)
+	if !foundFile {
 		return nil, fs.ErrNotExist
 	}
 
 	if file.IsDir() {
-		return &dirFile{extractedFile: extractedFile{File: *file}, entries: openReadDir(name, files)}, nil
+		return &dirFile{extractedFile: extractedFile{File: file}, entries: openReadDir(name, files)}, nil
 	}
 
 	// very unlikely
@@ -383,15 +387,15 @@ func (f ArchiveFS) Open(name string) (fs.File, error) {
 
 	// if named file is not a regular file, it can't be opened
 	if !file.Mode().IsRegular() {
-		return extractedFile{File: *file}, nil
+		return extractedFile{File: file}, nil
 	}
 
 	// regular files can be read, so open it for reading
 	rc, err := file.Open()
 	if err != nil {
 		return nil, err
 	}
-	return extractedFile{File: *file, ReadCloser: rc, parentArchive: archiveFile}, nil
+	return extractedFile{File: file, ReadCloser: rc, parentArchive: archiveFile}, nil
 }
 
 // copy of the same function from zip
@@ -414,7 +418,7 @@ func split(name string) (dir, elem string, isDir bool) {
 func fillImplicit(files []File) []File {
 	dirs := make(map[string]bool)
 	knownDirs := make(map[string]bool)
-	entries := make([]File, 0, 0)
+	entries := make([]File, 0)
 	for _, file := range files {
 		for dir := path.Dir(file.NameInArchive); dir != "."; dir = path.Dir(dir) {
 			dirs[dir] = true
@@ -444,7 +448,7 @@ func fillImplicit(files []File) []File {
 }
 
 // modified from zip.Reader openLookup
-func search(name string, entries []File) *File {
+func search(name string, entries []File) (File, bool) {
 	dir, elem, _ := split(name)
 	i := sort.Search(len(entries), func(i int) bool {
 		idir, ielem, _ := split(entries[i].NameInArchive)
@@ -453,10 +457,10 @@ func search(name string, entries []File) *File {
 	if i < len(entries) {
 		fname := entries[i].NameInArchive
 		if fname == name || len(fname) == len(name)+1 && fname[len(name)] == '/' && fname[:len(name)] == name {
-			return &entries[i]
+			return entries[i], true
 		}
 	}
-	return nil
+	return File{}, false
 }
 
 // modified from zip.Reader openReadDir
@@ -538,8 +542,8 @@ func (f ArchiveFS) Stat(name string) (fs.FileInfo, error) {
 	}
 
 	files = fillImplicit(files)
-	file := search(name, files)
-	if file == nil {
+	file, found := search(name, files)
+	if !found {
 		return nil, fs.ErrNotExist
 	}
 	return file.FileInfo, nil
@@ -608,8 +612,8 @@ func (f ArchiveFS) ReadDir(name string) ([]fs.DirEntry, error) {
 		return openReadDir(name, files), nil
 	}
 
-	file := search(name, files)
-	if file == nil {
+	file, foundFile := search(name, files)
+	if !foundFile {
 		return nil, fs.ErrNotExist
 	}
 
@@ -799,6 +803,36 @@ func (ef extractedFile) Close() error {
 	return nil
 }
 
+// compressorCloser is a type that closes two closers at the same time.
+// It only exists to fix #365. If a better solution can be found, I'd
+// likely prefer it.
+type compressorCloser interface {
+	io.Closer
+	closeCompressor(io.Closer)
+}
+
+// closeBoth closes both the file and an associated
+// closer, such as a (de)compressor that wraps the
+// reading/writing of the file. See issue #365. If a
+// better solution is found, I'd probably prefer that.
+type closeBoth struct {
+	fs.File
+	c io.Closer
+}
+
+// closeCompressor will have the closer closed when the associated File closes.
+func (dc *closeBoth) closeCompressor(c io.Closer) { dc.c = c }
+
+// Close closes both the file and the associated closer. It always calls
+// Close() on both, but returns only the first error, if any.
+func (dc closeBoth) Close() error {
+	err1, err2 := dc.File.Close(), dc.c.Close()
+	if err1 != nil {
+		return err1
+	}
+	return err2
+}
+
 // implicitDirEntry represents a directory that does
 // not actually exist in the archive but is inferred
 // from the paths of actual files in the archive.
@@ -840,4 +874,6 @@ var (
 	_ fs.ReadDirFS = (*ArchiveFS)(nil)
 	_ fs.StatFS    = (*ArchiveFS)(nil)
 	_ fs.SubFS     = (*ArchiveFS)(nil)
+
+	_ compressorCloser = (*closeBoth)(nil)
 )

go.mod

@@ -1,29 +1,28 @@
 module github.com/mholt/archiver/v4
 
-go 1.18
+go 1.20
 
 require (
-	github.com/andybalholm/brotli v1.0.4
+	github.com/andybalholm/brotli v1.0.5
 	github.com/dsnet/compress v0.0.1
-	github.com/klauspost/compress v1.15.9
-	github.com/klauspost/pgzip v1.2.5
+	github.com/klauspost/compress v1.16.7
+	github.com/klauspost/pgzip v1.2.6
 	github.com/nwaples/rardecode/v2 v2.0.0-beta.2
 	github.com/therootcompany/xz v1.0.1
-	github.com/ulikunitz/xz v0.5.10
+	github.com/ulikunitz/xz v0.5.11
 )
 
 require (
-	github.com/bodgit/sevenzip v1.3.0
+	github.com/bodgit/sevenzip v1.4.3
 	github.com/golang/snappy v0.0.4
-	github.com/pierrec/lz4/v4 v4.1.15
-	golang.org/x/text v0.3.8
+	github.com/pierrec/lz4/v4 v4.1.18
+	golang.org/x/text v0.13.0
 )
 
 require (
-	github.com/bodgit/plumbing v1.2.0 // indirect
-	github.com/bodgit/windows v1.0.0 // indirect
-	github.com/connesc/cipherio v0.2.1 // indirect
-	github.com/hashicorp/errwrap v1.0.0 // indirect
+	github.com/bodgit/plumbing v1.3.0 // indirect
+	github.com/bodgit/windows v1.0.1 // indirect
+	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
-	go4.org v0.0.0-20200411211856-f5505b9728dd // indirect
+	go4.org v0.0.0-20230225012048-214862532bf5 // indirect
 )