From e0810cbde79f55fcc88dc25c68319c3e506a44e4 Mon Sep 17 00:00:00 2001 From: gracekarina Date: Sat, 2 Feb 2019 12:50:08 -0500 Subject: [PATCH] ref #294 - fix circular refs stack overflow --- pom.xml | 10 ++- .../swagger/inflector/utils/ResolverUtil.java | 75 +++++++++++++++---- .../test/examples/ExampleBuilderTest.java | 10 +++ .../swagger/test/utils/ResolverUtilTest.java | 17 ++++- src/test/swagger/issue-294/issue-294.yaml | 54 +++++++++++++ 5 files changed, 147 insertions(+), 19 deletions(-) create mode 100644 src/test/swagger/issue-294/issue-294.yaml diff --git a/pom.xml b/pom.xml index 23756a8b..49c36153 100644 --- a/pom.xml +++ b/pom.xml @@ -297,6 +297,11 @@ swagger-jersey2-jaxrs ${swagger-core-version} + + io.swagger + swagger-core + ${swagger-core-version} + io.swagger swagger-parser @@ -429,9 +434,10 @@ + 1.5.21 - 1.0.38 - 2.8.7 + 1.0.42 + 2.9.8 2.9.1 1.8.1 9.2.9.v20150224 diff --git a/src/main/java/io/swagger/inflector/utils/ResolverUtil.java b/src/main/java/io/swagger/inflector/utils/ResolverUtil.java index 538055fa..d9774100 100644 --- a/src/main/java/io/swagger/inflector/utils/ResolverUtil.java +++ b/src/main/java/io/swagger/inflector/utils/ResolverUtil.java @@ -34,6 +34,12 @@ public class ResolverUtil { private Map schemas; private Map resolvedModels = new HashMap<>(); private Map resolvedProperties = new HashMap<>(); + private Map processedModels = new HashMap<>(); + private Map processedProperties = new HashMap<>(); + + /* set resolveCircularRefsAsObjectRefs to true to allow (in some cases) resolving circular refs in spec + as circular object references in models/properties, see issue #984 */ + private boolean resolveCircularRefsAsObjectRefs = System.getProperty("resolveCircularRefsAsObjectRefs") == null ? false : Boolean.valueOf(System.getProperty("resolveCircularRefsAsObjectRefs")); public Map getResolvedModels() { return resolvedModels; @@ -100,8 +106,8 @@ public void resolvePath(Path path) { public Model resolveModel(Model schema) { if (schema instanceof RefModel) { String ref = ((RefModel) schema).getSimpleRef(); - Model resolved = schemas.get(ref); - if (resolved == null) { + Model definitionsSchema = schemas.get(ref); + if (definitionsSchema == null) { LOGGER.error("unresolved model " + ref); return schema; } @@ -109,13 +115,24 @@ public Model resolveModel(Model schema) { LOGGER.debug("avoiding infinite loop"); return this.resolvedModels.get(ref); } - this.resolvedModels.put(ref, schema); + if (!resolveCircularRefsAsObjectRefs) { + if (this.processedModels.containsKey(ref)) { + return this.processedModels.get(ref); + } - Model model = resolveModel(resolved); + if (this.processedProperties.containsKey(ref)) { + PropertyModelConverter converter = new PropertyModelConverter(); + return converter.propertyToModel(this.processedProperties.get(ref)); + } + this.processedModels.put(ref, schema); + } else { + this.resolvedModels.put(ref, schema); + } + Model resolved = resolveModel(definitionsSchema); // if we make it without a resolution loop, we can update the reference - this.resolvedModels.put(ref, model); - return model; + this.resolvedModels.put(ref, resolved); + return resolved; } if (schema instanceof ArrayModel) { ArrayModel arrayModel = (ArrayModel) schema; @@ -151,6 +168,7 @@ public Model resolveModel(Model schema) { } return model; } + return schema; } if (schema instanceof ComposedModel) { ComposedModel composedSchema = (ComposedModel) schema; @@ -166,7 +184,11 @@ public Model resolveModel(Model schema) { if (property.getRequired()) { requiredProperties.add(key); } - model.addProperty(key, resolveProperty(property)); + if (!resolveCircularRefsAsObjectRefs) { + model.addProperty(key, property); + } else { + model.addProperty(key, resolveProperty(property)); + } } } @@ -190,8 +212,8 @@ public Model resolveModel(Model schema) { private Property resolveProperty(Property property) { if (property instanceof RefProperty) { String ref = ((RefProperty) property).getSimpleRef(); - Model resolved = schemas.get(ref); - if (resolved == null) { + Model definitionsSchema = schemas.get(ref); + if (definitionsSchema == null) { LOGGER.error("unresolved model " + ref); return property; } @@ -215,15 +237,38 @@ private Property resolveProperty(Property property) { } - this.resolvedProperties.put(ref, property); - Model model = resolveModel(resolved); + if (!resolveCircularRefsAsObjectRefs) { + if (this.processedModels.containsKey(ref) || this.processedProperties.containsKey(ref)) { + LOGGER.debug("avoiding infinite loop"); + Model modelResolved = this.processedModels.get(ref); + Property propertyResolved = this.processedProperties.get(ref); + if (modelResolved != null) { + PropertyModelConverter converter = new PropertyModelConverter(); + Property convertedProperty = converter.modelToProperty(modelResolved); + if (convertedProperty instanceof UntypedProperty && modelResolved instanceof ModelImpl) { + Property property1 = createObjectProperty(modelResolved); + this.processedProperties.put(ref, property1); + return property1; + } else { + return convertedProperty; + } + } else if (propertyResolved != null) { + return propertyResolved; + } + + } + this.processedProperties.put(ref, property); + } else { + this.resolvedProperties.put(ref, property); + } + Model resolved = resolveModel(definitionsSchema); // if we make it without a resolution loop, we can update the reference - this.resolvedModels.put(ref, model); + this.resolvedModels.put(ref, resolved); PropertyModelConverter converter = new PropertyModelConverter(); - Property prop = converter.modelToProperty(model); - if (prop instanceof UntypedProperty && model instanceof ModelImpl) { - Property property1 = createObjectProperty(model); + Property prop = converter.modelToProperty(resolved); + if (prop instanceof UntypedProperty && resolved instanceof ModelImpl) { + Property property1 = createObjectProperty(resolved); this.resolvedProperties.put(ref, property1); return property1; } else { diff --git a/src/test/java/io/swagger/test/examples/ExampleBuilderTest.java b/src/test/java/io/swagger/test/examples/ExampleBuilderTest.java index 0e2afb5f..03df34cd 100644 --- a/src/test/java/io/swagger/test/examples/ExampleBuilderTest.java +++ b/src/test/java/io/swagger/test/examples/ExampleBuilderTest.java @@ -658,8 +658,10 @@ public void testCircularRefSchema() throws Exception { @Test public void testCircularRefSchemaInResponse() throws Exception { Swagger swagger = new SwaggerParser().read("./src/test/swagger/circuler-refs-SPLAT-56-2.yaml"); + System.setProperty("resolveCircularRefsAsObjectRefs", "true"); ResolverUtil resolverUtil = new ResolverUtil(); resolverUtil.resolveFully(swagger); + String yaml = Yaml.pretty().writeValueAsString(swagger); Response response = swagger.getPaths().get("/candidates").getOperationMap().get(HttpMethod.GET).getResponses().get("200"); Example example = ExampleBuilder.fromModel("", response.getResponseSchema(), swagger.getDefinitions(), new HashMap()); assertNotNull(example); @@ -693,6 +695,14 @@ public void testCircularRefSchemaInResponse() throws Exception { " \"selfObj\" : { }\n" + " }\n" + "}"); + + System.setProperty("resolveCircularRefsAsObjectRefs", "false"); + + swagger = new SwaggerParser().read("./src/test/swagger/circuler-refs-SPLAT-56-2.yaml"); + resolverUtil = new ResolverUtil(); + resolverUtil.resolveFully(swagger); + String yaml2 = Yaml.pretty().writeValueAsString(swagger); + assertEquals(yaml, yaml2); } @Test diff --git a/src/test/java/io/swagger/test/utils/ResolverUtilTest.java b/src/test/java/io/swagger/test/utils/ResolverUtilTest.java index bd9aaa9b..533b6375 100644 --- a/src/test/java/io/swagger/test/utils/ResolverUtilTest.java +++ b/src/test/java/io/swagger/test/utils/ResolverUtilTest.java @@ -10,8 +10,6 @@ import io.swagger.models.properties.ArrayProperty; import io.swagger.models.properties.ObjectProperty; import io.swagger.models.properties.Property; -import io.swagger.models.properties.RefProperty; -import io.swagger.models.properties.UntypedProperty; import io.swagger.parser.SwaggerParser; import io.swagger.sample.models.Dog; import io.swagger.util.Json; @@ -25,6 +23,21 @@ public class ResolverUtilTest { + + @Test + public void testIssue294() throws Exception { + Swagger swagger = new SwaggerParser().read("./src/test/swagger/issue-294/issue-294.yaml"); + new ResolverUtil().resolveFully(swagger); + Yaml.prettyPrint(swagger); + try { + Json.mapper().writeValueAsString(swagger); + + } + catch (Exception e) { + fail("Recursive loop found"); + } + } + @Test public void testRefs2() { Swagger swagger = new SwaggerParser().read("./src/test/swagger/anotherSpec.yaml"); diff --git a/src/test/swagger/issue-294/issue-294.yaml b/src/test/swagger/issue-294/issue-294.yaml new file mode 100644 index 00000000..44d890fd --- /dev/null +++ b/src/test/swagger/issue-294/issue-294.yaml @@ -0,0 +1,54 @@ +{ + "swagger": "2.0", + "info": { + "description": "Data", + "version": "1.0", + "title": "Data" + }, + "basePath": "/dcs/api", + "paths": { + "/concept": { + "get": { + "summary": "Retrieve settings", + "description": "settings", + "operationId": "getSettings", + "produces": [ + "application/json" + ], + "responses": { + "200": { + "description": "successful operation", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/Concept" + } + } + } + } + } + } + }, + "definitions": { + + "Concept": { + "type": "object", + "properties": { + "thing": { + "type": "array", + "uniqueItems": true, + "items": { + "$ref": "#/definitions/Thing" + } + } + } + }, + "Thing": { + "allOf": [ + { + "$ref": "#/definitions/Concept" + } + ] + } + } +}