Skip to content

Commit 2247499

Browse files
mfleaderfredbi
authored andcommitted
fix: prevent panic on nil intermediate values during Set operations
Add nil checks in setSingleImpl() and traversal loop to handle nil intermediate values gracefully. Per RFC 6901 Section 7, implementations must raise error conditions for unresolvable paths instead of panicking. - Add isNil() check before reflection operations in setSingleImpl() - Add nil validation in set() method traversal loop - Return descriptive errors: "cannot set field X on nil value" - Add comprehensive test cases for nil traversal scenarios Fixes panic: "reflect: call of reflect.Value.Type on zero Value" when traversing JSON paths containing nil intermediate values. Signed-off-by: Matthew F Leader <[email protected]>
1 parent f485c21 commit 2247499

File tree

2 files changed

+98
-0
lines changed

2 files changed

+98
-0
lines changed

pointer.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,11 @@ func getSingleImpl(node any, decodedToken string, nameProvider *swag.NameProvide
179179
func setSingleImpl(node, data any, decodedToken string, nameProvider *swag.NameProvider) error {
180180
rValue := reflect.Indirect(reflect.ValueOf(node))
181181

182+
// Check for nil to prevent panic when calling rValue.Type()
183+
if isNil(node) {
184+
return fmt.Errorf("cannot set field %q on nil value: %w", decodedToken, ErrPointer)
185+
}
186+
182187
if ns, ok := node.(JSONSetable); ok { // pointer impl
183188
return ns.JSONSet(decodedToken, data)
184189
}
@@ -285,6 +290,11 @@ func (p *Pointer) set(node, data any, nameProvider *swag.NameProvider) error {
285290
return setSingleImpl(node, data, decodedToken, nameProvider)
286291
}
287292

293+
// Check for nil during traversal
294+
if isNil(node) {
295+
return fmt.Errorf("cannot traverse through nil value at %q: %w", decodedToken, ErrPointer)
296+
}
297+
288298
rValue := reflect.Indirect(reflect.ValueOf(node))
289299
kind := rValue.Kind()
290300

pointer_test.go

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,94 @@ func TestSetNode(t *testing.T) {
644644
assert.Equal(t, 999, setDoc.Coll.Items[0].C)
645645
})
646646
})
647+
648+
t.Run("with nil traversal panic", func(t *testing.T) {
649+
// This test exposes the panic that occurs when trying to set a value
650+
// through a path that contains nil intermediate values
651+
data := map[string]any{
652+
"level1": map[string]any{
653+
"level2": map[string]any{
654+
"level3": nil, // This nil causes the panic
655+
},
656+
},
657+
}
658+
659+
ptr, err := New("/level1/level2/level3/value")
660+
require.NoError(t, err)
661+
662+
// This should return an error, not panic
663+
_, err = ptr.Set(data, "test-value")
664+
665+
// The library should handle this gracefully and return an error
666+
// instead of panicking
667+
require.Error(t, err, "Setting value through nil intermediate path should return an error, not panic")
668+
})
669+
670+
t.Run("with direct nil map value", func(t *testing.T) {
671+
// Simpler test case that directly tests nil traversal
672+
data := map[string]any{
673+
"container": nil,
674+
}
675+
676+
ptr, err := New("/container/nested/value")
677+
require.NoError(t, err)
678+
679+
// Attempting to traverse through nil should return an error, not panic
680+
_, err = ptr.Set(data, "test")
681+
require.Error(t, err, "Cannot traverse through nil intermediate values")
682+
})
683+
684+
t.Run("with nil in nested structure", func(t *testing.T) {
685+
// Test case with multiple nil values in nested structure
686+
data := map[string]any{
687+
"config": map[string]any{
688+
"settings": nil,
689+
},
690+
"data": map[string]any{
691+
"nested": map[string]any{
692+
"properties": map[string]any{
693+
"attributes": nil, // Nil intermediate value
694+
},
695+
},
696+
},
697+
}
698+
699+
ptr, err := New("/data/nested/properties/attributes/name")
700+
require.NoError(t, err)
701+
702+
// Should return error, not panic
703+
_, err = ptr.Set(data, "test-name")
704+
require.Error(t, err, "Setting through nil intermediate path should return error")
705+
})
706+
707+
t.Run("with path creation through nil intermediate", func(t *testing.T) {
708+
// Test case that simulates path creation functions encountering nil
709+
// This happens when tools try to create missing paths but encounter nil intermediate values
710+
data := map[string]any{
711+
"spec": map[string]any{
712+
"template": nil, // This blocks path creation attempts
713+
},
714+
}
715+
716+
// Attempting to create a path like /spec/template/metadata/labels should fail gracefully
717+
ptr, err := New("/spec/template/metadata")
718+
require.NoError(t, err)
719+
720+
// Should return error when trying to set on nil intermediate during path creation
721+
_, err = ptr.Set(data, map[string]any{"labels": map[string]any{}})
722+
require.Error(t, err, "Setting on nil intermediate during path creation should return error")
723+
})
724+
725+
t.Run("with SetForToken on nil", func(t *testing.T) {
726+
// Test the single-level SetForToken function with nil
727+
data := map[string]any{
728+
"container": nil,
729+
}
730+
731+
// Should handle nil gracefully at single token level
732+
_, err := SetForToken(data["container"], "nested", "value")
733+
require.Error(t, err, "SetForToken on nil should return error, not panic")
734+
})
647735
}
648736

649737
func TestOffset(t *testing.T) {

0 commit comments

Comments
 (0)