Skip to content

Commit 5c63758

Browse files
authored
do not support ApplyDefaults on structs (#57)
We cannot reliably apply schema defaults to structs. We do not know if a struct field was missing in the JSON, or was present with its zero value. In the latter case, we could overwrite a legitimate value with a non-zero default.
1 parent 8b38616 commit 5c63758

File tree

2 files changed

+15
-153
lines changed

2 files changed

+15
-153
lines changed

jsonschema/validate.go

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -676,15 +676,16 @@ func (st *state) resolveDynamicRef(schema *Schema) (*Schema, error) {
676676
}
677677

678678
// ApplyDefaults modifies an instance by applying the schema's defaults to it. If
679-
// a schema or sub-schema has a default, then a corresponding zero instance value
679+
// a schema or sub-schema has a default, then a corresponding missing instance value
680680
// is set to the default.
681681
//
682682
// The JSON Schema specification does not describe how defaults should be interpreted.
683683
// This method honors defaults only on properties, and only those that are not required.
684684
// If the instance is a map and the property is missing, the property is added to
685685
// the map with the default.
686-
// If the instance is a struct, the field corresponding to the property exists, and
687-
// its value is zero, the field is set to the default.
686+
// ApplyDefaults does not support structs, because it cannot know whether a field
687+
// is missing in the JSON, or was explicitly set to its zero value.
688+
//
688689
// ApplyDefaults can panic if a default cannot be assigned to a field.
689690
//
690691
// The argument must be a pointer to the instance.
@@ -770,57 +771,7 @@ func (st *state) applyDefaults(instancep reflect.Value, schema *Schema) (err err
770771
}
771772
}
772773
case reflect.Struct:
773-
// If there is a default for this property, and the field exists but is zero,
774-
// set the field to the default.
775-
if subschema.Default != nil && val.IsValid() && val.IsZero() {
776-
if err := json.Unmarshal(subschema.Default, val.Addr().Interface()); err != nil {
777-
return err
778-
}
779-
// Recurse into newly set object to apply deeper defaults.
780-
if val.Kind() == reflect.Map {
781-
if val.IsNil() {
782-
val.Set(reflect.MakeMap(val.Type()))
783-
}
784-
if err := st.applyDefaults(val.Addr(), subschema); err != nil {
785-
return err
786-
}
787-
} else if val.Kind() == reflect.Struct {
788-
if err := st.applyDefaults(val.Addr(), subschema); err != nil {
789-
return err
790-
}
791-
}
792-
} else if val.IsValid() {
793-
// Recurse into existing sub-instance when object-like.
794-
switch val.Kind() {
795-
case reflect.Map:
796-
if val.IsNil() && schemaHasDefaultsInProperties(subschema) {
797-
val.Set(reflect.MakeMap(val.Type()))
798-
}
799-
if !val.IsNil() {
800-
if err := st.applyDefaults(val.Addr(), subschema); err != nil {
801-
return err
802-
}
803-
}
804-
case reflect.Struct:
805-
if err := st.applyDefaults(val.Addr(), subschema); err != nil {
806-
return err
807-
}
808-
case reflect.Pointer:
809-
et := val.Type().Elem()
810-
if (et.Kind() == reflect.Map || et.Kind() == reflect.Struct) && val.IsNil() && schemaHasDefaultsInProperties(subschema) {
811-
nv := reflect.New(et)
812-
if et.Kind() == reflect.Map {
813-
nv.Elem().Set(reflect.MakeMap(et))
814-
}
815-
val.Set(nv)
816-
}
817-
if !val.IsNil() && (et.Kind() == reflect.Map || et.Kind() == reflect.Struct) {
818-
if err := st.applyDefaults(val, subschema); err != nil {
819-
return err
820-
}
821-
}
822-
}
823-
}
774+
return errors.New("cannot apply defaults to a struct")
824775
default:
825776
panic(fmt.Sprintf("applyDefaults: property %s: bad value %s of kind %s",
826777
prop, instance, instance.Kind()))

jsonschema/validate_test.go

Lines changed: 10 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ func TestApplyDefaults(t *testing.T) {
160160
t.Fatal(err)
161161
}
162162

