Skip to content

Commit 8c81e37

Browse files
erraggyclaude
andcommitted
fix: address CodeRabbit review findings from PR #359
- jsonpath: guard against typed-nil map[string]any in Modify root path (a typed nil passes the type assertion but panics on write) - parser: handle []any in promoteSchemaOrBool for OAS 2.0 tuple-form items arrays (matches existing decodeSchemaOrBool YAML path) - tests: add TestModifyRoot_InvalidDoc for non-map and typed-nil cases - tests: add Items bool and tuple-array round-trip cases to schema JSON tests - tests: add TestConvertOAS2ParameterToOAS3_ArrayWithoutItems for nil Items Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c1bed75 commit 8c81e37

5 files changed

Lines changed: 75 additions & 5 deletions

File tree

converter/helpers_test.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,6 +740,26 @@ func TestConvertOAS2ParameterToOAS3_ItemsCollectionFormat(t *testing.T) {
740740
"should warn about non-csv collectionFormat on items")
741741
}
742742

743+
// TestConvertOAS2ParameterToOAS3_ArrayWithoutItems verifies that an array
744+
// parameter with nil Items is handled gracefully.
745+
func TestConvertOAS2ParameterToOAS3_ArrayWithoutItems(t *testing.T) {
746+
c := newConverter()
747+
result := &ConversionResult{}
748+
749+
param := &parser.Parameter{
750+
Name: "tags",
751+
In: "query",
752+
Type: "array",
753+
// Items intentionally nil
754+
}
755+
756+
converted := c.convertOAS2ParameterToOAS3(param, result, "test")
757+
require.NotNil(t, converted)
758+
require.NotNil(t, converted.Schema)
759+
assert.Equal(t, "array", converted.Schema.Type, "Schema.Type should be array")
760+
assert.Nil(t, converted.Schema.Items, "Schema.Items should be nil when source has no Items")
761+
}
762+
743763
// newConverter creates a Converter for unit testing helpers using the same
744764
// initialization path as production code.
745765
func newConverter() *Converter {

internal/jsonpath/eval.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,12 @@ func (p *Path) Remove(doc any) (any, error) {
119119
// the caller's variable cannot be reassigned.
120120
func (p *Path) Modify(doc any, fn func(any) any) error {
121121
if len(p.segments) < 2 {
122-
if _, ok := doc.(map[string]any); !ok {
123-
return fmt.Errorf("jsonpath: root path Modify requires a map document; got %T", doc)
122+
m, ok := doc.(map[string]any)
123+
if !ok || m == nil {
124+
return fmt.Errorf("jsonpath: root path Modify requires a non-nil map document; got %T", doc)
124125
}
125126
// fn is expected to mutate the map in place; return value is ignored.
126-
fn(doc)
127+
fn(m)
127128
return nil
128129
}
129130

internal/jsonpath/jsonpath_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,24 @@ func TestModifyRoot(t *testing.T) {
10421042
assert.Equal(t, "api.example.com", doc["host"], "root Modify should merge into document")
10431043
}
10441044

1045+
// TestModifyRoot_InvalidDoc verifies that Modify on root path "$" returns an
1046+
// error for non-map and typed-nil map documents.
1047+
func TestModifyRoot_InvalidDoc(t *testing.T) {
1048+
p, err := Parse("$")
1049+
require.NoError(t, err)
1050+
1051+
t.Run("non-map value returns error", func(t *testing.T) {
1052+
err := p.Modify("not a map", func(v any) any { return v })
1053+
require.Error(t, err)
1054+
})
1055+
1056+
t.Run("typed-nil map returns error", func(t *testing.T) {
1057+
var doc map[string]any
1058+
err := p.Modify(doc, func(v any) any { return v })
1059+
require.Error(t, err)
1060+
})
1061+
}
1062+
10451063
// TestRemoveArrayIndex_Splices verifies that Remove on an index segment produces
10461064
// a correctly spliced slice (no nil gaps) — regression for issue #351.
10471065
func TestRemoveArrayIndex_Splices(t *testing.T) {

parser/schema_json.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,11 @@ func (s *Schema) UnmarshalJSON(data []byte) error {
122122
// map cannot be round-tripped through JSON into a *Schema, so callers get a
123123
// clear parse error rather than a silent type-assertion panic downstream.
124124
func promoteSchemaOrBool(v any) (any, error) {
125-
switch v.(type) {
125+
switch val := v.(type) {
126126
case nil, bool, *Schema:
127127
return v, nil
128128
case map[string]any:
129-
data, err := json.Marshal(v)
129+
data, err := json.Marshal(val)
130130
if err != nil {
131131
return nil, fmt.Errorf("parser: schema field promotion: %w", err)
132132
}
@@ -135,6 +135,19 @@ func promoteSchemaOrBool(v any) (any, error) {
135135
return nil, fmt.Errorf("parser: schema field promotion: %w", err)
136136
}
137137
return s, nil
138+
case []any:
139+
// OAS 2.0 tuple validation: items can be an array of schemas.
140+
schemas := make([]*Schema, 0, len(val))
141+
for _, elem := range val {
142+
promoted, err := promoteSchemaOrBool(elem)
143+
if err != nil {
144+
return nil, err
145+
}
146+
if s, ok := promoted.(*Schema); ok {
147+
schemas = append(schemas, s)
148+
}
149+
}
150+
return schemas, nil
138151
}
139152
return v, nil
140153
}

parser/schema_json_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,24 @@ func TestSchemaJSONRoundTrip(t *testing.T) {
295295
assert.True(t, ok, "Items should be *Schema after JSON unmarshal, not map[string]any")
296296
})
297297

298+
t.Run("Items bool true preserved (OAS 3.1+)", func(t *testing.T) {
299+
data := []byte(`{"type":"array","items":true}`)
300+
var s Schema
301+
require.NoError(t, json.Unmarshal(data, &s))
302+
b, ok := s.Items.(bool)
303+
assert.True(t, ok, "items true should remain bool")
304+
assert.True(t, b)
305+
})
306+
307+
t.Run("Items array decoded as []*Schema (OAS 2.0 tuple)", func(t *testing.T) {
308+
data := []byte(`{"type":"array","items":[{"type":"string"},{"type":"integer"}]}`)
309+
var s Schema
310+
require.NoError(t, json.Unmarshal(data, &s))
311+
arr, ok := s.Items.([]*Schema)
312+
assert.True(t, ok, "items array should be []*Schema after JSON unmarshal")
313+
assert.Len(t, arr, 2)
314+
})
315+
298316
t.Run("additionalProperties object decoded as *Schema", func(t *testing.T) {
299317
data := []byte(`{"type":"object","additionalProperties":{"type":"string"}}`)
300318
var s Schema

0 commit comments

Comments
 (0)