From 40185dd013d82076b9fee1f74535c8694d3cf9f5 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Fri, 25 Apr 2025 15:20:37 -0400 Subject: [PATCH] jsonschema: represent integer keywords as ints Some JSON Schema keywords, like minLength, are logically integers. The JSON Schema spec allows them to be written as floating-point values with zero fractional part, which is a reasonable choice, since JSON doesn't distinguish the two. We could simply validate these fields as integers in the normal course of events (namely, validating the schema against its meta-schema), but this CL does a little better: it uses *int struct fields for these keywords, preventing them from ever being non-integers. The UnmarshalJSON method still accepts integral floating-point values. Marshaling is unaffected, and the results don't change: encoding/json has always marshaled integral floats without the decimal part. Other choices: - Leave them as float64: you could then get away with Schema{MinLength: Ptr(1.5)} and though you couldn't get far with that, it's still irksome. - json.Number: same problem as float64, plus it's even harder to write as a literal. - int64: now `Ptr(1)` doesn't work (it's a *int, not a *int64). Anyway, these keywords all compare against len(x) in Go, which is of type int. Change-Id: Ie3f08bd8a5deaf2a21344d6ab873551c011b12d5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/668315 LUCI-TryBot-Result: Go LUCI Reviewed-by: Robert Findley --- internal/jsonschema/infer.go | 4 +- internal/jsonschema/schema.go | 61 ++++++++++++++++++++----- internal/jsonschema/schema_test.go | 72 +++++++++++++++++++++++++++++- internal/jsonschema/validate.go | 20 ++++----- 4 files changed, 132 insertions(+), 25 deletions(-) diff --git a/internal/jsonschema/infer.go b/internal/jsonschema/infer.go index 8d96de4..9f1ed5e 100644 --- a/internal/jsonschema/infer.go +++ b/internal/jsonschema/infer.go @@ -92,8 +92,8 @@ func typeSchema(t reflect.Type, seen map[reflect.Type]*Schema) (*Schema, error) return nil, fmt.Errorf("computing element schema: %v", err) } if t.Kind() == reflect.Array { - s.MinItems = Ptr(float64(t.Len())) - s.MaxItems = Ptr(float64(t.Len())) + s.MinItems = Ptr(t.Len()) + s.MaxItems = Ptr(t.Len()) } case reflect.String: diff --git a/internal/jsonschema/schema.go b/internal/jsonschema/schema.go index fdc8baf..0fdac69 100644 --- a/internal/jsonschema/schema.go +++ b/internal/jsonschema/schema.go @@ -8,6 +8,7 @@ package jsonschema import ( "bytes" + "cmp" "encoding/json" "errors" "fmt" @@ -67,25 +68,25 @@ type Schema struct { Maximum *float64 `json:"maximum,omitempty"` ExclusiveMinimum *float64 `json:"exclusiveMinimum,omitempty"` ExclusiveMaximum *float64 `json:"exclusiveMaximum,omitempty"` - MinLength *float64 `json:"minLength,omitempty"` - MaxLength *float64 `json:"maxLength,omitempty"` + MinLength *int `json:"minLength,omitempty"` + MaxLength *int `json:"maxLength,omitempty"` Pattern string `json:"pattern,omitempty"` // arrays PrefixItems []*Schema `json:"prefixItems,omitempty"` Items *Schema `json:"items,omitempty"` - MinItems *float64 `json:"minItems,omitempty"` - MaxItems *float64 `json:"maxItems,omitempty"` + MinItems *int `json:"minItems,omitempty"` + MaxItems *int `json:"maxItems,omitempty"` AdditionalItems *Schema `json:"additionalItems,omitempty"` UniqueItems bool `json:"uniqueItems,omitempty"` Contains *Schema `json:"contains,omitempty"` - MinContains *float64 `json:"minContains,omitempty"` - MaxContains *float64 `json:"maxContains,omitempty"` + MinContains *int `json:"minContains,omitempty"` // *int, not int: default is 1, not 0 + MaxContains *int `json:"maxContains,omitempty"` UnevaluatedItems *Schema `json:"unevaluatedItems,omitempty"` // objects - MinProperties *float64 `json:"minProperties,omitempty"` - MaxProperties *float64 `json:"maxProperties,omitempty"` + MinProperties *int `json:"minProperties,omitempty"` + MaxProperties *int `json:"maxProperties,omitempty"` Required []string `json:"required,omitempty"` DependentRequired map[string][]string `json:"dependentRequired,omitempty"` Properties map[string]*Schema `json:"properties,omitempty"` @@ -174,8 +175,17 @@ func (s *Schema) UnmarshalJSON(data []byte) error { } ms := struct { - Type json.RawMessage `json:"type,omitempty"` - Const json.RawMessage `json:"const,omitempty"` + Type json.RawMessage `json:"type,omitempty"` + Const json.RawMessage `json:"const,omitempty"` + MinLength *float64 `json:"minLength,omitempty"` + MaxLength *float64 `json:"maxLength,omitempty"` + MinItems *float64 `json:"minItems,omitempty"` + MaxItems *float64 `json:"maxItems,omitempty"` + MinProperties *float64 `json:"minProperties,omitempty"` + MaxProperties *float64 `json:"maxProperties,omitempty"` + MinContains *float64 `json:"minContains,omitempty"` + MaxContains *float64 `json:"maxContains,omitempty"` + *schemaWithoutMethods }{ schemaWithoutMethods: (*schemaWithoutMethods)(s), @@ -192,12 +202,13 @@ func (s *Schema) UnmarshalJSON(data []byte) error { case '[': err = json.Unmarshal(ms.Type, &s.Types) default: - err = fmt.Errorf("invalid type: %q", ms.Type) + err = fmt.Errorf(`invalid value for "type": %q`, ms.Type) } } if err != nil { return err } + // Setting Const to a pointer to null will marshal properly, but won't unmarshal: // the *any is set to nil, not a pointer to nil. if len(ms.Const) > 0 { @@ -207,6 +218,34 @@ func (s *Schema) UnmarshalJSON(data []byte) error { return err } } + + // Store integer properties as ints. + setInt := func(name string, dst **int, src *float64) error { + if src == nil { + return nil + } + i := int(*src) + if float64(i) != *src { + return fmt.Errorf("%s: %f is not an int", name, *src) + } + *dst = &i + return nil + } + + err = cmp.Or( + setInt("minLength", &s.MinLength, ms.MinLength), + setInt("maxLength", &s.MaxLength, ms.MaxLength), + setInt("minItems", &s.MinItems, ms.MinItems), + setInt("maxItems", &s.MaxItems, ms.MaxItems), + setInt("minProperties", &s.MinProperties, ms.MinProperties), + setInt("maxProperties", &s.MaxProperties, ms.MaxProperties), + setInt("minContains", &s.MinContains, ms.MinContains), + setInt("maxContains", &s.MaxContains, ms.MaxContains), + ) + if err != nil { + return err + } + return nil } diff --git a/internal/jsonschema/schema_test.go b/internal/jsonschema/schema_test.go index 2374aa0..0f475c9 100644 --- a/internal/jsonschema/schema_test.go +++ b/internal/jsonschema/schema_test.go @@ -6,14 +6,16 @@ package jsonschema import ( "encoding/json" + "regexp" "testing" ) -func TestMarshal(t *testing.T) { +func TestGoRoundTrip(t *testing.T) { + // Verify that Go representations round-trip. for _, s := range []*Schema{ {Type: "null"}, {Types: []string{"null", "number"}}, - {Type: "string", MinLength: Ptr(20.0)}, + {Type: "string", MinLength: Ptr(20)}, {Minimum: Ptr(20.0)}, {Items: &Schema{Type: "integer"}}, {Const: Ptr(any(0))}, @@ -38,3 +40,69 @@ func TestMarshal(t *testing.T) { } } } + +func TestJSONRoundTrip(t *testing.T) { + // Verify that JSON texts for schemas marshal into equivalent forms. + // We don't expect everything to round-trip perfectly. For example, "true" and "false" + // will turn into their object equivalents. + // But most things should. + // Some of these cases test Schema.{UnM,M}arshalJSON. + // Most of others follow from the behavior of encoding/json, but they are still + // valuable as regression tests of this package's behavior. + for _, tt := range []struct { + in, want string + }{ + {`true`, `{}`}, // boolean schemas become object schemas + {`false`, `{"not":{}}`}, + {`{"type":"", "enum":null}`, `{}`}, // empty fields are omitted + {`{"minimum":1}`, `{"minimum":1}`}, + {`{"minimum":1.0}`, `{"minimum":1}`}, // floating-point integers lose their fractional part + {`{"minLength":1.0}`, `{"minLength":1}`}, // some floats are unmarshaled into ints, but you can't tell + { + // map keys are sorted + `{"$vocabulary":{"b":true, "a":false}}`, + `{"$vocabulary":{"a":false,"b":true}}`, + }, + {`{"unk":0}`, `{}`}, // unknown fields are dropped, unfortunately + } { + var s Schema + if err := json.Unmarshal([]byte(tt.in), &s); err != nil { + t.Fatal(err) + } + data, err := json.Marshal(s) + if err != nil { + t.Fatal(err) + } + if got := string(data); got != tt.want { + t.Errorf("%s:\ngot %s\nwant %s", tt.in, got, tt.want) + } + } +} + +func TestUnmarshalErrors(t *testing.T) { + for _, tt := range []struct { + in string + want string // error must match this regexp + }{ + {`1`, "cannot unmarshal number"}, + {`{"type":1}`, `invalid value for "type"`}, + {`{"minLength":1.5}`, `minLength:.*not an int`}, + {`{"maxLength":1.5}`, `maxLength:.*not an int`}, + {`{"minItems":1.5}`, `minItems:.*not an int`}, + {`{"maxItems":1.5}`, `maxItems:.*not an int`}, + {`{"minProperties":1.5}`, `minProperties:.*not an int`}, + {`{"maxProperties":1.5}`, `maxProperties:.*not an int`}, + {`{"minContains":1.5}`, `minContains:.*not an int`}, + {`{"maxContains":1.5}`, `maxContains:.*not an int`}, + } { + var s Schema + err := json.Unmarshal([]byte(tt.in), &s) + if err == nil { + t.Fatalf("%s: no error but expected one", tt.in) + } + if !regexp.MustCompile(tt.want).MatchString(err.Error()) { + t.Errorf("%s: error %q does not match %q", tt.in, err, tt.want) + } + + } +} diff --git a/internal/jsonschema/validate.go b/internal/jsonschema/validate.go index 0149bd0..1d23564 100644 --- a/internal/jsonschema/validate.go +++ b/internal/jsonschema/validate.go @@ -145,12 +145,12 @@ func (st *state) validate(instance reflect.Value, schema *Schema, callerAnns *an str := instance.String() n := utf8.RuneCountInString(str) if schema.MinLength != nil { - if m := int(*schema.MinLength); n < m { + if m := *schema.MinLength; n < m { return fmt.Errorf("minLength: %q contains %d Unicode code points, fewer than %d", str, n, m) } } if schema.MaxLength != nil { - if m := int(*schema.MaxLength); n > m { + if m := *schema.MaxLength; n > m { return fmt.Errorf("maxLength: %q contains %d Unicode code points, more than %d", str, n, m) } } @@ -268,7 +268,7 @@ func (st *state) validate(instance reflect.Value, schema *Schema, callerAnns *an anns.noteIndex(i) } } - if nContains == 0 && (schema.MinContains == nil || int(*schema.MinContains) > 0) { + if nContains == 0 && (schema.MinContains == nil || *schema.MinContains > 0) { return fmt.Errorf("contains: %s does not have an item matching %s", instance, schema.Contains) } @@ -277,23 +277,23 @@ func (st *state) validate(instance reflect.Value, schema *Schema, callerAnns *an // https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-01#section-6.4 // TODO(jba): check that these next four keywords' values are integers. if schema.MinContains != nil && schema.Contains != nil { - if m := int(*schema.MinContains); nContains < m { + if m := *schema.MinContains; nContains < m { return fmt.Errorf("minContains: contains validated %d items, less than %d", nContains, m) } } if schema.MaxContains != nil && schema.Contains != nil { - if m := int(*schema.MaxContains); nContains > m { + if m := *schema.MaxContains; nContains > m { return fmt.Errorf("maxContains: contains validated %d items, greater than %d", nContains, m) } } if schema.MinItems != nil { - if m := int(*schema.MinItems); instance.Len() < m { + if m := *schema.MinItems; instance.Len() < m { return fmt.Errorf("minItems: array length %d is less than %d", instance.Len(), m) } } if schema.MaxItems != nil { - if m := int(*schema.MaxItems); instance.Len() > m { - return fmt.Errorf("minItems: array length %d is greater than %d", instance.Len(), m) + if m := *schema.MaxItems; instance.Len() > m { + return fmt.Errorf("maxItems: array length %d is greater than %d", instance.Len(), m) } } if schema.UniqueItems { @@ -385,12 +385,12 @@ func (st *state) validate(instance reflect.Value, schema *Schema, callerAnns *an // https://json-schema.org/draft/2020-12/draft-bhutton-json-schema-validation-01#section-6.5 if schema.MinProperties != nil { - if n, m := instance.Len(), int(*schema.MinProperties); n < m { + if n, m := instance.Len(), *schema.MinProperties; n < m { return fmt.Errorf("minProperties: object has %d properties, less than %d", n, m) } } if schema.MaxProperties != nil { - if n, m := instance.Len(), int(*schema.MaxProperties); n > m { + if n, m := instance.Len(), *schema.MaxProperties; n > m { return fmt.Errorf("maxProperties: object has %d properties, greater than %d", n, m) } }