Skip to content

Commit

Permalink
encoding/jsonschema: fix a few json schema bugs
Browse files Browse the repository at this point in the history
Issue #378

- encoders should strip leading BOM. (Note that we do
not write the BOM back).

- fix bug in Validate that incorrectly required a struct
to be concrete when in non-concrete mode

- improve some error messages

- disable example handling (was a noop currently anyway)

- don't mark a type corresponding to a constraint as  used
if it is not actually used.

Change-Id: I687fb1e17b9d4c2ef2136f829690569cf77d6707
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/6104
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed May 19, 2020
1 parent fb7bdab commit f395f12
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 24 deletions.
2 changes: 1 addition & 1 deletion cmd/cue/cmd/testdata/script/def_jsonschema.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import "strings"
...
}
-- schema.json --
{
{
"$id": "https://example.com/person.schema.json",
"$schema": "http://json-schema.org/draft-07/schema#",
"title": "Person",
Expand Down
4 changes: 3 additions & 1 deletion cue/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,9 @@ func (x *validator) walk(v Value, opts options) {
x.depth++
obj, err := v.structValOpts(ctx, opts)
if err != nil {
x.errs = errors.Append(x.errs, v.toErr(err))
if !isIncomplete(err) && opts.concrete {
x.errs = errors.Append(x.errs, v.toErr(err))
}
}
for i := 0; i < obj.Len(); i++ {
_, v := obj.At(i)
Expand Down
13 changes: 13 additions & 0 deletions cue/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,19 @@ func TestValidate(t *testing.T) {
a: b: c: *["\(x)"] | _
d: yaml.Marshal(a.b)
`,
}, {
desc: "allow non-concrete values for definitions",
in: `
variables: #variables
{[!~"^[.]"]: #job}
#variables: [string]: int | string
#job: ({a: int} | {b: int}) & {
"variables"?: #variables
}
`,
}}
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion cue/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ func boolTonode(src source, b bool) evaluated {

type bytesLit struct {
baseValue
b []byte
b []byte
// Also support https://github.com/dlclark/regexp2 to
// accommodate JSON Schema?
re *regexp.Regexp // only set if needed
}

Expand Down
57 changes: 39 additions & 18 deletions encoding/jsonschema/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"cuelang.org/go/internal"
)

// TODO: skip invalid regexps containing ?! and foes.
// alternatively, fall back to https://github.com/dlclark/regexp2

type constraint struct {
key string

Expand Down Expand Up @@ -71,11 +74,11 @@ func init() {

func addDefinitions(n cue.Value, s *state) {
if n.Kind() != cue.StructKind {
s.errf(n, `"definitions" expected an object, found %v`, n.Kind)
s.errf(n, `"definitions" expected an object, found %s`, n.Kind())
}

if len(s.path) != 1 {
s.errf(n, `"definitions" expected an object, found %v`, n.Kind)
s.errf(n, `"definitions" only allowed at root`)
}

old := s.isSchema
Expand Down Expand Up @@ -191,11 +194,12 @@ var constraints = []*constraint{
if n.Kind() != cue.ListKind {
s.errf(n, `value of "examples" must be an array, found %v`, n.Kind)
}
for _, n := range s.listItems("examples", n, true) {
if ex := s.schema(n); !isAny(ex) {
s.examples = append(s.examples, ex)
}
}
// TODO: implement examples properly.
// for _, n := range s.listItems("examples", n, true) {
// if ex := s.value(n); !isAny(ex) {
// s.examples = append(s.examples, ex)
// }
// }
}),

p0("description", func(n cue.Value, s *state) {
Expand Down Expand Up @@ -340,11 +344,13 @@ var constraints = []*constraint{
}),

p0d("contentMediaType", 7, func(n cue.Value, s *state) {
s.usedTypes |= cue.StringKind
// TODO: only mark as used if it generates something.
// s.usedTypes |= cue.StringKind
}),

p0d("contentEncoding", 7, func(n cue.Value, s *state) {
s.usedTypes |= cue.StringKind
// TODO: only mark as used if it generates something.
// s.usedTypes |= cue.StringKind
// 7bit, 8bit, binary, quoted-printable and base64.
// RFC 2054, part 6.1.
// https://tools.ietf.org/html/rfc2045
Expand Down Expand Up @@ -424,12 +430,13 @@ var constraints = []*constraint{
}),

p1("required", func(n cue.Value, s *state) {
s.usedTypes |= cue.StructKind
if n.Kind() != cue.ListKind {
s.errf(n, `value of "required" must be list of strings, found %v`, n.Kind)
return
}

s.usedTypes |= cue.StructKind

if s.obj == nil {
s.obj = &ast.StructLit{}
// TODO: detect that properties is defined somewhere.
Expand All @@ -439,7 +446,10 @@ var constraints = []*constraint{
// Create field map
fields := map[string]*ast.Field{}
for _, d := range s.obj.Elts {
f := d.(*ast.Field)
f, ok := d.(*ast.Field)
if !ok {
continue // Could be embedding? See cirrus.json
}
str, _, err := ast.LabelName(f.Label)
if err == nil {
fields[str] = f
Expand All @@ -466,10 +476,9 @@ var constraints = []*constraint{
}),

p0d("propertyNames", 6, func(n cue.Value, s *state) {
s.usedTypes |= cue.StructKind

// [=~pattern]: _
if names, _ := s.schemaState(n, cue.StringKind, false); !isAny(names) {
s.usedTypes |= cue.StructKind
s.addConjunct(ast.NewStruct(ast.NewList((names)), ast.NewIdent("_")))
}
}),
Expand Down Expand Up @@ -529,12 +538,15 @@ var constraints = []*constraint{
}),

p2("additionalProperties", func(n cue.Value, s *state) {
s.usedTypes |= cue.StructKind
switch n.Kind() {
case cue.BoolKind:
s.closeStruct = !s.boolValue(n)
if !s.closeStruct {
s.usedTypes |= cue.StructKind
}

case cue.StructKind:
s.usedTypes |= cue.StructKind
s.closeStruct = true
if s.obj == nil {
s.obj = &ast.StructLit{}
Expand Down Expand Up @@ -586,10 +598,19 @@ var constraints = []*constraint{
}),

p0("additionalItems", func(n cue.Value, s *state) {
s.usedTypes |= cue.ListKind
if s.list != nil {
elem := s.schema(n)
s.list.Elts = append(s.list.Elts, &ast.Ellipsis{Type: elem})
switch n.Kind() {
case cue.BoolKind:
// TODO: support

case cue.StructKind:
if s.list != nil {
s.usedTypes |= cue.ListKind
elem := s.schema(n)
s.list.Elts = append(s.list.Elts, &ast.Ellipsis{Type: elem})
}

default:
s.errf(n, `value of "additionalItems" must be an object or boolean`)
}
}),

Expand Down
5 changes: 4 additions & 1 deletion encoding/jsonschema/testdata/basic.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
"type": "object",
"required": [ "name" ],
"properties": {
"name": { "type": "string" },
"name": {
"type": "string",
"examples": [ "foo" ]
},
"address": {
"description": "where does this person live?",
"type": "string",
Expand Down
14 changes: 12 additions & 2 deletions internal/encoding/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ import (
"cuelang.org/go/internal"
"cuelang.org/go/internal/filetypes"
"cuelang.org/go/internal/third_party/yaml"
"golang.org/x/text/encoding/unicode"
"golang.org/x/text/transform"
)

type Decoder struct {
Expand Down Expand Up @@ -169,13 +171,21 @@ func NewDecoder(f *build.File, cfg *Config) *Decoder {
return i
}

r, err := reader(f, cfg.Stdin)
i.closer = r
rc, err := reader(f, cfg.Stdin)
i.closer = rc
i.err = err
if err != nil {
return i
}

// For now we assume that all encodings require UTF-8. This will not be the
// case for some binary protocols. We need to exempt those explicitly here
// once we introduce them.
// TODO: this code also allows UTF16, which is too permissive for some
// encodings. Switch to unicode.UTF8Sig once available.
t := unicode.BOMOverride(unicode.UTF8.NewDecoder())
r := transform.NewReader(rc, t)

switch f.Interpretation {
case "":
case build.Auto:
Expand Down

0 comments on commit f395f12

Please sign in to comment.