Repository: ogen-go/ogen
Pull request #306 (2022-04-22 20:35:40)

fix(gen): check schema for some cases which lead to stack overflow


Patch:
diff --git a/examples/ex_openapi/output.gen.go b/examples/ex_openapi/output.gen.go
index 452dc427..26d2d37d 100644
--- a/examples/ex_openapi/output.gen.go
+++ b/examples/ex_openapi/output.gen.go
@@ -14821,6 +14821,36 @@ func (s *SchemaOrReference) Decode(d *jx.Decoder) error {
 			case "type":
 				found = true
 				s.Type = SchemaSchemaOrReference
+			case "allOf":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "oneOf":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "anyOf":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "not":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "items":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "properties":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "additionalProperties":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "default":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "description":
+				found = true
+				s.Type = SchemaSchemaOrReference
+			case "format":
+				found = true
+				s.Type = SchemaSchemaOrReference
 			case "$ref":
 				found = true
 				s.Type = ReferenceSchemaOrReference
diff --git a/gen/schema_gen.go b/gen/schema_gen.go
index 6d611771..f3cc61a3 100644
--- a/gen/schema_gen.go
+++ b/gen/schema_gen.go
@@ -88,19 +88,27 @@ func (g *schemaGen) generate(name string, schema *jsonschema.Schema) (_ *ir.Type
 
 	switch {
 	case len(schema.AnyOf) > 0:
-		t, err := g.anyOf(name, schema)
-		if err != nil {
+		t := side(&ir.Type{
+			Name:   name,
+			Kind:   ir.KindSum,
+			Schema: schema,
+		})
+		if err := g.anyOf(t, name, schema); err != nil {
 			return nil, errors.Wrap(err, "anyOf")
 		}
-		return side(t), nil
+		return t, nil
 	case len(schema.AllOf) > 0:
 		return nil, &ErrNotImplemented{"allOf"}
 	case len(schema.OneOf) > 0:
-		t, err := g.oneOf(name, schema)
-		if err != nil {
+		t := side(&ir.Type{
+			Name:   name,
+			Kind:   ir.KindSum,
+			Schema: schema,
+		})
+		if err := g.oneOf(t, name, schema); err != nil {
 			return nil, errors.Wrap(err, "oneOf")
 		}
-		return side(t), nil
+		return t, nil
 	}
 
 	switch schema.Type {
diff --git a/gen/schema_gen_sum.go b/gen/schema_gen_sum.go
index 4f7f050b..fe8f3a98 100644
--- a/gen/schema_gen_sum.go
+++ b/gen/schema_gen_sum.go
@@ -56,35 +56,40 @@ func canUseTypeDiscriminator(sum []*ir.Type) bool {
 	return true
 }
 
-func (g *schemaGen) collectSumVariants(name string, schemas []*jsonschema.Schema) (sum []*ir.Type, _ error) {
+func (g *schemaGen) collectSumVariants(
+	name string,
+	parent *jsonschema.Schema,
+	schemas []*jsonschema.Schema,
+) (sum []*ir.Type, _ error) {
 	names := map[string]struct{}{}
 	for i, s := range schemas {
+		if ref := s.Ref; ref != "" && ref == parent.Ref {
+			return nil, errors.Errorf("reference %q [%d] leads to infinite recursion", ref, i)
+		}
+
 		t, err := g.generate(fmt.Sprintf("%s%d", name, i), s)
 		if err != nil {
 			return nil, errors.Wrapf(err, "oneOf[%d]", i)
 		}
+
 		t.Name = variantFieldName(t)
 		if _, ok := names[t.Name]; ok {
 			return nil, errors.Wrap(&ErrNotImplemented{
 				Name: "sum types with same names",
 			}, name)
 		}
+
 		names[t.Name] = struct{}{}
 		sum = append(sum, t)
 	}
 	return sum, nil
 }
 
-func (g *schemaGen) anyOf(name string, schema *jsonschema.Schema) (*ir.Type, error) {
-	sum := &ir.Type{
-		Name:   name,
-		Kind:   ir.KindSum,
-		Schema: schema,
-	}
+func (g *schemaGen) anyOf(sum *ir.Type, name string, schema *jsonschema.Schema) error {
 	{
-		variants, err := g.collectSumVariants(name, schema.AnyOf)
+		variants, err := g.collectSumVariants(name, schema, schema.AnyOf)
 		if err != nil {
-			return nil, errors.Wrap(err, "collect variants")
+			return errors.Wrap(err, "collect variants")
 		}
 		sum.SumOf = variants
 	}
@@ -99,18 +104,18 @@ func (g *schemaGen) anyOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 				case v.IsInteger():
 					if !v.Validators.Int.Set() {
 						if err := v.Validators.SetInt(schema); err != nil {
-							return nil, errors.Wrap(err, "int validator")
+							return errors.Wrap(err, "int validator")
 						}
 					}
 				case v.IsFloat():
 					if !v.Validators.Float.Set() {
 						if err := v.Validators.SetFloat(schema); err != nil {
-							return nil, errors.Wrap(err, "float validator")
+							return errors.Wrap(err, "float validator")
 						}
 					}
 				case !v.Validators.String.Set():
 					if err := v.Validators.SetString(schema); err != nil {
-						return nil, errors.Wrap(err, "string validator")
+						return errors.Wrap(err, "string validator")
 					}
 				}
 			case ir.KindArray:
@@ -123,21 +128,16 @@ func (g *schemaGen) anyOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 				}
 			}
 		}
-		return sum, nil
+		return nil
 	}
-	return nil, &ErrNotImplemented{"complex anyOf"}
+	return &ErrNotImplemented{"complex anyOf"}
 }
 
-func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, error) {
-	sum := &ir.Type{
-		Name:   name,
-		Kind:   ir.KindSum,
-		Schema: schema,
-	}
+func (g *schemaGen) oneOf(sum *ir.Type, name string, schema *jsonschema.Schema) error {
 	{
-		variants, err := g.collectSumVariants(name, schema.OneOf)
+		variants, err := g.collectSumVariants(name, schema, schema.OneOf)
 		if err != nil {
-			return nil, errors.Wrap(err, "collect variants")
+			return errors.Wrap(err, "collect variants")
 		}
 		sum.SumOf = variants
 	}
@@ -151,7 +151,7 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 			var found bool
 			for i, s := range sum.SumOf {
 				if !s.Is(ir.KindStruct, ir.KindMap) {
-					return nil, errors.Wrapf(&ErrNotImplemented{"unsupported sum type variant"}, "%T", s.Kind)
+					return errors.Wrapf(&ErrNotImplemented{"unsupported sum type variant"}, "%T", s.Kind)
 				}
 				var ref string
 				if s.Schema != nil {
@@ -179,7 +179,7 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 				}
 			}
 			if !found {
-				return nil, errors.Errorf("discriminator: unable to map %q to %q", k, v)
+				return errors.Errorf("discriminator: unable to map %q to %q", k, v)
 			}
 		}
 		if len(sum.SumSpec.Mapping) == 0 {
@@ -203,13 +203,13 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 			b := sum.SumSpec.Mapping[j]
 			return strings.Compare(a.Key, b.Key) < 0
 		})
-		return sum, nil
+		return nil
 	}
 
 	// 2nd case: distinguish by serialization type.
 	if canUseTypeDiscriminator(sum.SumOf) {
 		sum.SumSpec.TypeDiscriminator = true
-		return sum, nil
+		return nil
 	}
 
 	// 3rd case: distinguish by unique fields.
