From 5a53f65ef34de482fab4553822c82c905b9bf898 Mon Sep 17 00:00:00 2001 From: Dan Kortschak Date: Thu, 10 Feb 2022 11:38:05 +1030 Subject: [PATCH] manual replacement of github.com/pkg/error with standard library error handling This is the manual part of the change and includes some complete removal of errors and some silently passed bugs resulting from the use of Wrap. In one place the exact semantics of errors.Cause is implemented to fit in with a testing strategy. --- auditbeat/helper/hasher/hasher_test.go | 17 ++++++++++++++++- auditbeat/module/auditd/audit_linux.go | 5 ++++- auditbeat/module/file_integrity/config.go | 8 +++++--- auditbeat/module/file_integrity/event.go | 11 +++++++---- .../module/file_integrity/eventreader_test.go | 3 ++- .../module/file_integrity/fileorigin_darwin.go | 16 ++++++++++++---- .../module/system/socket/guess/guess.go | 5 ++++- .../module/system/socket/helper/loopback.go | 5 ++++- 8 files changed, 54 insertions(+), 16 deletions(-) diff --git a/auditbeat/helper/hasher/hasher_test.go b/auditbeat/helper/hasher/hasher_test.go index 67c02de81dd..a50d3cc0c07 100644 --- a/auditbeat/helper/hasher/hasher_test.go +++ b/auditbeat/helper/hasher/hasher_test.go @@ -87,5 +87,20 @@ func TestHasherLimits(t *testing.T) { hashes, err := hasher.HashFile(file) assert.Empty(t, hashes) assert.Error(t, err) - assert.IsType(t, FileTooLargeError{}, errors.Cause(err)) + assert.IsType(t, FileTooLargeError{}, cause(err)) +} + +func cause(err error) error { + type unwrapper interface { + Unwrap() error + } + + for err != nil { + w, ok := err.(unwrapper) + if !ok { + break + } + err = w.Unwrap() + } + return err } diff --git a/auditbeat/module/auditd/audit_linux.go b/auditbeat/module/auditd/audit_linux.go index 1ac11f97dd4..63febb6fcec 100644 --- a/auditbeat/module/auditd/audit_linux.go +++ b/auditbeat/module/auditd/audit_linux.go @@ -306,7 +306,10 @@ func (ms *MetricSet) initClient() error { // required to ensure that auditing is enabled if the process is only // given CAP_AUDIT_READ. err := ms.client.SetEnabled(true, libaudit.NoWait) - return errors.Wrap(err, "failed to enable auditing in the kernel") + if err != nil { + return fmt.Errorf("failed to enable auditing in the kernel: %w", err) + } + return nil } // Unicast client initialization (requires CAP_AUDIT_CONTROL and that the diff --git a/auditbeat/module/file_integrity/config.go b/auditbeat/module/file_integrity/config.go index 6fa67303e36..f1fa35ebdca 100644 --- a/auditbeat/module/file_integrity/config.go +++ b/auditbeat/module/file_integrity/config.go @@ -114,10 +114,12 @@ nextHash: } c.MaxFileSizeBytes, err = humanize.ParseBytes(c.MaxFileSize) - if err != nil || c.MaxFileSizeBytes > MaxValidFileSizeLimit { - errs = append(errs, errors.Wrap(err, "invalid max_file_size value")) + if err != nil { + errs = append(errs, fmt.Errorf("invalid max_file_size value: %w", err)) + } else if c.MaxFileSizeBytes > MaxValidFileSizeLimit { + errs = append(errs, fmt.Errorf("invalid max_file_size value: %s is too large (max=%s)", c.MaxFileSize, humanize.Bytes(MaxValidFileSizeLimit))) } else if c.MaxFileSizeBytes <= 0 { - errs = append(errs, errors.Errorf("max_file_size value (%v) must be positive", c.MaxFileSize)) + errs = append(errs, fmt.Errorf("max_file_size value (%v) must be positive", c.MaxFileSize)) } c.ScanRateBytesPerSec, err = humanize.ParseBytes(c.ScanRatePerSec) diff --git a/auditbeat/module/file_integrity/event.go b/auditbeat/module/file_integrity/event.go index aa4123c2b1f..5e423d04fec 100644 --- a/auditbeat/module/file_integrity/event.go +++ b/auditbeat/module/file_integrity/event.go @@ -213,11 +213,14 @@ func NewEvent( hashTypes []HashType, ) Event { info, err := os.Lstat(path) - if err != nil && os.IsNotExist(err) { - // deleted file is signaled by info == nil - err = nil + if err != nil { + if os.IsNotExist(err) { + // deleted file is signaled by info == nil + err = nil + } else { + err = fmt.Errorf("failed to lstat: %w", err) + } } - err = errors.Wrap(err, "failed to lstat") return NewEventFromFileInfo(path, info, err, action, source, maxFileSize, hashTypes) } diff --git a/auditbeat/module/file_integrity/eventreader_test.go b/auditbeat/module/file_integrity/eventreader_test.go index 6caaa3a0551..f18d0817df3 100644 --- a/auditbeat/module/file_integrity/eventreader_test.go +++ b/auditbeat/module/file_integrity/eventreader_test.go @@ -23,6 +23,7 @@ import ( "os" "path/filepath" "runtime" + "runtime/debug" "strings" "syscall" "testing" @@ -313,7 +314,7 @@ func TestRaces(t *testing.T) { func readTimeout(t testing.TB, events <-chan Event) Event { select { case <-time.After(time.Second): - t.Fatalf("%+v", errors.Errorf("timed-out waiting for event")) + t.Fatalf("timed-out waiting for event:\n%s", debug.Stack()) case e, ok := <-events: if !ok { t.Fatal("failed reading from event channel") diff --git a/auditbeat/module/file_integrity/fileorigin_darwin.go b/auditbeat/module/file_integrity/fileorigin_darwin.go index 740987f25fe..227bef1e4da 100644 --- a/auditbeat/module/file_integrity/fileorigin_darwin.go +++ b/auditbeat/module/file_integrity/fileorigin_darwin.go @@ -74,9 +74,13 @@ func GetFileOrigin(path string) ([]string, error) { defer C.free(unsafe.Pointer(cPath)) // Query length kMDItemWhereFroms extended-attribute - attrSize, err := C.getxattr(cPath, kMDItemWhereFroms, nil, 0, 0, 0) + attrSize, errno := C.getxattr(cPath, kMDItemWhereFroms, nil, 0, 0, 0) if attrSize == -1 { - return nil, errors.Wrap(filterErrno(err), "getxattr: query attribute length failed") + err := filterErrno(errno) + if err != nil { + return nil, fmt.Errorf("getxattr: query attribute length failed: %w", err) + } + return nil, nil } if attrSize == 0 { return nil, nil @@ -84,9 +88,13 @@ func GetFileOrigin(path string) ([]string, error) { // Read the kMDItemWhereFroms attribute data := make([]byte, attrSize) - newSize, err := C.getxattr(cPath, kMDItemWhereFroms, unsafe.Pointer(&data[0]), C.size_t(attrSize), 0, 0) + newSize, errno := C.getxattr(cPath, kMDItemWhereFroms, unsafe.Pointer(&data[0]), C.size_t(attrSize), 0, 0) if newSize == -1 { - return nil, errors.Wrap(filterErrno(err), "getxattr failed") + err := filterErrno(errno) + if err != nil { + return nil, fmt.Errorf("getxattr failed: %w", filterErrno(err)) + } + return nil, nil } if newSize != attrSize { return nil, errors.New("getxattr: attribute changed while reading") diff --git a/x-pack/auditbeat/module/system/socket/guess/guess.go b/x-pack/auditbeat/module/system/socket/guess/guess.go index c31233b6d3d..59fd8f5df5c 100644 --- a/x-pack/auditbeat/module/system/socket/guess/guess.go +++ b/x-pack/auditbeat/module/system/socket/guess/guess.go @@ -236,7 +236,10 @@ func guessOnce(guesser Guesser, installer helper.ProbeInstaller, ctx Context) (r } case <-perfchan.LostC(): - return nil, errors.Wrap(err, "event loss in perf channel") + if err != nil { + return nil, fmt.Errorf("event loss in perf channel: %w", err) + } + return nil, errors.New("event loss in perf channel") } } } diff --git a/x-pack/auditbeat/module/system/socket/helper/loopback.go b/x-pack/auditbeat/module/system/socket/helper/loopback.go index ae576052213..eea98399049 100644 --- a/x-pack/auditbeat/module/system/socket/helper/loopback.go +++ b/x-pack/auditbeat/module/system/socket/helper/loopback.go @@ -112,7 +112,10 @@ func (lo *IPv6Loopback) AddRandomAddress() (addr net.IP, err error) { } time.Sleep(time.Millisecond * time.Duration(i)) } - return addr, errors.Wrap(err, "bind failed") + if err != nil { + err = fmt.Errorf("bind failed: %w", err) + } + return addr, err } // Cleanup removes the addresses registered to this loopback.