encoding/openapi: detect cycles when expanding references
Fixes #915
Change-Id: Ie781ee316e8675da66f7ca3bea4c841acaaa8a5b
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9603
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/encoding/openapi/build.go b/encoding/openapi/build.go
index b7a958d..f22a921 100644
--- a/encoding/openapi/build.go
+++ b/encoding/openapi/build.go
@@ -36,6 +36,7 @@
instExt *cue.Instance
refPrefix string
path []string
+ errs errors.Error
expandRefs bool
structural bool
@@ -49,6 +50,14 @@
// Track external schemas.
externalRefs map[string]*externalType
+
+ // Used for cycle detection in case of using ExpandReferences. At the
+ // moment, CUE does not detect cycles when a user forcefully steps into a
+ // pattern constraint.
+ //
+ // TODO: consider an option in the CUE API where optional fields are
+ // recursively evaluated.
+ cycleNodes []*adt.Vertex
}
type externalType struct {
@@ -168,7 +177,7 @@
return x < y
})
- return (*ast.StructLit)(c.schemas), nil
+ return (*ast.StructLit)(c.schemas), c.errs
}
func (c *buildContext) build(name string, v cue.Value) *ast.StructLit {
@@ -314,6 +323,9 @@
}
func (b *builder) value(v cue.Value, f typeFunc) (isRef bool) {
+ b.pushNode(v)
+ defer b.popNode()
+
count := 0
disallowDefault := false
var values cue.Value
@@ -770,7 +782,8 @@
b.setSingle("properties", (*ast.StructLit)(properties), false)
}
- if t, ok := v.Elem(); ok && (b.core == nil || b.core.items == nil) {
+ if t, ok := v.Elem(); ok &&
+ (b.core == nil || b.core.items == nil) && b.checkCycle(t) {
schema := b.schema(nil, "*", t)
if len(schema.Elts) > 0 {
b.setSingle("additionalProperties", schema, true) // Not allowed in structural.
@@ -871,7 +884,7 @@
}
if !hasMax || int64(len(items)) < maxLength {
- if typ, ok := v.Elem(); ok {
+ if typ, ok := v.Elem(); ok && b.checkCycle(typ) {
var core *builder
if b.core != nil {
core = b.core.items
diff --git a/encoding/openapi/crd.go b/encoding/openapi/crd.go
index 7251b43..dbf689f 100644
--- a/encoding/openapi/crd.go
+++ b/encoding/openapi/crd.go
@@ -109,6 +109,9 @@
// To this extent, all fields of both conjunctions and disjunctions are
// collected in a single properties map.
func (b *builder) buildCore(v cue.Value) {
+ b.pushNode(v)
+ defer b.popNode()
+
if !b.ctx.expandRefs {
_, r := v.Reference()
if len(r) > 0 {
@@ -126,6 +129,9 @@
switch b.kind {
case cue.StructKind:
if typ, ok := v.Elem(); ok {
+ if !b.checkCycle(typ) {
+ return
+ }
if b.items == nil {
b.items = newCoreBuilder(b.ctx)
}
@@ -135,6 +141,9 @@
case cue.ListKind:
if typ, ok := v.Elem(); ok {
+ if !b.checkCycle(typ) {
+ return
+ }
if b.items == nil {
b.items = newCoreBuilder(b.ctx)
}
diff --git a/encoding/openapi/cycle.go b/encoding/openapi/cycle.go
new file mode 100644
index 0000000..ef62294
--- /dev/null
+++ b/encoding/openapi/cycle.go
@@ -0,0 +1,59 @@
+// Copyright 2021 CUE Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package openapi
+
+import (
+ "cuelang.org/go/cue"
+ "cuelang.org/go/cue/errors"
+ "cuelang.org/go/cue/token"
+ "cuelang.org/go/internal/core/dep"
+ "cuelang.org/go/internal/core/eval"
+ internalvalue "cuelang.org/go/internal/value"
+)
+
+func (b *builder) pushNode(v cue.Value) {
+ _, n := internalvalue.ToInternal(v)
+ b.ctx.cycleNodes = append(b.ctx.cycleNodes, n)
+}
+
+func (b *builder) popNode() {
+ b.ctx.cycleNodes = b.ctx.cycleNodes[:len(b.ctx.cycleNodes)-1]
+}
+
+func (b *builder) checkCycle(v cue.Value) bool {
+ if !b.ctx.expandRefs {
+ return true
+ }
+ r, n := internalvalue.ToInternal(v)
+ ctx := eval.NewContext(r, n)
+
+ err := dep.Visit(ctx, n, func(d dep.Dependency) error {
+ for _, m := range b.ctx.cycleNodes {
+ if m == d.Node {
+ var p token.Pos
+ if src := d.Node.Source(); src != nil {
+ p = src.Pos()
+ }
+ err := errors.Newf(p,
+ "cycle in reference at %v: cyclic structures not allowed when reference expansion is requested", v.Path())
+ b.ctx.errs = errors.Append(b.ctx.errs, err)
+ return err
+ }
+ }
+ return nil
+ })
+
+ return err == nil
+}
diff --git a/encoding/openapi/openapi_test.go b/encoding/openapi/openapi_test.go
index 8e93a80..3f96175 100644
--- a/encoding/openapi/openapi_test.go
+++ b/encoding/openapi/openapi_test.go
@@ -43,70 +43,71 @@
testCases := []struct {
in, out string
config *openapi.Config
+ err string
}{{
- "structural.cue",
- "structural.json",
- resolveRefs,
+ in: "structural.cue",
+ out: "structural.json",
+ config: resolveRefs,
}, {
- "nested.cue",
- "nested.json",
- defaultConfig,
+ in: "nested.cue",
+ out: "nested.json",
+ config: defaultConfig,
}, {
- "simple.cue",
- "simple.json",
- resolveRefs,
+ in: "simple.cue",
+ out: "simple.json",
+ config: resolveRefs,
}, {
- "simple.cue",
- "simple-filter.json",
- &openapi.Config{Info: info, FieldFilter: "min.*|max.*"},
+ in: "simple.cue",
+ out: "simple-filter.json",
+ config: &openapi.Config{Info: info, FieldFilter: "min.*|max.*"},
}, {
- "array.cue",
- "array.json",
- defaultConfig,
+ in: "array.cue",
+ out: "array.json",
+ config: defaultConfig,
}, {
- "enum.cue",
- "enum.json",
- defaultConfig,
+ in: "enum.cue",
+ out: "enum.json",
+ config: defaultConfig,
}, {
- "struct.cue",
- "struct.json",
- defaultConfig,
+ in: "struct.cue",
+ out: "struct.json",
+ config: defaultConfig,
}, {
- "strings.cue",
- "strings.json",
- defaultConfig,
+ in: "strings.cue",
+ out: "strings.json",
+ config: defaultConfig,
}, {
- "nums.cue",
- "nums.json",
- defaultConfig,
+ in: "nums.cue",
+ out: "nums.json",
+ config: defaultConfig,
}, {
- "nums.cue",
- "nums-v3.1.0.json",
- &openapi.Config{Info: info, Version: "3.1.0"},
+ in: "nums.cue",
+ out: "nums-v3.1.0.json",
+ config: &openapi.Config{Info: info, Version: "3.1.0"},
}, {
- "builtins.cue",
- "builtins.json",
- defaultConfig,
+ in: "builtins.cue",
+ out: "builtins.json",
+ config: defaultConfig,
}, {
- "oneof.cue",
- "oneof.json",
- defaultConfig,
+ in: "oneof.cue",
+ out: "oneof.json",
+ config: defaultConfig,
}, {
- "oneof.cue",
- "oneof-resolve.json",
- resolveRefs,
+ in: "oneof.cue",
+ out: "oneof-resolve.json",
+ config: resolveRefs,
}, {
- "openapi.cue",
- "openapi.json",
- defaultConfig,
+ in: "openapi.cue",
+ out: "openapi.json",
+ config: defaultConfig,
}, {
- "openapi.cue",
- "openapi-norefs.json",
- resolveRefs,
+ in: "openapi.cue",
+ out: "openapi-norefs.json",
+ config: resolveRefs,
}, {
- "oneof.cue",
- "oneof-funcs.json",
- &openapi.Config{
+ in: "oneof.cue",
+ out: "oneof-funcs.json",
+ config: &openapi.Config{
Info: info,
ReferenceFunc: func(inst *cue.Instance, path []string) string {
return strings.ToUpper(strings.Join(path, "_"))
@@ -116,9 +117,9 @@
},
},
}, {
- "refs.cue",
- "refs.json",
- &openapi.Config{
+ in: "refs.cue",
+ out: "refs.json",
+ config: &openapi.Config{
Info: info,
ReferenceFunc: func(inst *cue.Instance, path []string) string {
switch {
@@ -129,9 +130,19 @@
},
},
}, {
- "issue131.cue",
- "issue131.json",
- &openapi.Config{Info: info, SelfContained: true},
+ in: "issue131.cue",
+ out: "issue131.json",
+ config: &openapi.Config{Info: info, SelfContained: true},
+ }, {
+ // Issue #915
+ in: "cycle.cue",
+ out: "cycle.json",
+ config: &openapi.Config{Info: info},
+ }, {
+ // Issue #915
+ in: "cycle.cue",
+ config: &openapi.Config{Info: info, ExpandReferences: true},
+ err: "cycle",
}}
for _, tc := range testCases {
t.Run(tc.out, func(t *testing.T) {
@@ -144,16 +155,24 @@
t.Fatal(errors.Details(inst.Err, nil))
}
- all, err := tc.config.All(inst)
- if err != nil {
- t.Fatal(err)
- }
- walk(all)
-
b, err := openapi.Gen(inst, tc.config)
if err != nil {
- t.Fatal(err)
+ if tc.err == "" {
+ t.Fatal("unexpected error:", errors.Details(inst.Err, nil))
+ }
+ return
}
+
+ if tc.err != "" {
+ t.Fatal("unexpected success:", tc.err)
+ } else {
+ all, err := tc.config.All(inst)
+ if err != nil {
+ t.Fatal(err)
+ }
+ walk(all)
+ }
+
var out = &bytes.Buffer{}
_ = json.Indent(out, b, "", " ")
@@ -206,7 +225,7 @@
ExpandReferences: true,
})
if err != nil {
- t.Fatal(err)
+ t.Fatal(errors.Details(err, nil))
}
var out = &bytes.Buffer{}
diff --git a/encoding/openapi/testdata/cycle.cue b/encoding/openapi/testdata/cycle.cue
new file mode 100644
index 0000000..eb5cd11
--- /dev/null
+++ b/encoding/openapi/testdata/cycle.cue
@@ -0,0 +1,2 @@
+ // Issue #915
+#Foo: [string]: #Foo
diff --git a/encoding/openapi/testdata/cycle.json b/encoding/openapi/testdata/cycle.json
new file mode 100644
index 0000000..49c86dc
--- /dev/null
+++ b/encoding/openapi/testdata/cycle.json
@@ -0,0 +1,19 @@
+{
+ "openapi": "3.0.0",
+ "info": {
+ "title": "test",
+ "version": "v1"
+ },
+ "paths": {},
+ "components": {
+ "schemas": {
+ "Foo": {
+ "description": "Issue #915",
+ "type": "object",
+ "additionalProperties": {
+ "$ref": "#/components/schemas/Foo"
+ }
+ }
+ }
+ }
+}
\ No newline at end of file