@@ -220,7 +220,7 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 	for _, s := range sum.SumOf {
 		uniq[s.Name] = map[string]struct{}{}
 		if !s.Is(ir.KindStruct) {
-			return nil, errors.Wrapf(&ErrNotImplemented{Name: "discriminator inference"},
+			return errors.Wrapf(&ErrNotImplemented{Name: "discriminator inference"},
 				"oneOf %s: variant %s: no unique fields, "+
 					"unable to parse without discriminator", sum.Name, s.Name,
 			)
@@ -263,7 +263,7 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 			if len(uniq[k]) == 0 {
 				if metNoUniqueFields {
 					// Unable to deterministically select sub-schema only on fields.
-					return nil, errors.Wrapf(&ErrNotImplemented{Name: "discriminator inference"},
+					return errors.Wrapf(&ErrNotImplemented{Name: "discriminator inference"},
 						"oneOf %s: variant %s: no unique fields, "+
 							"unable to parse without discriminator", sum.Name, k,
 					)
@@ -321,5 +321,5 @@ func (g *schemaGen) oneOf(name string, schema *jsonschema.Schema) (*ir.Type, err
 			}
 		}
 	}
-	return sum, nil
+	return nil
 }
diff --git a/tools/sgcollector/main.go b/tools/sgcollector/main.go
index e3e7807a..cc328ff6 100644
--- a/tools/sgcollector/main.go
+++ b/tools/sgcollector/main.go
@@ -63,6 +63,7 @@ func run(ctx context.Context) error {
 		clean          = flag.Bool("clean", false, "Clean generated files before generation")
 		generateYaml   = flag.Bool("yaml", false, "Query yaml files")
 		q              = flag.String("query", "", "Sourcegraph query")
+		workers        = flag.Int("workers", runtime.GOMAXPROCS(-1), "Number of generator workers to spawn")
 		cpuProfile     = flag.String("cpuprofile", "", "Write cpu profile to file")
 		memProfile     = flag.String("memprofile", "", "Write memory profile to file")
 		memProfileRate = flag.Int64("memprofilerate", 0, "Set runtime.MemProfileRate")
@@ -119,12 +120,11 @@ func run(ctx context.Context) error {
 	}
 
 	var (
-		workers   = 1
-		links     = make(chan FileMatch, workers)
+		links     = make(chan FileMatch, *workers)
 		reporters = &Reporters{}
 		total     int
 	)
-	reporters.init(workers)
+	reporters.init(*workers)
 
 	g, ctx := errgroup.WithContext(ctx)
 	g.Go(func() error {
@@ -157,7 +157,7 @@ func run(ctx context.Context) error {
 	})
 
 	var workersWg sync.WaitGroup
-	for i := 0; i < workers; i++ {
+	for i := 0; i < *workers; i++ {
 		workersWg.Add(1)
 		g.Go(func() error {
 			defer workersWg.Done()
@@ -171,7 +171,6 @@ func run(ctx context.Context) error {
 					}
 
 					link := m.Link()
-					fmt.Printf("Checking %q\n", link)
 					if err := worker(ctx, m, reporters); err != nil {
 						fmt.Printf("Check %q: %v\n", link, err)
 					}


Test patch:
diff --git a/_testdata/negative/cyclic_allOf.json b/_testdata/negative/cyclic_allOf.json
new file mode 100644
index 00000000..7f05772f
--- /dev/null
+++ b/_testdata/negative/cyclic_allOf.json
@@ -0,0 +1,76 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "title",
+    "version": "v0.1.0"
+  },
+  "paths": {
+    "/healthz": {
+      "get": {
+        "operationId": "probeLiveness",
+        "description": "Liveness probe for kubernetes",
+        "responses": {
+          "200": {
+            "description": "User info",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Pet"
+                }
+              }
+            }
+          },
+          "default": {
+            "$ref": "#/components/responses/Error"
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Pet": {
+        "allOf": [
+          {
+            "$ref": "#/components/schemas/Pet"
+          },
+          {
+            "$ref": "#/components/schemas/Cat"
+          }
+        ]
+      },
+      "Cat": {
+        "type": "object"
+      },
+      "Error": {
+        "description": "Represents error object",
+        "type": "object",
+        "properties": {
+          "code": {
+            "type": "integer",
+            "format": "int64"
+          },
+          "message": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "code",
+          "message"
+        ]
+      }
+    },
+    "responses": {
+      "Error": {
+        "description": "General Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/Error"
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/_testdata/negative/cyclic_anyOf.json b/_testdata/negative/cyclic_anyOf.json
new file mode 100644
index 00000000..fba5a696
--- /dev/null
+++ b/_testdata/negative/cyclic_anyOf.json
@@ -0,0 +1,76 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "title",
+    "version": "v0.1.0"
+  },
+  "paths": {
+    "/healthz": {
+      "get": {
+        "operationId": "probeLiveness",
+        "description": "Liveness probe for kubernetes",
+        "responses": {
+          "200": {
+            "description": "User info",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Pet"
+                }
+              }
+            }
+          },
+          "default": {
+            "$ref": "#/components/responses/Error"
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Pet": {
+        "anyOf": [
+          {
+            "$ref": "#/components/schemas/Pet"
+          },
+          {
+            "$ref": "#/components/schemas/Cat"
+          }
+        ]
+      },
+      "Cat": {
+        "type": "object"
+      },
+      "Error": {
+        "description": "Represents error object",
+        "type": "object",
+        "properties": {
+          "code": {
+            "type": "integer",
+            "format": "int64"
+          },
+          "message": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "code",
+          "message"
+        ]
+      }
+    },
+    "responses": {
+      "Error": {
+        "description": "General Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/Error"
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/_testdata/negative/cyclic_oneOf.json b/_testdata/negative/cyclic_oneOf.json
new file mode 100644
index 00000000..bf048dcb
--- /dev/null
+++ b/_testdata/negative/cyclic_oneOf.json
@@ -0,0 +1,76 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "title",
+    "version": "v0.1.0"
+  },
+  "paths": {
+    "/healthz": {
+      "get": {
+        "operationId": "probeLiveness",
+        "description": "Liveness probe for kubernetes",
+        "responses": {
+          "200": {
+            "description": "User info",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Pet"
+                }
+              }
+            }
+          },
+          "default": {
+            "$ref": "#/components/responses/Error"
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Pet": {
+        "oneOf": [
+          {
+            "$ref": "#/components/schemas/Pet"
+          },
+          {
+            "$ref": "#/components/schemas/Cat"
+          }
+        ]
+      },
+      "Cat": {
+        "type": "object"
+      },
+      "Error": {
+        "description": "Represents error object",
+        "type": "object",
+        "properties": {
+          "code": {
+            "type": "integer",
+            "format": "int64"
+          },
+          "message": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "code",
+          "message"
+        ]
+      }
+    },
+    "responses": {
+      "Error": {
+        "description": "General Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/Error"
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/_testdata/negative/ref_to_cyclic_oneOf.json b/_testdata/negative/ref_to_cyclic_oneOf.json
new file mode 100644
index 00000000..9f7e8dad
--- /dev/null
+++ b/_testdata/negative/ref_to_cyclic_oneOf.json
@@ -0,0 +1,82 @@
+{
+  "openapi": "3.0.3",
+  "info": {
+    "title": "title",
+    "version": "v0.1.0"
+  },
+  "paths": {
+    "/healthz": {
+      "get": {
+        "operationId": "probeLiveness",
+        "description": "Liveness probe for kubernetes",
+        "responses": {
+          "200": {
+            "description": "User info",
+            "content": {
+              "application/json": {
+                "schema": {
+                  "$ref": "#/components/schemas/Foo"
+                }
+              }
+            }
+          },
+          "default": {
+            "$ref": "#/components/responses/Error"
+          }
+        }
+      }
+    }
+  },
+  "components": {
+    "schemas": {
+      "Pet": {
+        "oneOf": [
+          {
+            "$ref": "#/components/schemas/Bar"
+          },
+          {
+            "$ref": "#/components/schemas/Cat"
+          }
+        ]
+      },
+      "Foo": {
+        "$ref": "#/components/schemas/Pet"
+      },
+      "Bar": {
+        "$ref": "#/components/schemas/Foo"
+      },
+      "Cat": {
+        "type": "object"
+      },
+      "Error": {
+        "description": "Represents error object",
+        "type": "object",
+        "properties": {
+          "code": {
+            "type": "integer",
+            "format": "int64"
+          },
+          "message": {
+            "type": "string"
+          }
+        },
+        "required": [
+          "code",
+          "message"
+        ]
+      }
+    },
+    "responses": {
+      "Error": {
+        "description": "General Error",
+        "content": {
+          "application/json": {
+            "schema": {
+              "$ref": "#/components/schemas/Error"
+            }
+          }
+        }
+      }
+    }
+  }
+}