Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Fails to read file from archiver.FileSystem when archive is .tar.zst #365

Closed
zmwangx opened this issue Jan 9, 2023 · 6 comments
Closed
Labels

Comments

@zmwangx
Copy link

zmwangx commented Jan 9, 2023

What version of the package or command are you using?

v4.0.0-alpha.7

What are you trying to do?

Extract files from a .tar.zst (zstd) archive with the archiver.FileSystem feature.

What steps did you take?

Minimal reproducible example:

package main

import (
	"io"
	"log"

	"github.com/mholt/archiver/v4"
)

func main() {
	archive, err := archiver.FileSystem("test.tar.zst")
	if err != nil {
		log.Fatal(err)
	}
	f, err := archive.Open("test")
	if err != nil {
		log.Fatal(err)
	}
	if _, err := io.ReadAll(f); err != nil {
		log.Fatal(err)
	}
}

A simple archive test.tar.zst with a non-empty test file inside it is needed, created through

$ echo 12345678 > test
$ tar -cf test.tar.zst --zstd test

What did you expect to happen, and what actually happened instead?

Expected successful read without error. Actually got an error from the io.ReadAll:

decoder used after Close

This is ErrDecoderClosed from the zstd module. See https://pkg.go.dev/github.com/klauspost/compress/zstd.

Note that if we use a .tar.gz instead there's no error. The problem is zstd-specific.

Also, Tar.Extract() works fine with .tar.zst, so this is specific to the FS implementation.

How do you think this should be fixed?

I haven't got time to investigate yet.

Please link to any related issues, pull requests, and/or discussion

Bonus: What do you use archiver for, and do you find it useful?

I use it as a high level interface to archive/tar and compress/gzip, compress/zstd, etc. Saves a lot of boilerplate code.

@mholt
Copy link
Owner

mholt commented Jan 9, 2023

Thanks for the report. That's very interesting! Upon scanning the source code of ArchiveFS.Open() I'm not sure how Close() is being called in the err == nil case. I appreciate the simple reproducer... I'll try to take a closer look soon unless you beat me to it. :)

@mholt mholt added the bug label Jan 9, 2023
@imkos
Copy link

imkos commented Apr 19, 2023

// Extract reads files out of an archive while decompressing the results.
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()
		sourceArchive = rc
	}
	return caf.Archival.(Extractor).Extract(ctx, sourceArchive, pathsInArchive, handleFile)
}

when will it be repaired.

@mholt
Copy link
Owner

mholt commented May 3, 2023

@imkos Are you referring to the commented line in that code? That's probably not correct, since leaving the reader open leaks resources.

@imkos
Copy link

imkos commented Sep 15, 2023

Is there a suitable solution to this bug?

@mholt
Copy link
Owner

mholt commented Sep 15, 2023

There is, but we need to figure out how to close the compression reader beyond the scope of the Extract() method. In other words, in this case, the function that's handling each extracted file is opening and closing files after Extract() returns. It holds a pointer to the file which is then used outside of Extract(). So removing that line of code you commented is one part of the solution, but we need to still close it at the right time.

@mholt mholt closed this as completed in aa12f39 Sep 15, 2023
@mholt
Copy link
Owner

mholt commented Sep 15, 2023

@imkos @zmwangx I've finally pushed a fix for this -- but be aware it's not my favorite. It's not elegant. But it works.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants