From e6d56c6d31340a86e01bae8408293f7219f2f8fa Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Mon, 5 Feb 2024 17:34:50 +0100
Subject: [PATCH 1/2] Fix dyn.Value equality check

This function could panic when either side of the comparison is a nil or empty
slice. This logic is triggered when comparing the input value to the output
value when calling `dyn.Map`.
---
 libs/dyn/value.go          | 14 +++++++++++-
 libs/dyn/visit_map_test.go | 46 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/libs/dyn/value.go b/libs/dyn/value.go
index a487e13e12..b3dbc7cf54 100644
--- a/libs/dyn/value.go
+++ b/libs/dyn/value.go
@@ -154,7 +154,19 @@ func (v Value) eq(w Value) bool {
 		// This is safe because we don't allow slices to be mutated.
 		vs := v.v.([]Value)
 		ws := w.v.([]Value)
-		return &vs[0] == &ws[0] && len(vs) == len(ws)
+		lv := len(vs)
+		lw := len(ws)
+		// If both slices are empty, they are equal.
+		if lv == 0 && lw == 0 {
+			return true
+		}
+		// If they have different lengths, they are not equal.
+		if lv != lw {
+			return false
+		}
+		// They are both non-empty and have the same length.
+		// Compare pointers to the underlying slice.
+		return &vs[0] == &ws[0]
 	default:
 		return v.v == w.v
 	}
diff --git a/libs/dyn/visit_map_test.go b/libs/dyn/visit_map_test.go
index a5af3411f7..117d03f0a6 100644
--- a/libs/dyn/visit_map_test.go
+++ b/libs/dyn/visit_map_test.go
@@ -74,6 +74,29 @@ func TestMapFuncOnMap(t *testing.T) {
 	assert.ErrorIs(t, err, ref)
 }
 
+func TestMapFuncOnMapWithEmptySequence(t *testing.T) {
+	variants := []dyn.Value{
+		// empty sequence
+		dyn.V([]dyn.Value{}),
+		// non-empty sequence
+		dyn.V([]dyn.Value{dyn.V(42)}),
+	}
+
+	for i := 0; i < len(variants); i++ {
+		vin := dyn.V(map[string]dyn.Value{
+			"key": variants[i],
+		})
+
+		for j := 0; j < len(variants); j++ {
+			vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Key("key")), func(v dyn.Value) (dyn.Value, error) {
+				return variants[j], nil
+			})
+			assert.NoError(t, err)
+			assert.Equal(t, variants[j], vout.Get("key"))
+		}
+	}
+}
+
 func TestMapFuncOnSequence(t *testing.T) {
 	vin := dyn.V([]dyn.Value{
 		dyn.V(42),
@@ -115,6 +138,29 @@ func TestMapFuncOnSequence(t *testing.T) {
 	assert.ErrorIs(t, err, ref)
 }
 
+func TestMapFuncOnSequenceWithEmptySequence(t *testing.T) {
+	variants := []dyn.Value{
+		// empty sequence
+		dyn.V([]dyn.Value{}),
+		// non-empty sequence
+		dyn.V([]dyn.Value{dyn.V(42)}),
+	}
+
+	for i := 0; i < len(variants); i++ {
+		vin := dyn.V([]dyn.Value{
+			variants[i],
+		})
+
+		for j := 0; j < len(variants); j++ {
+			vout, err := dyn.MapByPath(vin, dyn.NewPath(dyn.Index(0)), func(v dyn.Value) (dyn.Value, error) {
+				return variants[j], nil
+			})
+			assert.NoError(t, err)
+			assert.Equal(t, variants[j], vout.Index(0))
+		}
+	}
+}
+
 func TestMapForeachOnMap(t *testing.T) {
 	vin := dyn.V(map[string]dyn.Value{
 		"foo": dyn.V(42),

From 4e7f953421a3b8474c85aac69ab8a646da95f8ce Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Mon, 5 Feb 2024 17:49:13 +0100
Subject: [PATCH 2/2] Move comment

---
 libs/dyn/value.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libs/dyn/value.go b/libs/dyn/value.go
index b3dbc7cf54..e9c22bfbea 100644
--- a/libs/dyn/value.go
+++ b/libs/dyn/value.go
@@ -150,8 +150,6 @@ func (v Value) eq(w Value) bool {
 		// This is safe because we don't allow maps to be mutated.
 		return &v.v == &w.v
 	case KindSequence:
-		// Compare pointers to the underlying slice and slice length.
-		// This is safe because we don't allow slices to be mutated.
 		vs := v.v.([]Value)
 		ws := w.v.([]Value)
 		lv := len(vs)
@@ -166,6 +164,7 @@ func (v Value) eq(w Value) bool {
 		}
 		// They are both non-empty and have the same length.
 		// Compare pointers to the underlying slice.
+		// This is safe because we don't allow slices to be mutated.
 		return &vs[0] == &ws[0]
 	default:
 		return v.v == w.v