From d38d0aaf74e642d9004b8fee09ab93befeffd174 Mon Sep 17 00:00:00 2001 From: Matt Dainty Date: Sun, 17 Nov 2024 18:13:31 +0000 Subject: [PATCH] feat: Add ReadError to wrap I/O errors (#278) * feat: Add ReadError to wrap I/O errors * fix: Return ReadError where appropriate * style: Lint fixes * style: More lint fixes Finally enable and quieten err113 and wrapcheck linters. --- .golangci.yaml | 4 + .pre-commit-config.yaml | 2 +- README.md | 24 +++++ internal/aes7z/key.go | 3 +- internal/aes7z/reader.go | 47 ++++++--- internal/bcj2/reader.go | 86 +++++++++++------ internal/bra/bra.go | 1 + internal/bra/reader.go | 30 ++++-- internal/brotli/reader.go | 45 ++++++--- internal/bzip2/reader.go | 32 +++++-- internal/deflate/reader.go | 48 ++++++---- internal/delta/reader.go | 41 +++++--- internal/lz4/reader.go | 41 +++++--- internal/lzma/reader.go | 55 ++++++++--- internal/lzma2/reader.go | 39 +++++--- internal/pool/pool.go | 5 +- internal/util/reader.go | 3 +- internal/zstd/reader.go | 45 ++++++--- reader.go | 191 +++++++++++++++++++++++++------------ reader_test.go | 167 +++++++++++++++++++++++++++----- register.go | 10 +- struct.go | 81 ++++++++++------ testdata/t4.7z | Bin 0 -> 189 bytes testdata/t5.7z | Bin 0 -> 217 bytes types.go | 11 ++- 25 files changed, 733 insertions(+), 278 deletions(-) create mode 100644 testdata/t4.7z create mode 100644 testdata/t5.7z diff --git a/.golangci.yaml b/.golangci.yaml index ee8fbd0..a13515e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,4 +1,6 @@ --- +issues: + exclude-use-default: false linters: disable-all: true enable: @@ -16,6 +18,7 @@ linters: - dupl - dupword - durationcheck + - err113 - errcheck - errchkjson - errname @@ -97,5 +100,6 @@ linters: - usestdlibvars - wastedassign - whitespace + - wrapcheck - wsl - zerologlint diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ee68def..690eec1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: hooks: - id: commitizen - repo: https://github.com/golangci/golangci-lint - rev: v1.60.3 + rev: v1.61.0 hooks: - id: golangci-lint - repo: https://github.com/gitleaks/gitleaks diff --git a/README.md b/README.md index 673c14a..a1e7f02 100644 --- a/README.md +++ b/README.md @@ -119,3 +119,27 @@ This achieves a 50% speed improvement with the LZMA SDK archive, but it very muc In general, don't try and extract the files in a different order compared to the natural order within the archive as that will also undo the optimisation. The worst scenario would likely be to extract the archive in reverse order. + +### Detecting the wrong password + +It's virtually impossible to _reliably_ detect the wrong password versus some other corruption in a password protected archive. +This is partly due to how CBC decryption works; with the wrong password you don't get any sort of decryption error, you just a stream of bytes that aren't the correct ones. +This manifests itself when the file has been compressed _and_ encrypted; during extraction the file is decrypted and then decompressed so with the wrong password the decompression algorithm gets handed a stream which isn't valid so that's the error you see. + +A `sevenzip.ReadError` error type can be returned for certain operations. +If `sevenzip.ReadError.Encrypted` is `true` then encryption is involved and you can use that as a **hint** to either set a password or try a different one. +Use `errors.As()` to check like this: +```golang +r, err := sevenzip.OpenReaderWithPassword(archive, password) +if err != nil { + var e *sevenzip.ReadError + if errors.As(err, &e) && e.Encrypted { + // Encryption involved, retry with a different password + } + + return err +} +``` +Be aware that if the archive does not have the headers encrypted, (`7za a -mhe=off -ppassword test.7z ...`), then you can always open the archive and the password is only used when extracting the files. + +If files are added to the archive encrypted and _not_ compressed, (`7za a -m0=copy -ppassword test.7z ...`), then you will never get an error extracting with the wrong password as the only consumer of the decrypted content will be your own code. To detect a potentially wrong password, calculate the CRC value and check that it matches the value in `sevenzip.FileHeader.CRC32`. diff --git a/internal/aes7z/key.go b/internal/aes7z/key.go index 79c78dd..04532aa 100644 --- a/internal/aes7z/key.go +++ b/internal/aes7z/key.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "encoding/binary" "encoding/hex" + "fmt" lru "github.com/hashicorp/golang-lru/v2" "go4.org/syncutil" @@ -32,7 +33,7 @@ func calculateKey(password string, cycles int, salt []byte) ([]byte, error) { return }); err != nil { - return nil, err + return nil, fmt.Errorf("aes7z: error creating cache: %w", err) } ck := cacheKey{ diff --git a/internal/aes7z/reader.go b/internal/aes7z/reader.go index 760db29..cbd5cff 100644 --- a/internal/aes7z/reader.go +++ b/internal/aes7z/reader.go @@ -1,3 +1,4 @@ +// Package aes7z implements the 7-zip AES decryption. package aes7z import ( @@ -5,10 +6,17 @@ import ( "crypto/aes" "crypto/cipher" "errors" + "fmt" "io" ) -var errProperties = errors.New("aes7z: not enough properties") +var ( + errAlreadyClosed = errors.New("aes7z: already closed") + errNeedOneReader = errors.New("aes7z: need exactly one reader") + errInsufficientProperties = errors.New("aes7z: not enough properties") + errNoPasswordSet = errors.New("aes7z: no password set") + errUnsupportedMethod = errors.New("aes7z: unsupported compression method") +) type readCloser struct { rc io.ReadCloser @@ -19,13 +27,17 @@ type readCloser struct { } func (rc *readCloser) Close() error { - var err error - if rc.rc != nil { - err = rc.rc.Close() - rc.rc = nil + if rc.rc == nil { + return errAlreadyClosed } - return err + if err := rc.rc.Close(); err != nil { + return fmt.Errorf("aes7z: error closing: %w", err) + } + + rc.rc = nil + + return nil } func (rc *readCloser) Password(p string) error { @@ -36,7 +48,7 @@ func (rc *readCloser) Password(p string) error { block, err := aes.NewCipher(key) if err != nil { - return err + return fmt.Errorf("aes7z: error creating cipher: %w", err) } rc.cbc = cipher.NewCBCDecrypter(block, rc.iv) @@ -46,11 +58,11 @@ func (rc *readCloser) Password(p string) error { func (rc *readCloser) Read(p []byte) (int, error) { if rc.rc == nil { - return 0, errors.New("aes7z: Read after Close") + return 0, errAlreadyClosed } if rc.cbc == nil { - return 0, errors.New("aes7z: no password set") + return 0, errNoPasswordSet } var block [aes.BlockSize]byte @@ -61,7 +73,7 @@ func (rc *readCloser) Read(p []byte) (int, error) { break } - return 0, err + return 0, fmt.Errorf("aes7z: error reading block: %w", err) } rc.cbc.CryptBlocks(block[:], block[:]) @@ -69,7 +81,12 @@ func (rc *readCloser) Read(p []byte) (int, error) { _, _ = rc.buf.Write(block[:]) } - return rc.buf.Read(p) + n, err := rc.buf.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("aes7z: error reading: %w", err) + } + + return n, err } // NewReader returns a new AES-256-CBC & SHA-256 io.ReadCloser. The Password @@ -77,16 +94,16 @@ func (rc *readCloser) Read(p []byte) (int, error) { // cipher is correctly initialised. func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("aes7z: need exactly one reader") + return nil, errNeedOneReader } // Need at least two bytes initially if len(p) < 2 { - return nil, errProperties + return nil, errInsufficientProperties } if p[0]&0xc0 == 0 { - return nil, errors.New("aes7z: unsupported compression method") + return nil, errUnsupportedMethod } rc := new(readCloser) @@ -95,7 +112,7 @@ func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro iv := p[0]>>6&1 + p[1]&0x0f if len(p) != int(2+salt+iv) { - return nil, errProperties + return nil, errInsufficientProperties } rc.salt = p[2 : 2+salt] diff --git a/internal/bcj2/reader.go b/internal/bcj2/reader.go index 343ec5f..957ea23 100644 --- a/internal/bcj2/reader.go +++ b/internal/bcj2/reader.go @@ -1,15 +1,34 @@ +// Package bcj2 implements the BCJ2 filter for x86 binaries. package bcj2 import ( "bytes" "encoding/binary" "errors" + "fmt" "io" "github.com/bodgit/sevenzip/internal/util" "github.com/hashicorp/go-multierror" ) +type readCloser struct { + main util.ReadCloser + call io.ReadCloser + jump io.ReadCloser + + rd util.ReadCloser + nrange uint + code uint + + sd [256 + 2]uint + + previous byte + written uint32 + + buf *bytes.Buffer +} + const ( numMoveBits = 5 numbitModelTotalBits = 11 @@ -18,6 +37,11 @@ const ( topValue uint = 1 << numTopBits ) +var ( + errAlreadyClosed = errors.New("bcj2: already closed") + errNeedFourReaders = errors.New("bcj2: need exactly four readers") +) + func isJcc(b0, b1 byte) bool { return b0 == 0x0f && (b1&0xf0) == 0x80 } @@ -37,27 +61,10 @@ func index(b0, b1 byte) int { } } -type readCloser struct { - main util.ReadCloser - call io.ReadCloser - jump io.ReadCloser - - rd util.ReadCloser - nrange uint - code uint - - sd [256 + 2]uint - - previous byte - written uint32 - - buf *bytes.Buffer -} - // NewReader returns a new BCJ2 io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 4 { - return nil, errors.New("bcj2: need exactly four readers") + return nil, errNeedFourReaders } rc := &readCloser{ @@ -72,6 +79,10 @@ func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro b := make([]byte, 5) if _, err := io.ReadFull(rc.rd, b); err != nil { + if !errors.Is(err, io.EOF) { + err = fmt.Errorf("bcj2: error reading initial state: %w", err) + } + return nil, err } @@ -87,31 +98,42 @@ func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro } func (rc *readCloser) Close() error { - var err *multierror.Error - if rc.main != nil { - err = multierror.Append(err, rc.main.Close(), rc.call.Close(), rc.jump.Close(), rc.rd.Close()) + if rc.main == nil || rc.call == nil || rc.jump == nil || rc.rd == nil { + return errAlreadyClosed + } + + //nolint:lll + if err := multierror.Append(rc.main.Close(), rc.call.Close(), rc.jump.Close(), rc.rd.Close()).ErrorOrNil(); err != nil { + return fmt.Errorf("bcj2: error closing: %w", err) } - return err.ErrorOrNil() + rc.main, rc.call, rc.jump, rc.rd = nil, nil, nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { - if rc.main == nil { - return 0, errors.New("bcj2: Read after Close") + if rc.main == nil || rc.call == nil || rc.jump == nil || rc.rd == nil { + return 0, errAlreadyClosed } if err := rc.read(); err != nil && !errors.Is(err, io.EOF) { return 0, err } - return rc.buf.Read(p) + n, err := rc.buf.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("bcj2: error reading: %w", err) + } + + return n, err } func (rc *readCloser) update() error { if rc.nrange < topValue { b, err := rc.rd.ReadByte() - if err != nil { - return err + if err != nil && !errors.Is(err, io.EOF) { + return fmt.Errorf("bcj2: error reading byte: %w", err) } rc.code = (rc.code << 8) | uint(b) @@ -146,6 +168,7 @@ func (rc *readCloser) decode(i int) (bool, error) { return true, nil } +//nolint:cyclop,funlen func (rc *readCloser) read() error { var ( b byte @@ -154,6 +177,10 @@ func (rc *readCloser) read() error { for { if b, err = rc.main.ReadByte(); err != nil { + if !errors.Is(err, io.EOF) { + err = fmt.Errorf("bcj2: error reading byte: %w", err) + } + return err } @@ -176,6 +203,7 @@ func (rc *readCloser) read() error { return err } + //nolint:nestif if bit { var r io.Reader if b == 0xe8 { @@ -186,6 +214,10 @@ func (rc *readCloser) read() error { var dest uint32 if err = binary.Read(r, binary.BigEndian, &dest); err != nil { + if !errors.Is(err, io.EOF) { + err = fmt.Errorf("bcj2: error reading uint32: %w", err) + } + return err } diff --git a/internal/bra/bra.go b/internal/bra/bra.go index a2f0daa..a7a77d7 100644 --- a/internal/bra/bra.go +++ b/internal/bra/bra.go @@ -1,3 +1,4 @@ +// Package bra implements the branch rewriting filter for binaries. package bra type converter interface { diff --git a/internal/bra/reader.go b/internal/bra/reader.go index 42edf15..733333b 100644 --- a/internal/bra/reader.go +++ b/internal/bra/reader.go @@ -3,6 +3,7 @@ package bra import ( "bytes" "errors" + "fmt" "io" ) @@ -13,23 +14,33 @@ type readCloser struct { conv converter } -func (rc *readCloser) Close() (err error) { - if rc.rc != nil { - err = rc.rc.Close() - rc.rc = nil +var ( + errAlreadyClosed = errors.New("bra: already closed") + errNeedOneReader = errors.New("bra: need exactly one reader") +) + +func (rc *readCloser) Close() error { + if rc.rc == nil { + return errAlreadyClosed } - return + if err := rc.rc.Close(); err != nil { + return fmt.Errorf("bra: error closing: %w", err) + } + + rc.rc = nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.rc == nil { - return 0, errors.New("bra: Read after Close") + return 0, errAlreadyClosed } if _, err := io.CopyN(&rc.buf, rc.rc, int64(max(len(p), rc.conv.Size())-rc.buf.Len())); err != nil { if !errors.Is(err, io.EOF) { - return 0, err + return 0, fmt.Errorf("bra: error buffering: %w", err) } if rc.buf.Len() < rc.conv.Size() { @@ -40,6 +51,9 @@ func (rc *readCloser) Read(p []byte) (int, error) { rc.n += rc.conv.Convert(rc.buf.Bytes()[rc.n:], false) n, err := rc.buf.Read(p[:min(rc.n, len(p))]) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("bra: error reading: %w", err) + } rc.n -= n @@ -48,7 +62,7 @@ func (rc *readCloser) Read(p []byte) (int, error) { func newReader(readers []io.ReadCloser, conv converter) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("bra: need exactly one reader") + return nil, errNeedOneReader } return &readCloser{ diff --git a/internal/brotli/reader.go b/internal/brotli/reader.go index c044047..a07b764 100644 --- a/internal/brotli/reader.go +++ b/internal/brotli/reader.go @@ -1,9 +1,11 @@ +// Package brotli implements the Brotli decompressor. package brotli import ( "bytes" "encoding/binary" "errors" + "fmt" "io" "sync" @@ -11,9 +13,6 @@ import ( "github.com/bodgit/plumbing" ) -//nolint:gochecknoglobals -var brotliReaderPool sync.Pool - type readCloser struct { c io.Closer r *brotli.Reader @@ -25,6 +24,14 @@ const ( brotliMagic uint16 = 0x5242 // 'B', 'R' ) +var ( + //nolint:gochecknoglobals + brotliReaderPool sync.Pool + + errAlreadyClosed = errors.New("brotli: already closed") + errNeedOneReader = errors.New("brotli: need exactly one reader") +) + // This isn't part of the Brotli format but is prepended by the 7-zip implementation. type headerFrame struct { FrameMagic uint32 @@ -34,28 +41,38 @@ type headerFrame struct { UncompressedSize uint16 // * 64 KB } -func (rc *readCloser) Close() (err error) { - if rc.c != nil { - brotliReaderPool.Put(rc.r) - err = rc.c.Close() - rc.c, rc.r = nil, nil +func (rc *readCloser) Close() error { + if rc.c == nil || rc.r == nil { + return errAlreadyClosed + } + + if err := rc.c.Close(); err != nil { + return fmt.Errorf("brotli: error closing: %w", err) } - return + brotliReaderPool.Put(rc.r) + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("brotli: Read after Close") + return 0, errAlreadyClosed } - return rc.r.Read(p) + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("brotli: error reading: %w", err) + } + + return n, err } // NewReader returns a new Brotli io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("brotli: need exactly one reader") + return nil, errNeedOneReader } hr, b := new(headerFrame), new(bytes.Buffer) @@ -65,6 +82,10 @@ func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro // the data which will confuse a pure Brotli implementation. Read it // but keep a copy so we can add it back if it doesn't look right if err := binary.Read(io.TeeReader(readers[0], b), binary.LittleEndian, hr); err != nil { + if !errors.Is(err, io.EOF) { + err = fmt.Errorf("brotli: error reading frame: %w", err) + } + return nil, err } diff --git a/internal/bzip2/reader.go b/internal/bzip2/reader.go index 3b809df..3e82498 100644 --- a/internal/bzip2/reader.go +++ b/internal/bzip2/reader.go @@ -1,8 +1,10 @@ +// Package bzip2 implements the Bzip2 decompressor. package bzip2 import ( "compress/bzip2" "errors" + "fmt" "io" ) @@ -11,28 +13,42 @@ type readCloser struct { r io.Reader } +var ( + errAlreadyClosed = errors.New("bzip2: already closed") + errNeedOneReader = errors.New("bzip2: need exactly one reader") +) + func (rc *readCloser) Close() error { - var err error - if rc.c != nil { - err = rc.c.Close() - rc.c, rc.r = nil, nil + if rc.c == nil || rc.r == nil { + return errAlreadyClosed + } + + if err := rc.c.Close(); err != nil { + return fmt.Errorf("bzip2: error closing: %w", err) } - return err + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("bzip2: Read after Close") + return 0, errAlreadyClosed + } + + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("bzip2: error reading: %w", err) } - return rc.r.Read(p) + return n, err } // NewReader returns a new bzip2 io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("bzip2: need exactly one reader") + return nil, errNeedOneReader } return &readCloser{ diff --git a/internal/deflate/reader.go b/internal/deflate/reader.go index f3a4ab4..c0b4e4d 100644 --- a/internal/deflate/reader.go +++ b/internal/deflate/reader.go @@ -1,50 +1,62 @@ +// Package deflate implements the Deflate decompressor. package deflate import ( "errors" + "fmt" "io" "sync" "github.com/bodgit/sevenzip/internal/util" + "github.com/hashicorp/go-multierror" "github.com/klauspost/compress/flate" ) -//nolint:gochecknoglobals -var flateReaderPool sync.Pool - type readCloser struct { c io.Closer fr io.ReadCloser } -func (rc *readCloser) Close() error { - var err error +var ( + //nolint:gochecknoglobals + flateReaderPool sync.Pool - if rc.c != nil { - if err = rc.fr.Close(); err != nil { - return err - } + errAlreadyClosed = errors.New("deflate: already closed") + errNeedOneReader = errors.New("deflate: need exactly one reader") +) + +func (rc *readCloser) Close() error { + if rc.c == nil || rc.fr == nil { + return errAlreadyClosed + } - flateReaderPool.Put(rc.fr) - err = rc.c.Close() - rc.c, rc.fr = nil, nil + if err := multierror.Append(rc.fr.Close(), rc.c.Close()).ErrorOrNil(); err != nil { + return fmt.Errorf("deflate: error closing: %w", err) } - return err + flateReaderPool.Put(rc.fr) + rc.c, rc.fr = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { - if rc.fr == nil { - return 0, errors.New("deflate: Read after Close") + if rc.c == nil || rc.fr == nil { + return 0, errAlreadyClosed + } + + n, err := rc.fr.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("deflate: error reading: %w", err) } - return rc.fr.Read(p) + return n, err } // NewReader returns a new DEFLATE io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("deflate: need exactly one reader") + return nil, errNeedOneReader } fr, ok := flateReaderPool.Get().(io.ReadCloser) @@ -52,7 +64,7 @@ func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro frf, ok := fr.(flate.Resetter) if ok { if err := frf.Reset(util.ByteReadCloser(readers[0]), nil); err != nil { - return nil, err + return nil, fmt.Errorf("deflate: error resetting: %w", err) } } } else { diff --git a/internal/delta/reader.go b/internal/delta/reader.go index 2a2253f..0926a17 100644 --- a/internal/delta/reader.go +++ b/internal/delta/reader.go @@ -1,37 +1,50 @@ +// Package delta implements the Delta filter. package delta import ( "errors" + "fmt" "io" ) -const ( - stateSize = 256 -) - type readCloser struct { rc io.ReadCloser state [stateSize]byte delta int } -func (rc *readCloser) Close() (err error) { - if rc.rc != nil { - err = rc.rc.Close() - rc.rc = nil +const ( + stateSize = 256 +) + +var ( + errAlreadyClosed = errors.New("delta: already closed") + errNeedOneReader = errors.New("delta: need exactly one reader") + errInsufficientProperties = errors.New("delta: not enough properties") +) + +func (rc *readCloser) Close() error { + if rc.rc == nil { + return errAlreadyClosed } - return + if err := rc.rc.Close(); err != nil { + return fmt.Errorf("delta: error closing: %w", err) + } + + rc.rc = nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.rc == nil { - return 0, errors.New("delta: Read after Close") + return 0, errAlreadyClosed } n, err := rc.rc.Read(p) - if err != nil { - return n, err + if err != nil && !errors.Is(err, io.EOF) { + return n, fmt.Errorf("delta: error reading: %w", err) } var ( @@ -62,11 +75,11 @@ func (rc *readCloser) Read(p []byte) (int, error) { // NewReader returns a new Delta io.ReadCloser. func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("delta: need exactly one reader") + return nil, errNeedOneReader } if len(p) != 1 { - return nil, errors.New("delta: not enough properties") + return nil, errInsufficientProperties } return &readCloser{ diff --git a/internal/lz4/reader.go b/internal/lz4/reader.go index d8ba114..4299334 100644 --- a/internal/lz4/reader.go +++ b/internal/lz4/reader.go @@ -1,43 +1,60 @@ +// Package lz4 implements the LZ4 decompressor. package lz4 import ( "errors" + "fmt" "io" "sync" lz4 "github.com/pierrec/lz4/v4" ) -//nolint:gochecknoglobals -var lz4ReaderPool sync.Pool - type readCloser struct { c io.Closer r *lz4.Reader } -func (rc *readCloser) Close() (err error) { - if rc.c != nil { - lz4ReaderPool.Put(rc.r) - err = rc.c.Close() - rc.c, rc.r = nil, nil +var ( + //nolint:gochecknoglobals + lz4ReaderPool sync.Pool + + errAlreadyClosed = errors.New("lz4: already closed") + errNeedOneReader = errors.New("lz4: need exactly one reader") +) + +func (rc *readCloser) Close() error { + if rc.c == nil || rc.r == nil { + return errAlreadyClosed } - return + if err := rc.c.Close(); err != nil { + return fmt.Errorf("lz4: error closing: %w", err) + } + + lz4ReaderPool.Put(rc.r) + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("lz4: Read after Close") + return 0, errAlreadyClosed + } + + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("lz4: error reading: %w", err) } - return rc.r.Read(p) + return n, err } // NewReader returns a new LZ4 io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("lz4: need exactly one reader") + return nil, errNeedOneReader } r, ok := lz4ReaderPool.Get().(*lz4.Reader) diff --git a/internal/lzma/reader.go b/internal/lzma/reader.go index 485d9f6..b8d277c 100644 --- a/internal/lzma/reader.go +++ b/internal/lzma/reader.go @@ -1,9 +1,11 @@ +// Package lzma implements the LZMA decompressor. package lzma import ( "bytes" "encoding/binary" "errors" + "fmt" "io" "github.com/ulikunitz/xz/lzma" @@ -14,28 +16,42 @@ type readCloser struct { r io.Reader } +var ( + errAlreadyClosed = errors.New("lzma: already closed") + errNeedOneReader = errors.New("lzma: need exactly one reader") +) + func (rc *readCloser) Close() error { - var err error - if rc.c != nil { - err = rc.c.Close() - rc.c, rc.r = nil, nil + if rc.c == nil || rc.r == nil { + return errAlreadyClosed + } + + if err := rc.c.Close(); err != nil { + return fmt.Errorf("lzma: error closing: %w", err) } - return err + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("lzma: Read after Close") + return 0, errAlreadyClosed + } + + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("lzma: error reading: %w", err) } - return rc.r.Read(p) + return n, err } // NewReader returns a new LZMA io.ReadCloser. func NewReader(p []byte, s uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("lzma: need exactly one reader") + return nil, errNeedOneReader } h := bytes.NewBuffer(p) @@ -43,7 +59,7 @@ func NewReader(p []byte, s uint64, readers []io.ReadCloser) (io.ReadCloser, erro lr, err := lzma.NewReader(multiReader(h, readers[0])) if err != nil { - return nil, err + return nil, fmt.Errorf("lzma: error creating reader: %w", err) } return &readCloser{ @@ -72,14 +88,25 @@ type multiByteReader struct { mr io.Reader } -func (m *multiByteReader) ReadByte() (byte, error) { +func (m *multiByteReader) ReadByte() (b byte, err error) { if m.b.Len() > 0 { - return m.b.ReadByte() + b, err = m.b.ReadByte() + } else { + b, err = m.br.ReadByte() } - return m.br.ReadByte() + if err != nil { + err = fmt.Errorf("lzma: error multi byte reading: %w", err) + } + + return b, err } -func (m *multiByteReader) Read(p []byte) (n int, err error) { - return m.mr.Read(p) +func (m *multiByteReader) Read(p []byte) (int, error) { + n, err := m.mr.Read(p) + if err != nil { + err = fmt.Errorf("lzma: error multi reading: %w", err) + } + + return n, err } diff --git a/internal/lzma2/reader.go b/internal/lzma2/reader.go index 0c4677e..3f2e7be 100644 --- a/internal/lzma2/reader.go +++ b/internal/lzma2/reader.go @@ -1,7 +1,9 @@ +// Package lzma2 implements the LZMA2 decompressor. package lzma2 import ( "errors" + "fmt" "io" "github.com/ulikunitz/xz/lzma" @@ -12,32 +14,47 @@ type readCloser struct { r io.Reader } +var ( + errAlreadyClosed = errors.New("lzma2: already closed") + errNeedOneReader = errors.New("lzma2: need exactly one reader") + errInsufficientProperties = errors.New("lzma2: not enough properties") +) + func (rc *readCloser) Close() error { - var err error - if rc.c != nil { - err = rc.c.Close() - rc.c, rc.r = nil, nil + if rc.c == nil || rc.r == nil { + return errAlreadyClosed + } + + if err := rc.c.Close(); err != nil { + return fmt.Errorf("lzma2: error closing: %w", err) } - return err + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("lzma2: Read after Close") + return 0, errAlreadyClosed + } + + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("lzma2: error reading: %w", err) } - return rc.r.Read(p) + return n, err } // NewReader returns a new LZMA2 io.ReadCloser. func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("lzma2: need exactly one reader") + return nil, errNeedOneReader } if len(p) != 1 { - return nil, errors.New("lzma2: not enough properties") + return nil, errInsufficientProperties } config := lzma.Reader2Config{ @@ -45,12 +62,12 @@ func NewReader(p []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro } if err := config.Verify(); err != nil { - return nil, err + return nil, fmt.Errorf("lzma2: error verifying config: %w", err) } lr, err := config.NewReader2(readers[0]) if err != nil { - return nil, err + return nil, fmt.Errorf("lzma2: error creating reader: %w", err) } return &readCloser{ diff --git a/internal/pool/pool.go b/internal/pool/pool.go index 4cb61cf..bd7194d 100644 --- a/internal/pool/pool.go +++ b/internal/pool/pool.go @@ -1,3 +1,4 @@ +// Package pool implements the reader pooling. package pool import ( @@ -30,7 +31,7 @@ func (noopPool) Get(_ int64) (util.SizeReadSeekCloser, bool) { } func (noopPool) Put(_ int64, rc util.SizeReadSeekCloser) (bool, error) { - return false, rc.Close() + return false, rc.Close() //nolint:wrapcheck } type pool struct { @@ -130,7 +131,7 @@ func (p *pool) removeElement(e *list.Element, cb bool) error { delete(p.items, kv.key) if cb { - return kv.value.Close() + return kv.value.Close() //nolint:wrapcheck } return nil diff --git a/internal/util/reader.go b/internal/util/reader.go index 4fc2cb4..21d2c94 100644 --- a/internal/util/reader.go +++ b/internal/util/reader.go @@ -1,3 +1,4 @@ +// Package util implements various utility types and interfaces. package util import "io" @@ -46,7 +47,7 @@ func (rc *byteReadCloser) ReadByte() (byte, error) { n, err := rc.Read(b[:]) if err != nil { - return 0, err + return 0, err //nolint:wrapcheck } if n == 0 { diff --git a/internal/zstd/reader.go b/internal/zstd/reader.go index 0d68a3c..6817f1a 100644 --- a/internal/zstd/reader.go +++ b/internal/zstd/reader.go @@ -1,7 +1,9 @@ +// Package zstd implements the Zstandard decompressor. package zstd import ( "errors" + "fmt" "io" "runtime" "sync" @@ -9,36 +11,51 @@ import ( "github.com/klauspost/compress/zstd" ) -//nolint:gochecknoglobals -var zstdReaderPool sync.Pool - type readCloser struct { c io.Closer r *zstd.Decoder } -func (rc *readCloser) Close() (err error) { - if rc.c != nil { - zstdReaderPool.Put(rc.r) - err = rc.c.Close() - rc.c, rc.r = nil, nil +var ( + //nolint:gochecknoglobals + zstdReaderPool sync.Pool + + errAlreadyClosed = errors.New("zstd: already closed") + errNeedOneReader = errors.New("zstd: need exactly one reader") +) + +func (rc *readCloser) Close() error { + if rc.c == nil { + return errAlreadyClosed } - return + if err := rc.c.Close(); err != nil { + return fmt.Errorf("zstd: error closing: %w", err) + } + + zstdReaderPool.Put(rc.r) + rc.c, rc.r = nil, nil + + return nil } func (rc *readCloser) Read(p []byte) (int, error) { if rc.r == nil { - return 0, errors.New("zstd: Read after Close") + return 0, errAlreadyClosed + } + + n, err := rc.r.Read(p) + if err != nil && !errors.Is(err, io.EOF) { + err = fmt.Errorf("zstd: error reading: %w", err) } - return rc.r.Read(p) + return n, err } // NewReader returns a new Zstandard io.ReadCloser. func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("zstd: need exactly one reader") + return nil, errNeedOneReader } var err error @@ -46,11 +63,11 @@ func NewReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, erro r, ok := zstdReaderPool.Get().(*zstd.Decoder) if ok { if err = r.Reset(readers[0]); err != nil { - return nil, err + return nil, fmt.Errorf("zstd: error resetting: %w", err) } } else { if r, err = zstd.NewReader(readers[0]); err != nil { - return nil, err + return nil, fmt.Errorf("zstd: error creating reader: %w", err) } runtime.SetFinalizer(r, (*zstd.Decoder).Close) diff --git a/reader.go b/reader.go index 4e3d845..635f5c2 100644 --- a/reader.go +++ b/reader.go @@ -1,3 +1,4 @@ +// Package sevenzip provides read access to 7-zip archives. package sevenzip import ( @@ -25,11 +26,28 @@ import ( ) var ( - errFormat = errors.New("sevenzip: not a valid 7-zip file") - errChecksum = errors.New("sevenzip: checksum error") - errTooMuch = errors.New("sevenzip: too much data") + errFormat = errors.New("sevenzip: not a valid 7-zip file") + errChecksum = errors.New("sevenzip: checksum error") + errTooMuch = errors.New("sevenzip: too much data") + errNegativeSize = errors.New("sevenzip: size cannot be negative") + errOneHeaderStream = errors.New("sevenzip: expected only one folder in header stream") ) +// ReadError is used to wrap read I/O errors. +type ReadError struct { + // Encrypted is a hint that there is encryption involved. + Encrypted bool + Err error +} + +func (e ReadError) Error() string { + return fmt.Sprintf("sevenzip: read error: %v", e.Err) +} + +func (e ReadError) Unwrap() error { + return e.Err +} + // A Reader serves content from a 7-Zip archive. type Reader struct { r io.ReaderAt @@ -44,14 +62,15 @@ type Reader struct { fileList []fileListEntry } -// A ReadCloser is a Reader that must be closed when no longer needed. +// A ReadCloser is a [Reader] that must be closed when no longer needed. type ReadCloser struct { f []*os.File Reader } // A File is a single file in a 7-Zip archive. The file information is in the -// embedded FileHeader. The file content can be accessed by calling Open. +// embedded [FileHeader]. The file content can be accessed by calling +// [File.Open]. type File struct { FileHeader zip *Reader @@ -69,7 +88,7 @@ func (fr *fileReader) Stat() (fs.FileInfo, error) { return headerFileInfo{&fr.f.FileHeader}, nil } -func (fr *fileReader) Read(p []byte) (n int, err error) { +func (fr *fileReader) Read(p []byte) (int, error) { if len(p) == 0 { return 0, nil } @@ -82,10 +101,22 @@ func (fr *fileReader) Read(p []byte) (n int, err error) { p = p[0:fr.n] } - n, err = fr.rc.Read(p) + n, err := fr.rc.Read(p) fr.n -= int64(n) - return + if err != nil && !errors.Is(err, io.EOF) { + e := &ReadError{ + Err: err, + } + + if frc, ok := fr.rc.(*folderReadCloser); ok { + e.Encrypted = frc.hasEncryption + } + + return n, e + } + + return n, err //nolint:wrapcheck } func (fr *fileReader) Close() error { @@ -95,17 +126,17 @@ func (fr *fileReader) Close() error { offset, err := fr.rc.Seek(0, io.SeekCurrent) if err != nil { - return err + return fmt.Errorf("sevenzip: error seeking current position: %w", err) } if offset == fr.rc.Size() { // EOF reached if err := fr.rc.Close(); err != nil { - return err + return fmt.Errorf("sevenzip: error closing: %w", err) } } else { f := fr.f if _, err := f.zip.pool[f.folder].Put(offset, fr.rc); err != nil { - return err + return fmt.Errorf("sevenzip: error adding to pool: %w", err) } } @@ -114,26 +145,40 @@ func (fr *fileReader) Close() error { return nil } -// Open returns an io.ReadCloser that provides access to the File's contents. -// Multiple files may be read concurrently. +// Open returns an [io.ReadCloser] that provides access to the [File]'s +// contents. Multiple files may be read concurrently. func (f *File) Open() (io.ReadCloser, error) { if f.FileHeader.isEmptyStream || f.FileHeader.isEmptyFile { // Return empty reader for directory or empty file return io.NopCloser(bytes.NewReader(nil)), nil } - var err error - rc, _ := f.zip.pool[f.folder].Get(f.offset) if rc == nil { - rc, _, err = f.zip.folderReader(f.zip.si, f.folder) + var ( + encrypted bool + err error + ) + + rc, _, encrypted, err = f.zip.folderReader(f.zip.si, f.folder) if err != nil { - return nil, err + return nil, &ReadError{ + Encrypted: encrypted, + Err: err, + } } } - if _, err = rc.Seek(f.offset, io.SeekStart); err != nil { - return nil, err + if _, err := rc.Seek(f.offset, io.SeekStart); err != nil { + e := &ReadError{ + Err: err, + } + + if fr, ok := rc.(*folderReadCloser); ok { + e.Encrypted = fr.hasEncryption + } + + return nil, e } return &fileReader{ @@ -144,22 +189,22 @@ func (f *File) Open() (io.ReadCloser, error) { } // OpenReaderWithPassword will open the 7-zip file specified by name using -// password as the basis of the decryption key and return a ReadCloser. If +// password as the basis of the decryption key and return a [*ReadCloser]. If // name has a ".001" suffix it is assumed there are multiple volumes and each // sequential volume will be opened. // //nolint:cyclop,funlen func OpenReaderWithPassword(name, password string) (*ReadCloser, error) { - f, err := os.Open(name) + f, err := os.Open(filepath.Clean(name)) if err != nil { - return nil, err + return nil, fmt.Errorf("sevenzip: error opening: %w", err) } info, err := f.Stat() if err != nil { err = multierror.Append(err, f.Close()) - return nil, err + return nil, fmt.Errorf("sevenzip: error retrieving file info: %w", err) } var reader io.ReaderAt = f @@ -181,7 +226,7 @@ func OpenReaderWithPassword(name, password string) (*ReadCloser, error) { err = multierror.Append(err, file.Close()) } - return nil, err + return nil, fmt.Errorf("sevenzip: error opening: %w", err) } files = append(files, f) @@ -192,7 +237,7 @@ func OpenReaderWithPassword(name, password string) (*ReadCloser, error) { err = multierror.Append(err, file.Close()) } - return nil, err + return nil, fmt.Errorf("sevenzip: error retrieving file info: %w", err) } sr = append(sr, io.NewSectionReader(f, 0, info.Size())) @@ -210,7 +255,7 @@ func OpenReaderWithPassword(name, password string) (*ReadCloser, error) { err = multierror.Append(err, file.Close()) } - return nil, err + return nil, fmt.Errorf("sevenzip: error initialising: %w", err) } r.f = files @@ -219,18 +264,18 @@ func OpenReaderWithPassword(name, password string) (*ReadCloser, error) { } // OpenReader will open the 7-zip file specified by name and return a -// ReadCloser. If name has a ".001" suffix it is assumed there are multiple +// [*ReadCloser]. If name has a ".001" suffix it is assumed there are multiple // volumes and each sequential volume will be opened. func OpenReader(name string) (*ReadCloser, error) { return OpenReaderWithPassword(name, "") } -// NewReaderWithPassword returns a new Reader reading from r using password as -// the basis of the decryption key, which is assumed to have the given size in -// bytes. +// NewReaderWithPassword returns a new [*Reader] reading from r using password +// as the basis of the decryption key, which is assumed to have the given size +// in bytes. func NewReaderWithPassword(r io.ReaderAt, size int64, password string) (*Reader, error) { if size < 0 { - return nil, errors.New("sevenzip: size cannot be negative") + return nil, errNegativeSize } zr := new(Reader) @@ -243,13 +288,13 @@ func NewReaderWithPassword(r io.ReaderAt, size int64, password string) (*Reader, return zr, nil } -// NewReader returns a new Reader reading from r, which is assumed to have the -// given size in bytes. +// NewReader returns a new [*Reader] reading from r, which is assumed to have +// the given size in bytes. func NewReader(r io.ReaderAt, size int64) (*Reader, error) { return NewReaderWithPassword(r, size, "") } -func (z *Reader) folderReader(si *streamsInfo, f int) (*folderReadCloser, uint32, error) { +func (z *Reader) folderReader(si *streamsInfo, f int) (*folderReadCloser, uint32, bool, error) { // Create a SectionReader covering all of the streams data return si.FolderReader(io.NewSectionReader(z.r, z.start, z.end-z.start), f, z.p) } @@ -286,21 +331,24 @@ func findSignature(r io.ReaderAt, search []byte) ([]int64, error) { break } - return nil, err + return nil, fmt.Errorf("sevenzip: error reading chunk: %w", err) } } return offsets, nil } -//nolint:cyclop,funlen,gocognit,gocyclo -func (z *Reader) init(r io.ReaderAt, size int64) error { +//nolint:cyclop,funlen,gocognit,gocyclo,maintidx +func (z *Reader) init(r io.ReaderAt, size int64) (err error) { h := crc32.NewIEEE() tra := plumbing.TeeReaderAt(r, h) - signature := []byte{'7', 'z', 0xbc, 0xaf, 0x27, 0x1c} + var ( + signature = []byte{'7', 'z', 0xbc, 0xaf, 0x27, 0x1c} + offsets []int64 + ) - offsets, err := findSignature(r, signature) + offsets, err = findSignature(r, signature) if err != nil { return err } @@ -320,7 +368,7 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { var sh signatureHeader if err = binary.Read(sr, binary.LittleEndian, &sh); err != nil { - return err + return fmt.Errorf("sevenzip: error reading signature header: %w", err) } z.r = r @@ -328,7 +376,7 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { h.Reset() if err = binary.Read(sr, binary.LittleEndian, &start); err != nil { - return err + return fmt.Errorf("sevenzip: error reading start header: %w", err) } // CRC of the start header should match @@ -345,12 +393,12 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { // Work out where we are in the file (32, avoiding magic numbers) if z.start, err = sr.Seek(0, io.SeekCurrent); err != nil { - return err + return fmt.Errorf("sevenzip: error seeking current position: %w", err) } // Seek over the streams if z.end, err = sr.Seek(int64(start.Offset), io.SeekCurrent); err != nil { //nolint:gosec - return err + return fmt.Errorf("sevenzip: error seeking over streams: %w", err) } z.start += off @@ -361,16 +409,16 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { // Bound bufio.Reader otherwise it can read trailing garbage which screws up the CRC check br := bufio.NewReader(io.NewSectionReader(tra, z.end, int64(start.Size))) //nolint:gosec - id, err := br.ReadByte() - if err != nil { - return err - } - var ( + id byte header *header streamsInfo *streamsInfo ) + if id, err = br.ReadByte(); err != nil { + return fmt.Errorf("sevenzip: error reading header id: %w", err) + } + switch id { case idHeader: if header, err = readHeader(br); err != nil { @@ -399,17 +447,32 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { // to decode it if streamsInfo != nil { if streamsInfo.Folders() != 1 { - return errors.New("sevenzip: expected only one folder in header stream") + return errOneHeaderStream } - fr, crc, err := z.folderReader(streamsInfo, 0) + var ( + fr *folderReadCloser + crc uint32 + encrypted bool + ) + + fr, crc, encrypted, err = z.folderReader(streamsInfo, 0) if err != nil { - return err + return &ReadError{ + Encrypted: encrypted, + Err: err, + } } - defer fr.Close() + + defer func() { + err = multierror.Append(err, fr.Close()).ErrorOrNil() + }() if header, err = readEncodedHeader(util.ByteReadCloser(fr)); err != nil { - return err + return &ReadError{ + Encrypted: fr.hasEncryption, + Err: err, + } } if crc != 0 && !util.CRC32Equal(fr.Checksum(), crc) { @@ -476,7 +539,8 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { return nil } -// Volumes returns the list of volumes that have been opened as part of the current archive. +// Volumes returns the list of volumes that have been opened as part of the +// current archive. func (rc *ReadCloser) Volumes() []string { volumes := make([]string, len(rc.f)) for idx, f := range rc.f { @@ -487,13 +551,16 @@ func (rc *ReadCloser) Volumes() []string { } // Close closes the 7-zip file or volumes, rendering them unusable for I/O. -func (rc *ReadCloser) Close() error { - var err *multierror.Error +func (rc *ReadCloser) Close() (err error) { for _, f := range rc.f { - err = multierror.Append(err, f.Close()) + err = multierror.Append(err, f.Close()).ErrorOrNil() + } + + if err != nil { + err = fmt.Errorf("sevenzip: error closing: %w", err) } - return err.ErrorOrNil() + return err } type fileListEntry struct { @@ -510,7 +577,7 @@ type fileInfoDirEntry interface { func (e *fileListEntry) stat() (fileInfoDirEntry, error) { if e.isDup { - return nil, errors.New(e.name + ": duplicate entries in 7-zip file") + return nil, errors.New(e.name + ": duplicate entries in 7-zip file") //nolint:err113 } if !e.isDir { @@ -628,7 +695,7 @@ func fileEntryLess(x, y string) bool { } // Open opens the named file in the 7-zip archive, using the semantics of -// fs.FS.Open: paths are always slash separated, with no leading / or ../ +// [fs.FS.Open]: paths are always slash separated, with no leading / or ../ // elements. func (z *Reader) Open(name string) (fs.File, error) { z.initFileList() @@ -725,8 +792,10 @@ type openDir struct { func (d *openDir) Close() error { return nil } func (d *openDir) Stat() (fs.FileInfo, error) { return d.e.stat() } +var errIsDirectory = errors.New("is a directory") + func (d *openDir) Read([]byte) (int, error) { - return 0, &fs.PathError{Op: "read", Path: d.e.name, Err: errors.New("is a directory")} + return 0, &fs.PathError{Op: "read", Path: d.e.name, Err: errIsDirectory} } func (d *openDir) ReadDir(count int) ([]fs.DirEntry, error) { diff --git a/reader_test.go b/reader_test.go index 6169292..d9760ab 100644 --- a/reader_test.go +++ b/reader_test.go @@ -8,13 +8,16 @@ import ( "io" "path/filepath" "runtime" + "sync" "testing" "testing/fstest" "testing/iotest" "github.com/bodgit/sevenzip" "github.com/bodgit/sevenzip/internal/util" + "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" ) @@ -22,13 +25,15 @@ func reader(r io.Reader) io.Reader { return r } +var errCRCMismatch = errors.New("CRC doesn't match") + func extractFile(tb testing.TB, r io.Reader, h hash.Hash, f *sevenzip.File) error { tb.Helper() h.Reset() if _, err := io.Copy(h, r); err != nil { - return err + return fmt.Errorf("error extracting file: %w", err) } if f.UncompressedSize > 0 && f.CRC32 == 0 { @@ -38,14 +43,14 @@ func extractFile(tb testing.TB, r io.Reader, h hash.Hash, f *sevenzip.File) erro } if !util.CRC32Equal(h.Sum(nil), f.CRC32) { - return errors.New("CRC doesn't match") + return errCRCMismatch } return nil } //nolint:lll -func extractArchive(tb testing.TB, r *sevenzip.ReadCloser, stream int, h hash.Hash, fn func(io.Reader) io.Reader, optimised bool) error { +func extractArchive(tb testing.TB, r *sevenzip.ReadCloser, stream int, h hash.Hash, fn func(io.Reader) io.Reader, optimised bool) (err error) { tb.Helper() for _, f := range r.File { @@ -53,18 +58,25 @@ func extractArchive(tb testing.TB, r *sevenzip.ReadCloser, stream int, h hash.Ha continue } - rc, err := f.Open() + var rc io.ReadCloser + + rc, err = f.Open() if err != nil { - return err + return fmt.Errorf("error opening file: %w", err) } - defer rc.Close() + + defer func() { + err = multierror.Append(err, rc.Close()).ErrorOrNil() + }() if err = extractFile(tb, fn(rc), h, f); err != nil { return err } if optimised { - rc.Close() + if err = rc.Close(); err != nil { + return fmt.Errorf("error closing: %w", err) + } } } @@ -194,12 +206,19 @@ func TestOpenReader(t *testing.T) { t.Parallel() r, err := sevenzip.OpenReader(filepath.Join("testdata", table.file)) - if err != nil { + if table.err == nil { + require.NoError(t, err) + } else { assert.ErrorIs(t, err, table.err) return } - defer r.Close() + + defer func() { + if err := r.Close(); err != nil { + t.Fatal(err) + } + }() volumes := []string{} @@ -236,6 +255,16 @@ func TestOpenReaderWithPassword(t *testing.T) { file: "t3.7z", password: "password", }, + { + name: "unencrypted headers compressed files", + file: "t4.7z", + password: "password", + }, + { + name: "unencrypted headers uncompressed files", + file: "t5.7z", + password: "password", + }, { name: "issue 75", file: "7zcracker.7z", @@ -253,7 +282,12 @@ func TestOpenReaderWithPassword(t *testing.T) { if err != nil { t.Fatal(err) } - defer r.Close() + + defer func() { + if err := r.Close(); err != nil { + t.Fatal(err) + } + }() if err := extractArchive(t, r, -1, crc32.NewIEEE(), iotest.OneByteReader, true); err != nil { t.Fatal(err) @@ -262,6 +296,53 @@ func TestOpenReaderWithPassword(t *testing.T) { } } +func TestOpenReaderWithWrongPassword(t *testing.T) { + t.Parallel() + + t.Run("encrypted headers", func(t *testing.T) { + t.Parallel() + + _, err := sevenzip.OpenReaderWithPassword(filepath.Join("testdata", "t2.7z"), "notpassword") + + var e *sevenzip.ReadError + if assert.ErrorAs(t, err, &e) { + assert.True(t, e.Encrypted) + } + }) + + t.Run("unencrypted headers compressed files", func(t *testing.T) { + t.Parallel() + + r, err := sevenzip.OpenReaderWithPassword(filepath.Join("testdata", "t4.7z"), "notpassword") + require.NoError(t, err) + + defer func() { + require.NoError(t, r.Close()) + }() + + err = extractArchive(t, r, -1, crc32.NewIEEE(), iotest.OneByteReader, true) + + var e *sevenzip.ReadError + if assert.ErrorAs(t, err, &e) { + assert.True(t, e.Encrypted) + } + }) + + t.Run("unencrypted headers uncompressed files", func(t *testing.T) { + t.Parallel() + + r, err := sevenzip.OpenReaderWithPassword(filepath.Join("testdata", "t5.7z"), "notpassword") + require.NoError(t, err) + + defer func() { + require.NoError(t, r.Close()) + }() + + err = extractArchive(t, r, -1, crc32.NewIEEE(), iotest.OneByteReader, true) + assert.ErrorIs(t, err, errCRCMismatch) + }) +} + func TestFS(t *testing.T) { t.Parallel() @@ -269,7 +350,12 @@ func TestFS(t *testing.T) { if err != nil { t.Fatal(err) } - defer r.Close() + + defer func() { + if err := r.Close(); err != nil { + t.Fatal(err) + } + }() if err := fstest.TestFS(r, "Asm/arm/7zCrcOpt.asm", "bin/x64/7zr.exe"); err != nil { t.Fatal(err) @@ -281,7 +367,12 @@ func ExampleOpenReader() { if err != nil { panic(err) } - defer r.Close() + + defer func() { + if err := r.Close(); err != nil { + panic(err) + } + }() for _, file := range r.File { fmt.Println(file.Name) @@ -306,7 +397,16 @@ func benchmarkArchiveParallel(b *testing.B, file string) { if err != nil { b.Fatal(err) } - defer r.Close() + + var once sync.Once + + f := func() { + if err := r.Close(); err != nil { + b.Fatal(err) + } + } + + defer once.Do(f) streams := make(map[int]struct{}, len(r.File)) @@ -329,7 +429,7 @@ func benchmarkArchiveParallel(b *testing.B, file string) { b.Fatal(err) } - r.Close() + once.Do(f) } } @@ -341,7 +441,16 @@ func benchmarkArchiveNaiveParallel(b *testing.B, file string, workers int) { if err != nil { b.Fatal(err) } - defer r.Close() + + var once sync.Once + + f := func() { + if err := r.Close(); err != nil { + b.Fatal(err) + } + } + + defer once.Do(f) eg := new(errgroup.Group) eg.SetLimit(workers) @@ -349,12 +458,17 @@ func benchmarkArchiveNaiveParallel(b *testing.B, file string, workers int) { for _, f := range r.File { f := f - eg.Go(func() error { - rc, err := f.Open() + eg.Go(func() (err error) { + var rc io.ReadCloser + + rc, err = f.Open() if err != nil { - return err + return fmt.Errorf("error opening file: %w", err) } - defer rc.Close() + + defer func() { + err = multierror.Append(err, rc.Close()).ErrorOrNil() + }() return extractFile(b, rc, crc32.NewIEEE(), f) }) @@ -364,7 +478,7 @@ func benchmarkArchiveNaiveParallel(b *testing.B, file string, workers int) { b.Fatal(err) } - r.Close() + once.Do(f) } } @@ -378,13 +492,22 @@ func benchmarkArchive(b *testing.B, file, password string, optimised bool) { if err != nil { b.Fatal(err) } - defer r.Close() + + var once sync.Once + + f := func() { + if err := r.Close(); err != nil { + b.Fatal(err) + } + } + + defer once.Do(f) if err := extractArchive(b, r, -1, h, reader, optimised); err != nil { b.Fatal(err) } - r.Close() + once.Do(f) } } diff --git a/register.go b/register.go index a08a679..4cb8abc 100644 --- a/register.go +++ b/register.go @@ -24,12 +24,16 @@ import ( // one io.ReadCloser's providing the stream(s) of bytes. type Decompressor func([]byte, uint64, []io.ReadCloser) (io.ReadCloser, error) -//nolint:gochecknoglobals -var decompressors sync.Map +var ( + //nolint:gochecknoglobals + decompressors sync.Map + + errNeedOneReader = errors.New("copy: need exactly one reader") +) func newCopyReader(_ []byte, _ uint64, readers []io.ReadCloser) (io.ReadCloser, error) { if len(readers) != 1 { - return nil, errors.New("sevenzip: need exactly one reader") + return nil, errNeedOneReader } // just return the passed io.ReadCloser) return readers[0], nil diff --git a/struct.go b/struct.go index f3ce50e..1ccebdf 100644 --- a/struct.go +++ b/struct.go @@ -3,6 +3,7 @@ package sevenzip import ( "bufio" "errors" + "fmt" "hash" "hash/crc32" "io" @@ -14,7 +15,16 @@ import ( "github.com/bodgit/sevenzip/internal/util" ) -var errAlgorithm = errors.New("sevenzip: unsupported compression algorithm") +var ( + errAlgorithm = errors.New("sevenzip: unsupported compression algorithm") + errInvalidWhence = errors.New("invalid whence") + errNegativeSeek = errors.New("negative seek") + errSeekBackwards = errors.New("cannot seek backwards") + errSeekEOF = errors.New("cannot seek beyond EOF") + errMultipleOutputStreams = errors.New("more than one output stream") + errNoBoundStream = errors.New("cannot find bound stream") + errNoUnboundStream = errors.New("expecting one unbound output stream") +) // CryptoReadCloser adds a Password method to decompressors. type CryptoReadCloser interface { @@ -80,31 +90,33 @@ func (f *folder) findOutBindPair(i uint64) *bindPair { return nil } -func (f *folder) coderReader(readers []io.ReadCloser, coder uint64, password string) (io.ReadCloser, error) { +func (f *folder) coderReader(readers []io.ReadCloser, coder uint64, password string) (io.ReadCloser, bool, error) { dcomp := decompressor(f.coder[coder].id) if dcomp == nil { - return nil, errAlgorithm + return nil, false, errAlgorithm } cr, err := dcomp(f.coder[coder].properties, f.size[coder], readers) if err != nil { - return nil, err + return nil, false, err } - if crc, ok := cr.(CryptoReadCloser); ok { + crc, ok := cr.(CryptoReadCloser) + if ok { if err = crc.Password(password); err != nil { - return nil, err + return nil, true, fmt.Errorf("sevenzip: error setting password: %w", err) } } - return plumbing.LimitReadCloser(cr, int64(f.size[coder])), nil //nolint:gosec + return plumbing.LimitReadCloser(cr, int64(f.size[coder])), ok, nil //nolint:gosec } type folderReadCloser struct { io.ReadCloser - h hash.Hash - wc *plumbing.WriteCounter - size int64 + h hash.Hash + wc *plumbing.WriteCounter + size int64 + hasEncryption bool } func (rc *folderReadCloser) Checksum() []byte { @@ -122,23 +134,23 @@ func (rc *folderReadCloser) Seek(offset int64, whence int) (int64, error) { case io.SeekEnd: newo = rc.Size() + offset default: - return 0, errors.New("invalid whence") + return 0, errInvalidWhence } if newo < 0 { - return 0, errors.New("negative seek") + return 0, errNegativeSeek } if uint64(newo) < rc.wc.Count() { - return 0, errors.New("cannot seek backwards") + return 0, errSeekBackwards } if newo > rc.Size() { - return 0, errors.New("cannot seek beyond EOF") + return 0, errSeekEOF } if _, err := io.CopyN(io.Discard, rc, newo-int64(rc.wc.Count())); err != nil { //nolint:gosec - return 0, err + return 0, fmt.Errorf("sevenzip: error seeking: %w", err) } return newo, nil @@ -148,12 +160,13 @@ func (rc *folderReadCloser) Size() int64 { return rc.size } -func newFolderReadCloser(rc io.ReadCloser, size int64) *folderReadCloser { +func newFolderReadCloser(rc io.ReadCloser, size int64, hasEncryption bool) *folderReadCloser { nrc := new(folderReadCloser) nrc.h = crc32.NewIEEE() nrc.wc = new(plumbing.WriteCounter) nrc.ReadCloser = plumbing.TeeReadCloser(rc, io.MultiWriter(nrc.h, nrc.wc)) nrc.size = size + nrc.hasEncryption = hasEncryption return nrc } @@ -208,7 +221,7 @@ func (si *streamsInfo) FileFolderAndSize(file int) (int, uint64) { if si.subStreamsInfo != nil { for folder, streams = range si.subStreamsInfo.streams { total += streams - if uint64(file) < total { + if uint64(file) < total { //nolint:gosec break } } @@ -235,8 +248,8 @@ func (si *streamsInfo) folderOffset(folder int) int64 { return int64(si.packInfo.position + offset) //nolint:gosec } -//nolint:cyclop,funlen -func (si *streamsInfo) FolderReader(r io.ReaderAt, folder int, password string) (*folderReadCloser, uint32, error) { +//nolint:cyclop,funlen,lll +func (si *streamsInfo) FolderReader(r io.ReaderAt, folder int, password string) (*folderReadCloser, uint32, bool, error) { f := si.unpackInfo.folder[folder] in := make([]io.ReadCloser, f.in) out := make([]io.ReadCloser, f.out) @@ -254,11 +267,14 @@ func (si *streamsInfo) FolderReader(r io.ReaderAt, folder int, password string) offset += size } - input, output := uint64(0), uint64(0) + var ( + hasEncryption bool + input, output uint64 + ) for i, c := range f.coder { if c.out != 1 { - return nil, 0, errors.New("more than one output stream") + return nil, 0, hasEncryption, errMultipleOutputStreams } for j := input; j < input+c.in; j++ { @@ -268,17 +284,24 @@ func (si *streamsInfo) FolderReader(r io.ReaderAt, folder int, password string) bp := f.findInBindPair(j) if bp == nil || out[bp.out] == nil { - return nil, 0, errors.New("cannot find bound stream") + return nil, 0, hasEncryption, errNoBoundStream } in[j] = out[bp.out] } - var err error + var ( + isEncrypted bool + err error + ) - out[output], err = f.coderReader(in[input:input+c.in], uint64(i), password) + out[output], isEncrypted, err = f.coderReader(in[input:input+c.in], uint64(i), password) //nolint:gosec if err != nil { - return nil, 0, err + return nil, 0, hasEncryption, err + } + + if isEncrypted { + hasEncryption = true } input += c.in @@ -294,16 +317,16 @@ func (si *streamsInfo) FolderReader(r io.ReaderAt, folder int, password string) } if len(unbound) != 1 || out[unbound[0]] == nil { - return nil, 0, errors.New("expecting one unbound output stream") + return nil, 0, hasEncryption, errNoUnboundStream } - fr := newFolderReadCloser(out[unbound[0]], int64(f.unpackSize())) //nolint:gosec + fr := newFolderReadCloser(out[unbound[0]], int64(f.unpackSize()), hasEncryption) //nolint:gosec if si.unpackInfo.digest != nil { - return fr, si.unpackInfo.digest[folder], nil + return fr, si.unpackInfo.digest[folder], hasEncryption, nil } - return fr, 0, nil + return fr, 0, hasEncryption, nil } type filesInfo struct { diff --git a/testdata/t4.7z b/testdata/t4.7z new file mode 100644 index 0000000000000000000000000000000000000000..53c8f5ddf0cd550f3072d3511a7346ea8cd3660e GIT binary patch literal 189 zcmXr7+Ou9=hJi)Wo@;+C0|Y2QY4ewt-t0dS@kI36++3?&0w(i4o=s+8Xf$SD$A9kA zCLZ<;hi`q4ckP-U+@u*egHeD<=(4TKw8}fYKD$kqG&<}$I6LN7BH#Pds>&`Vd6t6w zwU=1G$se1RGi%De+l5h9CX*_JBz;PEOUc&!Uiv;g_3^8jyAAATJP$j_$A9++OC!(` k;%owpoVg6_+>8v2%FK+6tg#FN3=BMFT#RAur-~UE0Jo?@(EtDd literal 0 HcmV?d00001 diff --git a/testdata/t5.7z b/testdata/t5.7z new file mode 100644 index 0000000000000000000000000000000000000000..e9c7527b796e6d8b6851b24620302c7ebc621a24 GIT binary patch literal 217 zcmXr7+Ou9=hJl6Cb>Fuc3=p6QrCse6x*yM(%_TJL);7z0DSt=99e=wX%_@yc`|`u= zrA@jU-;2i#42{O@>kMZmaZV7uCa1WxT2NnX2@gZ=?qK7CYkAF--1lE@;|(%lmj2hN zS#;Y(VY%n}xO27XAre=YH-EnpKhHKk<-P3XtdpOjo{B5xge~KE^LKs9>)v^rHypF_ zPC4zC>v~f@oLy>L>Gr=~@&$>Z)tQaALxJuSXH#J0tYBd0W@KPgW@cn$jb#vEVBl$( N#>J=>rq#*70039lPCftt literal 0 HcmV?d00001 diff --git a/types.go b/types.go index c518d41..57583ee 100644 --- a/types.go +++ b/types.go @@ -46,9 +46,10 @@ const ( ) var ( - errIncompleteRead = errors.New("sevenzip: incomplete read") - errUnexpectedID = errors.New("sevenzip: unexpected id") - errMissingUnpackInfo = errors.New("sevenzip: missing unpack info") + errIncompleteRead = errors.New("sevenzip: incomplete read") + errUnexpectedID = errors.New("sevenzip: unexpected id") + errMissingUnpackInfo = errors.New("sevenzip: missing unpack info") + errWrongNumberOfFilenames = errors.New("sevenzip: wrong number of filenames") ) func readUint64(r io.ByteReader) (uint64, error) { @@ -240,7 +241,7 @@ func readCoder(r util.Reader) (*coder, error) { } c.properties = make([]byte, size) - if n, err := r.Read(c.properties); err != nil || uint64(n) != size { + if n, err := r.Read(c.properties); err != nil || uint64(n) != size { //nolint:gosec if err != nil { return nil, fmt.Errorf("readCoder: Read error: %w", err) } @@ -619,7 +620,7 @@ func readNames(r util.Reader, count, length uint64) ([]string, error) { } if i != count { - return nil, errors.New("sevenzip: wrong number of filenames") + return nil, errWrongNumberOfFilenames } return names, nil