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

@@ -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

.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

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`.

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{

internal/aes7z/reader.go

@@ -1,14 +1,22 @@
+// Package aes7z implements the 7-zip AES decryption.
 package aes7z
 
 import (
 	"bytes"
 	"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,32 +73,37 @@ 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[:])
 
 		_, _ = 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
 // method must be called before attempting to call Read so that the block
 // 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]

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
 		}
 

internal/bra/bra.go

@@ -1,3 +1,4 @@
+// Package bra implements the branch rewriting filter for binaries.
 package bra
 
 type converter interface {