163-
type S struct{ A, B, C int }
164163
for _, tt := range []struct {
165164
instancep any // pointer to instance value
166165
want any // desired value (not a pointer)
@@ -173,14 +172,6 @@ func TestApplyDefaults(t *testing.T) {
173172
// "C" not added: it is required (Validate will catch that)
174173
},
175174
},
176-
{
177-
&S{B: 1},
178-
S{
179-
A: 1, // filled from default
180-
B: 1, // untouched: non-zero
181-
C: 0, // untouched: required
182-
},
183-
},
184175
} {
185176
if err := rs.ApplyDefaults(tt.instancep); err != nil {
186177
t.Fatal(err)
@@ -218,14 +209,6 @@ func TestApplyNestedDefaults(t *testing.T) {
218209
},
219210
}
220211

221-
type Nested struct{ B string }
222-
type Root struct{ A Nested }
223-
type RootPtr struct{ A *Nested }
224-
type RootMap struct{ A map[string]any }
225-
type RootPtrMap struct{ A *map[string]any }
226-
227-
mapPtr := func(m map[string]any) *map[string]any { return &m }
228-
229212
for _, tc := range []struct {
230213
name string
231214
schema *Schema
@@ -264,83 +247,6 @@ func TestApplyNestedDefaults(t *testing.T) {
264247
instancep: &map[string]map[string]any{},
265248
want: map[string]map[string]any{"A": {"B": "foo"}},
266249
},
267-
{
268-
name: "MapValueStructMissingParentTyped",
269-
schema: base,
270-
instancep: &map[string]Nested{},
271-
want: map[string]Nested{"A": {B: "foo"}},
272-
},
273-
{
274-
name: "StructZeroValueParent",
275-
schema: base,
276-
instancep: &Root{},
277-
want: Root{A: Nested{B: "foo"}},
278-
},
279-
{
280-
name: "StructZeroValueParentWithParentDefault",
281-
schema: withParentDefault,
282-
instancep: &Root{},
283-
want: Root{A: Nested{B: "foo"}},
284-
},
285-
{
286-
name: "StructPointerNilParent",
287-
schema: base,
288-
instancep: &RootPtr{},
289-
want: RootPtr{A: &Nested{B: "foo"}},
290-
},
291-
{
292-
name: "StructPresentNonzeroChildPreserved",
293-
schema: base,
294-
instancep: &Root{A: Nested{B: "bar"}},
295-
want: Root{A: Nested{B: "bar"}},
296-
},
297-
{
298-
name: "StructPointerNonNilChildPreserved",
299-
schema: base,
300-
instancep: &RootPtr{A: &Nested{B: "bar"}},
301-
want: RootPtr{A: &Nested{B: "bar"}},
302-
},
303-
{
304-
name: "StructMapNilParent",
305-
schema: base,
306-
instancep: &RootMap{},
307-
want: RootMap{A: map[string]any{"B": "foo"}},
308-
},
309-
{
310-
name: "StructMapParentDefaultObjectMissing",
311-
schema: withParentDefault,
312-
instancep: &RootMap{},
313-
want: RootMap{A: map[string]any{"X": float64(1), "B": "foo"}},
314-
},
315-
{
316-
name: "StructMapParentDefaultObjectPresent",
317-
schema: withParentDefault,
318-
instancep: &RootMap{A: map[string]any{}},
319-
want: RootMap{A: map[string]any{"B": "foo"}},
320-
},
321-
{
322-
name: "StructPtrMapNilParent",
323-
schema: base,
324-
instancep: &RootPtrMap{},
325-
want: RootPtrMap{A: mapPtr(map[string]any{"B": "foo"})},
326-
},
327-
{
328-
name: "StructMapNilParentWithNullParentDefault",
329-
schema: &Schema{
330-
Type: "object",
331-
Properties: map[string]*Schema{
332-
"A": {
333-
// Default null exercises map allocation in struct subschemas
334-
Default: mustMarshal(nil),
335-
Properties: map[string]*Schema{
336-
"B": {Type: "string", Default: mustMarshal("foo")},
337-
},
338-
},
339-
},
340-
},
341-
instancep: &RootMap{},
342-
want: RootMap{A: map[string]any{"B": "foo"}},
343-
},
344250
} {
345251
t.Run(tc.name, func(t *testing.T) {
346252
rs, err := tc.schema.Resolve(&ResolveOptions{ValidateDefaults: true})
@@ -522,7 +428,8 @@ func TestStructEmbedding(t *testing.T) {
522428
},
523429
Required: []string{"id", "name", "extra"},
524430
AdditionalProperties: falseSchema(),
525-
}},
431+
},
432+
},
526433
validInstance: []Banana{
527434
{Apple: &Apple{ID: "foo1", Name: "Test Foo 2"}, Extra: "additional data 1"},
528435
{Apple: &Apple{ID: "foo2", Name: "Test Foo 2"}, Extra: "additional data 2"},
@@ -544,7 +451,8 @@ func TestStructEmbedding(t *testing.T) {
544451
},
545452
Required: []string{"id", "name", "extra"},
546453
AdditionalProperties: falseSchema(),
547-
}},
454+
},
455+
},
548456
validInstance: [2]Banana{
549457
{Apple: &Apple{ID: "foo1", Name: "Test Foo 2"}, Extra: "additional data 1"},
550458
{Apple: &Apple{ID: "foo2", Name: "Test Foo 2"}, Extra: "additional data 2"},
@@ -564,7 +472,8 @@ func TestStructEmbedding(t *testing.T) {
564472
},
565473
Required: []string{"id", "name", "extra"},
566474
AdditionalProperties: falseSchema(),
567-
}},
475+
},
476+
},
568477
validInstance: []Durian{
569478
{cranberry: &cranberry{ID: "foo1", Name: "Test Foo 2"}, Extra: "additional data 1"},
570479
{cranberry: &cranberry{ID: "foo2", Name: "Test Foo 2"}, Extra: "additional data 2"},
@@ -584,7 +493,8 @@ func TestStructEmbedding(t *testing.T) {
584493
},
585494
Required: []string{"id", "name", "extra"},
586495
AdditionalProperties: falseSchema(),
587-
}},
496+
},
497+
},
588498
validInstance: []Fig{
589499
{Elderberry: Elderberry{ID: "foo1", Name: "Test Foo 2"}, Extra: "additional data 1"},
590500
{Elderberry: Elderberry{ID: "foo2", Name: "Test Foo 2"}, Extra: "additional data 2"},
@@ -604,7 +514,8 @@ func TestStructEmbedding(t *testing.T) {
604514
},
605515
Required: []string{"id", "name", "extra"},
606516
AdditionalProperties: falseSchema(),
607-
}},
517+
},
518+
},
608519
validInstance: []Honeyberry{
609520
{grape: grape{ID: "foo1", Name: "Test Foo 2"}, Extra: "additional data 1"},
610521
{grape: grape{ID: "foo2", Name: "Test Foo 2"}, Extra: "additional data 2"},

0 commit comments

Comments
 (0)