Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
encoding/openapi: detect cycles when expanding references
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
mpvl committed May 20, 2021
1 parent cd94426 commit a4e0f52
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 64 deletions.
19 changes: 16 additions & 3 deletions encoding/openapi/build.go
Expand Up @@ -36,6 +36,7 @@ type buildContext struct {
instExt *cue.Instance
refPrefix string
path []string
errs errors.Error

expandRefs bool
structural bool
Expand All @@ -49,6 +50,14 @@ type buildContext struct {

// 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 {
Expand Down Expand Up @@ -168,7 +177,7 @@ func schemas(g *Generator, inst *cue.Instance) (schemas *ast.StructLit, err erro
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 {
Expand Down Expand Up @@ -314,6 +323,9 @@ var fieldOrder = map[string]int{
}

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
Expand Down Expand Up @@ -770,7 +782,8 @@ func (b *builder) object(v cue.Value) {
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.
Expand Down Expand Up @@ -871,7 +884,7 @@ func (b *builder) array(v cue.Value) {
}

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
Expand Down
9 changes: 9 additions & 0 deletions encoding/openapi/crd.go
Expand Up @@ -109,6 +109,9 @@ func (b *builder) coreSchema() *ast.StructLit {
// 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 {
Expand All @@ -126,6 +129,9 @@ func (b *builder) buildCore(v cue.Value) {
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)
}
Expand All @@ -135,6 +141,9 @@ func (b *builder) buildCore(v cue.Value) {

case cue.ListKind:
if typ, ok := v.Elem(); ok {
if !b.checkCycle(typ) {
return
}
if b.items == nil {
b.items = newCoreBuilder(b.ctx)
}
Expand Down
59 changes: 59 additions & 0 deletions 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
}
141 changes: 80 additions & 61 deletions encoding/openapi/openapi_test.go
Expand Up @@ -43,70 +43,71 @@ func TestParseDefinitions(t *testing.T) {
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, "_"))
Expand All @@ -116,9 +117,9 @@ func TestParseDefinitions(t *testing.T) {
},
},
}, {
"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 {
Expand All @@ -129,9 +130,19 @@ func TestParseDefinitions(t *testing.T) {
},
},
}, {
"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) {
Expand All @@ -144,16 +155,24 @@ func TestParseDefinitions(t *testing.T) {
t.Fatal(errors.Details(inst.Err, nil))
}

all, err := tc.config.All(inst)
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
}
walk(all)

b, err := openapi.Gen(inst, tc.config)
if err != nil {
t.Fatal(err)
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, "", " ")

Expand Down Expand Up @@ -206,7 +225,7 @@ func TestX(t *testing.T) {
ExpandReferences: true,
})
if err != nil {
t.Fatal(err)
t.Fatal(errors.Details(err, nil))
}

var out = &bytes.Buffer{}
Expand Down
2 changes: 2 additions & 0 deletions encoding/openapi/testdata/cycle.cue
@@ -0,0 +1,2 @@
// Issue #915
#Foo: [string]: #Foo
19 changes: 19 additions & 0 deletions 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"
}
}
}
}
}

0 comments on commit a4e0f52

Please sign in to comment.