From 1017506ce6d4855fac247e43acd428d9bbd8fd61 Mon Sep 17 00:00:00 2001 From: Graham Esau Date: Sun, 21 Mar 2021 20:09:32 +0000 Subject: [PATCH] Prevent stack overflow when using inline_subschemas --- schemars/src/gen.rs | 56 ++++++++--- .../expected/inline-subschemas-recursive.json | 95 +++++++++++++++++++ .../tests/expected/inline-subschemas.json | 28 +++--- schemars/tests/inline_subschemas.rs | 20 +++- 4 files changed, 169 insertions(+), 30 deletions(-) create mode 100644 schemars/tests/expected/inline-subschemas-recursive.json diff --git a/schemars/src/gen.rs b/schemars/src/gen.rs index 58d820b7..b7cdae73 100644 --- a/schemars/src/gen.rs +++ b/schemars/src/gen.rs @@ -11,7 +11,7 @@ use crate::flatten::Merge; use crate::schema::*; use crate::{visit::*, JsonSchema, Map}; use dyn_clone::DynClone; -use std::{any::Any, fmt::Debug}; +use std::{any::Any, collections::HashSet, fmt::Debug}; /// Settings to customize how Schemas are generated. /// @@ -146,10 +146,21 @@ impl SchemaSettings { /// let gen = SchemaGenerator::default(); /// let schema = gen.into_root_schema_for::(); /// ``` -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default)] pub struct SchemaGenerator { settings: SchemaSettings, definitions: Map, + pending_schema_names: HashSet, +} + +impl Clone for SchemaGenerator { + fn clone(&self) -> Self { + Self { + settings: self.settings.clone(), + definitions: self.definitions.clone(), + pending_schema_names: HashSet::new(), + } + } } impl From for SchemaGenerator { @@ -203,23 +214,28 @@ impl SchemaGenerator { /// If `T`'s schema depends on any [referenceable](JsonSchema::is_referenceable) schemas, then this method will /// add them to the `SchemaGenerator`'s schema definitions. pub fn subschema_for(&mut self) -> Schema { - if !T::is_referenceable() || self.settings.inline_subschemas { - return T::json_schema(self); - } - let name = T::schema_name(); - let reference = format!("{}{}", self.settings().definitions_path, name); - if !self.definitions.contains_key(&name) { - self.insert_new_subschema_for::(name); + let return_ref = T::is_referenceable() + && (!self.settings.inline_subschemas || self.pending_schema_names.contains(&name)); + + if return_ref { + let reference = format!("{}{}", self.settings().definitions_path, name); + if !self.definitions.contains_key(&name) { + self.insert_new_subschema_for::(name); + } + Schema::new_ref(reference) + } else { + self.json_schema_internal::(&name) } - Schema::new_ref(reference) } fn insert_new_subschema_for(&mut self, name: String) { let dummy = Schema::Bool(false); // insert into definitions BEFORE calling json_schema to avoid infinite recursion self.definitions.insert(name.clone(), dummy); - let schema = T::json_schema(self); + + let schema = self.json_schema_internal::(&name); + self.definitions.insert(name, schema); } @@ -259,8 +275,9 @@ impl SchemaGenerator { /// add them to the `SchemaGenerator`'s schema definitions and include them in the returned `SchemaObject`'s /// [`definitions`](../schema/struct.Metadata.html#structfield.definitions) pub fn root_schema_for(&mut self) -> RootSchema { - let mut schema = T::json_schema(self).into_object(); - schema.metadata().title.get_or_insert_with(T::schema_name); + let name = T::schema_name(); + let mut schema = self.json_schema_internal::(&name).into_object(); + schema.metadata().title.get_or_insert(name); let mut root = RootSchema { meta_schema: self.settings.meta_schema.clone(), definitions: self.definitions.clone(), @@ -279,8 +296,9 @@ impl SchemaGenerator { /// If `T`'s schema depends on any [referenceable](JsonSchema::is_referenceable) schemas, then this method will /// include them in the returned `SchemaObject`'s [`definitions`](../schema/struct.Metadata.html#structfield.definitions) pub fn into_root_schema_for(mut self) -> RootSchema { - let mut schema = T::json_schema(&mut self).into_object(); - schema.metadata().title.get_or_insert_with(T::schema_name); + let name = T::schema_name(); + let mut schema = self.json_schema_internal::(&name).into_object(); + schema.metadata().title.get_or_insert(name); let mut root = RootSchema { meta_schema: self.settings.meta_schema, definitions: self.definitions, @@ -352,6 +370,14 @@ impl SchemaGenerator { } } } + + fn json_schema_internal(&mut self, name: &str) -> Schema { + self.pending_schema_names.insert(name.to_owned()); + let schema = T::json_schema(self); + // FIXME schema name not removed if previous line panics + self.pending_schema_names.remove(name); + schema + } } /// A [Visitor](Visitor) which implements additional traits required to be included in a [SchemaSettings]. diff --git a/schemars/tests/expected/inline-subschemas-recursive.json b/schemars/tests/expected/inline-subschemas-recursive.json new file mode 100644 index 00000000..cba686fc --- /dev/null +++ b/schemars/tests/expected/inline-subschemas-recursive.json @@ -0,0 +1,95 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "RecursiveOuter", + "type": "object", + "properties": { + "direct": { + "anyOf": [ + { + "$ref": "#/definitions/RecursiveOuter" + }, + { + "type": "null" + } + ] + }, + "indirect": { + "type": [ + "object", + "null" + ], + "required": [ + "recursive" + ], + "properties": { + "recursive": { + "type": "object", + "properties": { + "direct": { + "anyOf": [ + { + "$ref": "#/definitions/RecursiveOuter" + }, + { + "type": "null" + } + ] + }, + "indirect": { + "anyOf": [ + { + "$ref": "#/definitions/RecursiveInner" + }, + { + "type": "null" + } + ] + } + } + } + } + } + }, + "definitions": { + "RecursiveOuter": { + "type": "object", + "properties": { + "direct": { + "anyOf": [ + { + "$ref": "#/definitions/RecursiveOuter" + }, + { + "type": "null" + } + ] + }, + "indirect": { + "type": [ + "object", + "null" + ], + "required": [ + "recursive" + ], + "properties": { + "recursive": { + "$ref": "#/definitions/RecursiveOuter" + } + } + } + } + }, + "RecursiveInner": { + "type": "object", + "required": [ + "recursive" + ], + "properties": { + "recursive": { + "$ref": "#/definitions/RecursiveOuter" + } + } + } + } +} \ No newline at end of file diff --git a/schemars/tests/expected/inline-subschemas.json b/schemars/tests/expected/inline-subschemas.json index 05716759..b7315534 100644 --- a/schemars/tests/expected/inline-subschemas.json +++ b/schemars/tests/expected/inline-subschemas.json @@ -1,23 +1,23 @@ { - "$schema": "https://spec.openapis.org/oas/3.0/schema/2019-04-02#/definitions/Schema", + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "MyJob", + "type": "object", + "required": [ + "spec" + ], "properties": { "spec": { + "type": "object", + "required": [ + "replicas" + ], "properties": { "replicas": { + "type": "integer", "format": "uint32", - "minimum": 0, - "type": "integer" + "minimum": 0.0 } - }, - "required": [ - "replicas" - ], - "type": "object" + } } - }, - "required": [ - "spec" - ], - "title": "MyJob", - "type": "object" + } } \ No newline at end of file diff --git a/schemars/tests/inline_subschemas.rs b/schemars/tests/inline_subschemas.rs index 62bc7cc2..c478c0ac 100644 --- a/schemars/tests/inline_subschemas.rs +++ b/schemars/tests/inline_subschemas.rs @@ -15,7 +15,25 @@ pub struct MyJobSpec { #[test] fn struct_normal() -> TestResult { - let mut settings = SchemaSettings::openapi3(); + let mut settings = SchemaSettings::default(); settings.inline_subschemas = true; test_generated_schema::("inline-subschemas", settings) } + +#[derive(Debug, JsonSchema)] +pub struct RecursiveOuter { + pub direct: Option>, + pub indirect: Option>, +} + +#[derive(Debug, JsonSchema)] +pub struct RecursiveInner { + pub recursive: RecursiveOuter, +} + +#[test] +fn struct_recursive() -> TestResult { + let mut settings = SchemaSettings::default(); + settings.inline_subschemas = true; + test_generated_schema::("inline-subschemas-recursive", settings) +}