This is an automated email from the ASF dual-hosted git repository.
sruehl pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git
The following commit(s) were added to refs/heads/develop by this push:
new 63369e83eb fix(plc4go): survive improperly constructed error chains in
transport-error handling
63369e83eb is described below
commit 63369e83ebf0a400f6fa45eb43fcf20e0013a08e
Author: Sebastian Rühl <[email protected]>
AuthorDate: Thu Jul 2 11:05:53 2026 +0200
fix(plc4go): survive improperly constructed error chains in transport-error
handling
A transport error chain can contain a typed-nil *net.OpError (wrapped via
%w), whose Unwrap method dereferences a nil receiver. Two paths panicked on
such chains in the field (recovered, but killing the receive worker mid
error-handling):
1. defaultCodec.handleTransportError used stdErrors.Is directly, which
panics while unwrapping the poisoned chain. Switched to the
panic-guarded transports.ErrorIs that exists for exactly this purpose.
2. errors.MarshalStack (installed as zerolog's ErrorStackMarshaler) walks
the chain via errors.Unwrap on every logged error, panicking the same
way from inside any log statement that attaches such an error. The walk
is now panic-guarded and degrades to "no stack" instead.
Adds a regression test feeding a typed-nil *net.OpError chain through
handleTransportError (panicked pre-fix on both paths).
---
plc4go/spi/default/DefaultCodec.go | 9 +++--
.../DefaultCodecTransportErrorPanic_test.go | 47 ++++++++++++++++++++++
plc4go/spi/errors/marshal.go | 12 +++++-
3 files changed, 64 insertions(+), 4 deletions(-)
diff --git a/plc4go/spi/default/DefaultCodec.go
b/plc4go/spi/default/DefaultCodec.go
index 9a6c40b487..b328265627 100644
--- a/plc4go/spi/default/DefaultCodec.go
+++ b/plc4go/spi/default/DefaultCodec.go
@@ -21,7 +21,6 @@ package _default
import (
"context"
- stdErrors "errors"
"runtime/debug"
"slices"
"sync"
@@ -544,13 +543,17 @@ func (m *defaultCodec) handleTransportError(workerLog
zerolog.Logger, err error)
if err == nil {
return true
}
- if stdErrors.Is(err, context.Canceled) {
+ // Use the panic-guarded transports.ErrorIs here: the error chain
delivered by a
+ // transport can contain improperly constructed values (e.g. a typed-nil
+ // *net.OpError), and stdErrors.Is dereferences them while unwrapping -
which
+ // killed the receive worker with a recovered nil-pointer panic in the
field.
+ if transports.ErrorIs(err, context.Canceled) {
workerLog.Debug().Msg("receive aborted due to context
cancellation")
return false
}
kind := transports.TransportErrorUnknown
- if stdErrors.Is(err, context.DeadlineExceeded) {
+ if transports.ErrorIs(err, context.DeadlineExceeded) {
kind = transports.TransportErrorRetryable
} else if m.transportInstance != nil {
kind = m.transportInstance.ClassifyError(err)
diff --git a/plc4go/spi/default/DefaultCodecTransportErrorPanic_test.go
b/plc4go/spi/default/DefaultCodecTransportErrorPanic_test.go
new file mode 100644
index 0000000000..161a764b07
--- /dev/null
+++ b/plc4go/spi/default/DefaultCodecTransportErrorPanic_test.go
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package _default
+
+import (
+ "fmt"
+ "net"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apache/plc4x/plc4go/spi/testutils"
+)
+
+// A transport error chain can contain improperly constructed values - most
+// notably a typed-nil *net.OpError wrapped into the chain. stdErrors.Is
+// dereferences such values while unwrapping ((*net.OpError).Unwrap reads
+// e.Err on a nil receiver), which killed the receive worker with a
+// nil-pointer panic in the field. handleTransportError must survive such
+// chains and classify them as fatal instead of panicking.
+func Test_defaultCodec_handleTransportError_typedNilOpErrorInChain(t
*testing.T) {
+ var nilOpErr *net.OpError
+ poisoned := fmt.Errorf("read failed: %w", error(nilOpErr))
+
+ codec := &defaultCodec{}
+ assert.NotPanics(t, func() {
+ keepRunning :=
codec.handleTransportError(testutils.ProduceTestingLogger(t), poisoned)
+ assert.False(t, keepRunning, "poisoned chain must be treated as
fatal and stop the worker")
+ })
+}
diff --git a/plc4go/spi/errors/marshal.go b/plc4go/spi/errors/marshal.go
index 958b972e7b..f5093f4254 100644
--- a/plc4go/spi/errors/marshal.go
+++ b/plc4go/spi/errors/marshal.go
@@ -34,7 +34,17 @@ const (
// and returns its frames as []map[string]string keyed by StackSource*Name.
// Returns nil if no stack trace is found in the chain. Suitable for use as
// zerolog.ErrorStackMarshaler.
-func MarshalStack(err error) any {
+//
+// The walk is panic-guarded: error chains can contain improperly constructed
+// values (e.g. a typed-nil *net.OpError wrapped via %w) whose Unwrap method
+// dereferences a nil receiver. This marshaler runs inside zerolog on every
+// logged error, so a panic here would take down whatever worker is logging.
+func MarshalStack(err error) (result any) {
+ defer func() {
+ if r := recover(); r != nil {
+ result = nil
+ }
+ }()
type stackTracer interface {
StackTrace() StackTrace
}
