Repository: swagger-api/swagger-parser
Pull request #943 (2018-11-24 03:29:57)

Recurssion issue
fix for #914  and #929 
Added a Property Resolved Map to avoid infinite loops


Patch:
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/ResolverFully.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/ResolverFully.java
index 8502da69..c3d358ba 100644
--- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/ResolverFully.java
+++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/ResolverFully.java
@@ -1,8 +1,10 @@
 package io.swagger.v3.parser.util;
 
+import io.swagger.v3.oas.models.Components;
 import io.swagger.v3.oas.models.OpenAPI;
 import io.swagger.v3.oas.models.Operation;
 import io.swagger.v3.oas.models.PathItem;
+import io.swagger.v3.oas.models.Paths;
 import io.swagger.v3.oas.models.callbacks.Callback;
 import io.swagger.v3.oas.models.examples.Example;
 import io.swagger.v3.oas.models.headers.Header;
@@ -19,7 +21,6 @@ import io.swagger.v3.oas.models.responses.ApiResponses;
 import io.swagger.v3.parser.models.RefFormat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -38,6 +39,8 @@ public class ResolverFully {
 
     private boolean aggregateCombinators;
 
+
+
     public ResolverFully() {
         this(true);
     }
@@ -53,52 +56,54 @@ public class ResolverFully {
     private Map<String, RequestBody> requestBodies;
     private Map<String, Header> headers;
     private Map<String, Link> links;
+    private Map<String, Schema> resolvedProperties = new HashMap<>();
 
     public void resolveFully(OpenAPI openAPI) {
-        if (openAPI.getComponents() != null && openAPI.getComponents().getRequestBodies() != null) {
-            requestBodies = openAPI.getComponents().getRequestBodies();
+        Components components = openAPI.getComponents();
+        if (components != null && components.getRequestBodies() != null) {
+            requestBodies = components.getRequestBodies();
             if (requestBodies == null) {
                 requestBodies = new HashMap<>();
             }
         }
 
-        if (openAPI.getComponents() != null && openAPI.getComponents().getSchemas() != null) {
-            schemas = openAPI.getComponents().getSchemas();
+        if (components != null && components.getSchemas() != null) {
+            schemas = components.getSchemas();
             if (schemas == null) {
                 schemas = new HashMap<>();
             }
         }
 
-        if (openAPI.getComponents() != null && openAPI.getComponents().getExamples() != null) {
-            examples = openAPI.getComponents().getExamples();
+        if (components != null && components.getExamples() != null) {
+            examples = components.getExamples();
             if (examples == null) {
                 examples = new HashMap<>();
             }
         }
 
-        if (openAPI.getComponents() != null && openAPI.getComponents().getHeaders() != null) {
-            headers = openAPI.getComponents().getHeaders();
+        if (components != null && components.getHeaders() != null) {
+            headers = components.getHeaders();
             if (headers == null) {
                 headers = new HashMap<>();
             }
         }  
 
-        if (openAPI.getComponents() != null && openAPI.getComponents().getParameters() != null) {
-            parameters = openAPI.getComponents().getParameters();
+        if (components != null && components.getParameters() != null) {
+            parameters = components.getParameters();
             if (parameters == null) {
                 parameters = new HashMap<>();
             }
         }
-        if (openAPI.getComponents() != null && openAPI.getComponents().getLinks() != null) {
-            links = openAPI.getComponents().getLinks();
+        if (components != null && components.getLinks() != null) {
+            links = components.getLinks();
             if (links == null) {
                 links = new HashMap<>();
             }
         }
-
-        if(openAPI.getPaths() != null) {
-            for (String pathname : openAPI.getPaths().keySet()) {
-                PathItem pathItem = openAPI.getPaths().get(pathname);
+        Paths paths = openAPI.getPaths();
+        if(paths != null) {
+            for (String pathname : paths.keySet()) {
+                PathItem pathItem = paths.get(pathname);
                 resolvePath(pathItem);
             }
         }
@@ -262,21 +267,24 @@ public class ResolverFully {
             String ref= schema.get$ref();
             ref = ref.substring(ref.lastIndexOf("/") + 1);
             Schema resolved = schemas.get(ref);
-            if(resolved == null) {
-                LOGGER.error("unresolved model " + ref);
-                return schema;
-            }
-            if(this.resolvedModels.containsKey(ref)) {
-                LOGGER.debug("avoiding infinite loop");
-                return this.resolvedModels.get(ref);
-            }
-            this.resolvedModels.put(ref, schema);
 
-            Schema model = resolveSchema(resolved);
+            if (resolved != null) {
 
-            // if we make it without a resolution loop, we can update the reference
-            this.resolvedModels.put(ref, model);
-            return model;
+                if (this.resolvedModels.containsKey(ref)) {
+                    LOGGER.debug("avoiding infinite loop");
+                    return resolvedModels.get(ref);
+                }
+                resolvedModels.put(ref, schema);
+                Schema model = resolveSchema(resolved);
+
+                // if we make it without a resolution loop, we can update the reference
+                resolvedModels.put(ref, model);
+
+                return model;
+
+            }else {
+                return schema;
+            }
         }
 
         if(schema instanceof ArraySchema) {
@@ -297,8 +305,14 @@ public class ResolverFully {
                     Schema innerProperty = obj.getProperties().get(propertyName);
                     // reference check
                     if(schema != innerProperty) {
-                        Schema resolved = resolveSchema(innerProperty);
-                        updated.put(propertyName, resolved);
+                        if(resolvedProperties.get(propertyName) == null && resolvedProperties.get(propertyName) != innerProperty) {
+                            LOGGER.debug("avoiding infinite loop");
+                            Schema resolved = resolveSchema(innerProperty);
+                            updated.put(propertyName, resolved);
+                            resolvedProperties.put(propertyName, resolved);
+                        }else {
+                            updated.put(propertyName, resolvedProperties.get(propertyName));
+                        }
                     }
                 }
                 obj.setProperties(updated);
@@ -331,7 +345,14 @@ public class ResolverFully {
                         if (resolved.getProperties() != null) {
                             for (String key : properties.keySet()) {
                                 Schema prop = (Schema) resolved.getProperties().get(key);
-                                model.addProperties(key, resolveSchema(prop));
+                                if(resolvedProperties.get(key) == null && resolvedProperties.get(key) != prop) {
+                                    LOGGER.debug("avoiding infinite loop");
+                                    Schema resolvedProp = resolveSchema(prop);
+                                    model.addProperties(key,resolvedProp );
+                                    resolvedProperties.put(key, resolvedProp);
+                                }else {
+                                    model.addProperties(key,resolvedProperties.get(key));
+                                }
                             }
                             if (resolved.getRequired() != null) {
                                 for (int i = 0; i < resolved.getRequired().size(); i++) {
@@ -372,7 +393,14 @@ public class ResolverFully {
                             if (resolved.getProperties() != null) {
                                 for (String key : properties.keySet()) {
                                     Schema prop = (Schema) resolved.getProperties().get(key);
-                                    model.addProperties(key, resolveSchema(prop));
+                                    if(resolvedProperties.get(key) == null && resolvedProperties.get(key) != prop) {
+                                        LOGGER.debug("avoiding infinite loop");
+                                        Schema resolvedProp = resolveSchema(prop);
+                                        model.addProperties(key,resolvedProp );
+                                        resolvedProperties.put(key, resolvedProp);
+                                    }else {
+                                        model.addProperties(key,resolvedProperties.get(key));
+                                    }
                                 }
                                 if (resolved.getRequired() != null) {
                                     for (int i = 0; i < resolved.getRequired().size(); i++) {
@@ -414,7 +442,14 @@ public class ResolverFully {
                             if (resolved.getProperties() != null) {
                                 for (String key : properties.keySet()) {
                                     Schema prop = (Schema) resolved.getProperties().get(key);
-                                    model.addProperties(key, resolveSchema(prop));
+                                    if(resolvedProperties.get(key) == null && resolvedProperties.get(key) != prop) {
+                                        LOGGER.debug("avoiding infinite loop");
+                                        Schema resolvedProp = resolveSchema(prop);
+                                        model.addProperties(key,resolvedProp );
+                                        resolvedProperties.put(key, resolvedProp);
+                                    }else {
+                                        model.addProperties(key,resolvedProperties.get(key));
+                                    }
                                 }
                                 if (resolved.getRequired() != null) {
                                     for (int i = 0; i < resolved.getRequired().size(); i++) {
@@ -463,8 +498,14 @@ public class ResolverFully {
             Map<String, Schema> properties = model.getProperties();
             for (String propertyName : properties.keySet()) {
                 Schema property = (Schema) model.getProperties().get(propertyName);
-                Schema resolved = resolveSchema(property);
-                updated.put(propertyName, resolved);
+                if(resolvedProperties.get(propertyName) == null && resolvedProperties.get(propertyName) != property) {
+                    LOGGER.debug("avoiding infinite loop");
+                    Schema resolved = resolveSchema(property);
+                    updated.put(propertyName, resolved);
+                    resolvedProperties.put(propertyName, resolved);
+                }else {
+                    updated.put(propertyName, resolvedProperties.get(propertyName));
+                }
             }
 
             for (String key : updated.keySet()) {


Test patch:
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
index 649f0b3f..e5558ed5 100644
--- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
+++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIResolverTest.java
@@ -8,6 +8,7 @@ import com.github.tomakehurst.wiremock.WireMockServer;
 import com.github.tomakehurst.wiremock.client.WireMock;
 import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
 import io.swagger.v3.core.util.Json;
+import io.swagger.v3.core.util.Yaml;
 import io.swagger.v3.oas.models.Components;
 import io.swagger.v3.oas.models.OpenAPI;
 import io.swagger.v3.oas.models.Operation;
@@ -69,6 +70,7 @@ import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertNull;
 import static org.testng.Assert.assertTrue;
+import static org.testng.FileAssert.fail;
 
 
 public class OpenAPIResolverTest {
@@ -432,7 +434,7 @@ public class OpenAPIResolverTest {
         OpenAPI openAPI = new OpenAPIV3Parser().readContents(yaml,auths,options).getOpenAPI();
         ResolverFully resolverUtil = new ResolverFully();
         resolverUtil.resolveFully(openAPI);
-        //System.out.println(openAPI.getPaths().get("/selfRefB").getGet().getRequestBody().getContent().get("application/json"));
+
 
         RequestBody body = openAPI.getPaths().get("/selfRefB").getGet().getRequestBody();
         Schema schema = body.getContent().get("application/json").getSchema();
@@ -595,6 +597,7 @@ public class OpenAPIResolverTest {
         assertTrue(allOf.getAllOf().get(0).getProperties().containsKey("street"));
         assertTrue(allOf.getAllOf().get(1).getProperties().containsKey("gps"));
 
+
         // Testing path item
         ComposedSchema schema = (ComposedSchema) openAPI.getPaths().get("/withInvalidComposedModel").getPost().getRequestBody().getContent().get("application/json").getSchema();
 
@@ -1134,6 +1137,35 @@ public class OpenAPIResolverTest {
         assertEquals(((Schema) coreSchema.getProperties().get("inner")).get$ref(), "#/components/schemas/innerCore");
     }
 
+    @Test
+    public void recursiveResolving() {
+        ParseOptions parseOptions = new ParseOptions();
+        parseOptions.setResolveFully(true);
+        OpenAPI openAPI = new OpenAPIV3Parser().read("recursive.yaml", null, parseOptions);
+        Assert.assertNotNull(openAPI.getPaths().get("/myPath").getGet().getResponses().get("200").getContent().get("application/json").getSchema().getProperties().get("myProp"));
+        try {
+            Json.mapper().writeValueAsString(openAPI);
+        }
+        catch (Exception e) {
+            fail("Recursive loop found");
+        }
+
+    }
+
+    @Test
+    public void recursiveResolving2() {
+        ParseOptions parseOptions = new ParseOptions();
+        parseOptions.setResolve(true);
+        parseOptions.setResolveFully(true);
+        OpenAPI openAPI = new OpenAPIV3Parser().read("recursive2.yaml", null, parseOptions);
+        try {
+            Json.mapper().writeValueAsString(openAPI);
+        }
+        catch (Exception e) {
+            fail("Recursive loop found");
+        }
+    }
+
     public String replacePort(String url){
         String pathFile = url.replace("${dynamicPort}", String.valueOf(this.serverPort));
         return pathFile;
diff --git a/modules/swagger-parser-v3/src/test/resources/recursive.yaml b/modules/swagger-parser-v3/src/test/resources/recursive.yaml
new file mode 100644
index 00000000..51b901c4
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/recursive.yaml
@@ -0,0 +1,33 @@
+openapi: 3.0.0
+info:
+  version: "0.0.2"
+paths:
+  /myPath:
+    get:
+      responses:
+        "200":
+          description: Success
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/Outer"
+            application/xml:
+              schema:
+                $ref: "#/components/schemas/Test"
+components:
+  schemas:
+    Outer:
+      allOf:
+      - $ref: "#/components/schemas/Inner"
+    Inner:
+      properties:
+        myProp:
+          type: array
+          items:
+            $ref: "#/components/schemas/Inner"
+    Test:
+      properties:
+        myProp:
+          type: array
+          items:
+            $ref: "#/components/schemas/Inner"
\ No newline at end of file
diff --git a/modules/swagger-parser-v3/src/test/resources/recursive2.yaml b/modules/swagger-parser-v3/src/test/resources/recursive2.yaml
new file mode 100644
index 00000000..301a2c51
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/recursive2.yaml
@@ -0,0 +1,44 @@
+openapi: 3.0.0
+info:
+  version: 'minimal'
+  title: 'recursion2'
+  description: 'problem in ResolverFully'
+
+paths:
+  /foo:
+    post:
+      responses:
+        '200':
+          description: Ok
+      requestBody:
+        $ref: '#/components/requestBodies/MyRequestBody'
+
+  /bar:
+    put:
+      responses:
+        '200':
+          description: Ok
+      requestBody:
+        $ref: '#/components/requestBodies/MyRequestBody'
+
+components:
+  schemas:
+    Schema1:
+      type: object
+      properties:
+        prop:
+          $ref: '#/components/schemas/Schema2'
+    Schema2:
+      type: object
+      properties:
+        prop1:
+          $ref: '#/components/schemas/Schema1'
+        prop2:
+          $ref: '#/components/schemas/Schema1'
+
+  requestBodies:
+    MyRequestBody:
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Schema2'
\ No newline at end of file