fix(spew): fixed more panic & hang cases in the internalized spew (go-openapi#29)

* fixed case of circular reference that hangs
* fixed unrecoverable panic when dumping a map with a circular reference

test(integrationtest): added new module for integration tests

This new module takes a systematic approach to explore edge cases in
spew by producing random scenarios with uncommon data types.

This approach uncovered 2 bugs in spew (fixed above)... and one in the
fmt standard library.

This new module leverages the rapid library to build generators and exercises spew.Sdump.
The "property based" tests produced with rapid are generalized in a fuzz test.

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>

Author: fredbi

.golangci.yml

@@ -30,6 +30,8 @@ linters:
       max-complexity: 22
     gocyclo:
       min-complexity: 22
+    gocognit:
+      min-complexity: 35
     exhaustive:
       default-signifies-exhaustive: true
       default-case-required: true

go.work

@@ -2,6 +2,7 @@ use (
 	.
 	./codegen
 	./enable/yaml
+	./internal/testintegration
 )
 
 go 1.24.0

internal/assertions/doc.go

@@ -8,6 +8,8 @@
 // whereas the publicly exposed API (in packages assert and require)
 // is generated.
 //
+// For convenience, assertion functions are classified by domain.
+// The entire API can be searched by domain at https://go-openapi.github.io/testify/api.
 // # Domains
 //
 // - boolean: asserting boolean values

internal/spew/dump.go

@@ -97,11 +97,12 @@ func (d *dumpState) dumpPtr(v reflect.Value) {
 	cycleFound := false
 	indirects := 0
 	ve := v
-	for ve.Kind() == reflect.Ptr {
+	for ve.Kind() == reflect.Pointer {
 		if ve.IsNil() {
 			nilFound = true
 			break
 		}
+
 		indirects++
 		addr := ve.Pointer()
 		pointerChain = append(pointerChain, addr)
@@ -114,11 +115,27 @@ func (d *dumpState) dumpPtr(v reflect.Value) {
 
 		ve = ve.Elem()
 		if ve.Kind() == reflect.Interface {
-			if ve.IsNil() {
+			if ve.IsNil() { // interface with nil value
 				nilFound = true
 				break
 			}
 			ve = ve.Elem()
+			if ve.Kind() == reflect.Pointer {
+				if ve.IsNil() {
+					nilFound = true
+					break
+				}
+
+				// case of interface containing a pointer that cycles to the same depth level.
+				// If we have a cycle at the same level, we should break the loop now.
+				addr = ve.Pointer()
+				if pd, ok := d.pointers[addr]; ok && pd <= d.depth {
+					cycleFound = true
+					indirects--
+					break
+				}
+				d.pointers[addr] = d.depth
+			}
 		}
 	}
 
@@ -156,6 +173,64 @@ func (d *dumpState) dumpPtr(v reflect.Value) {
 	d.w.Write(closeParenBytes)
 }
 
+func (d *dumpState) dumpMap(v reflect.Value) {
+	// Remove pointers at or below the current depth from map used to detect
+	// circular refs.
+	for k, depth := range d.pointers {
+		if depth >= d.depth {
+			delete(d.pointers, k)
+		}
+	}
+
+	// Keep list of all dereferenced pointers to show later.
+	cycleFound := false
+
+	// nil maps should be indicated as different than empty maps
+	if v.IsNil() {
+		_, _ = d.w.Write(nilAngleBytes)
+		return
+	}
+
+	// maps like pointers may present circular references
+	addr := v.Pointer()
+	if pd, ok := d.pointers[addr]; ok && pd <= d.depth {
+		cycleFound = true
+	}
+	d.pointers[addr] = d.depth
+
+	_, _ = d.w.Write(openBraceNewlineBytes)
+	d.depth++
+
+	switch {
+	case d.cs.MaxDepth != 0 && d.depth > d.cs.MaxDepth:
+		d.indent()
+		_, _ = d.w.Write(maxNewlineBytes)
+	case cycleFound:
+		_, _ = d.w.Write(circularBytes)
+	default:
+		numEntries := v.Len()
+		keys := v.MapKeys()
+		if d.cs.SortKeys {
+			sortValues(keys, d.cs)
+		}
+		for i, key := range keys {
+			d.dump(d.unpackValue(key))
+			_, _ = d.w.Write(colonSpaceBytes)
+			d.ignoreNextIndent = true
+			d.dump(d.unpackValue(v.MapIndex(key)))
+			if i < (numEntries - 1) {
+				_, _ = d.w.Write(commaNewlineBytes)
+			} else {
+				_, _ = d.w.Write(newlineBytes)
+			}
+		}
+	}
+
+	d.depth--
+	d.indent()
+	_, _ = d.w.Write(closeBraceBytes)
+}
+
 // dumpSlice handles formatting of arrays and slices.  Byte (uint8 under
 // reflection) arrays and slices are dumped in hexdump -C fashion.
 func (d *dumpState) dumpSlice(v reflect.Value) {
@@ -257,7 +332,7 @@ func (d *dumpState) dump(v reflect.Value) {
 	}
 
 	// Handle pointers specially.
-	if kind == reflect.Ptr {
+	if kind == reflect.Pointer {
 		d.indent()
 		d.dumpPtr(v)
 		return
@@ -367,43 +442,12 @@ func (d *dumpState) dump(v reflect.Value) {
 			d.w.Write(nilAngleBytes)
 		}
 
-	case reflect.Ptr:
+	case reflect.Pointer:
 		// Do nothing.  We should never get here since pointers have already
 		// been handled above.
 
 	case reflect.Map:
-		// nil maps should be indicated as different than empty maps
-		if v.IsNil() {
-			d.w.Write(nilAngleBytes)
-			break
-		}
-
-		d.w.Write(openBraceNewlineBytes)
-		d.depth++
-		if (d.cs.MaxDepth != 0) && (d.depth > d.cs.MaxDepth) {
-			d.indent()
-			d.w.Write(maxNewlineBytes)
-		} else {
-			numEntries := v.Len()
-			keys := v.MapKeys()
-			if d.cs.SortKeys {
-				sortValues(keys, d.cs)
-			}
-			for i, key := range keys {
-				d.dump(d.unpackValue(key))
-				d.w.Write(colonSpaceBytes)
-				d.ignoreNextIndent = true
-				d.dump(d.unpackValue(v.MapIndex(key)))
-				if i < (numEntries - 1) {
-					d.w.Write(commaNewlineBytes)
-				} else {
-					d.w.Write(newlineBytes)
-				}
-			}
-		}
-		d.depth--
-		d.indent()
-		d.w.Write(closeBraceBytes)
+		d.dumpMap(v)
 
 	case reflect.Struct:
 		d.w.Write(openBraceNewlineBytes)

internal/spew/edgecases_test.go

@@ -0,0 +1,98 @@
+package spew
+
+import (
+	"io"
+	"iter"
+	"slices"
+	"sync"
+	"testing"
+)
+
+func TestEdgeCases(t *testing.T) {
+	t.Parallel()
+	cfg := Config
+	output := io.Discard
+
+	for tt := range edgeCases() {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			defer func() {
+				if r := recover(); r != nil {
+					t.Errorf("fdump panicked: %v\nWith value type: %T\nValue: %#v", r, tt.value, tt.value)
+
+					return
+				}
+			}()
+
+			fdump(&cfg, output, tt.value)
+		})
+	}
+}
+
+type edgeCase struct {
+	name  string
+	value any
+}
+
+func edgeCases() iter.Seq[edgeCase] {
+	type withLock struct {
+		sync.Mutex
+	}
+	type withLockPtr struct {
+		*sync.Mutex
+	}
+	var iface any = "x"
+	str := "y"
+	var ifaceToPtr any = &str
+	var ifaceToNilPtr any = (*string)(nil)
+	ifacePtr := &iface
+	ifacePtrPtr := &ifaceToPtr
+	var ifaceCircular any
+	ifaceCircular = &ifaceCircular
+
+	// Map with circular value (map contains itself)
+	m := map[string]any{
+		"key1": 1,
+		"key2": "val",
+	}
+	m["circular"] = m // Map contains itself as a value
+
+	return slices.Values([]edgeCase{
+		{
+			name:  "self-referencing map",
+			value: m,
+		},
+		{
+			name:  "sync.Mutex",
+			value: withLock{},
+		},
+		{
+			name: "*sync.Mutex",
+			value: withLockPtr{
+				Mutex: &sync.Mutex{},
+			},
+		},
+		{
+			name:  "pointer to interface",
+			value: ifacePtr,
+		},
+		{
+			name:  "pointer to interface to pointer",
+			value: ifacePtrPtr,
+		},
+		{
+			name:  "pointer to interface to pointer",
+			value: &ifaceToNilPtr,
+		},
+		{
+			name:  "pointer to pointer to interface to pointer",
+			value: &ifaceToPtr,
+		},
+		{
+			// case that used to hang
+			name:  "pointer to interface with circular reference",
+			value: &ifaceCircular,
+		},
+	})
+}