Repository: swagger-api/swagger-parser
Pull request #1313 (2020-02-07 07:08:34)

enable STRICT_DUPLICATE_DETECTION in Jackson parser for Yaml
This PR is doing the same change as @sullis did in #1271 for JSON but for the YAML format.

Without the `STRICT_DUPLICATE_DETECTION`, when there are duplicates keys Jackson is silently ignoring the first one. This results in unpredictable behavior.

### Example

Following Yaml is invalid because of the 2 `"200"` keys in the response map:
```yaml
openapi: 3.0.1
info:
  title: title
  description: This is a sample server
  license:
    name: Apache-2.0
    url: http://www.apache.org/licenses/LICENSE-2.0.html
  version: 1.0.0
servers:
- url: https://api.absolute.org/v2
  description: An absolute path
paths:
  /whatever:
    get:
      summary: Some operation
      description: Some operation
      operationId: doWhatever
      responses:
        "200":
          description: OK
        "200":
          description: duplicate HTTP status code
```

As comparison the Swagger-Editor Javascript parser is doing a better job and displays an error:

<img width="1464" alt="image" src="https://user-images.githubusercontent.com/1222165/74007816-8dfc0700-497f-11ea-82cd-fa7b548c3517.png">

### Other example

```yaml
openapi: 3.0.1
info:
  title: test with duplicate keys
  version: '1.0'
paths:
  /ping:
    get:
      operationId: pingOperation
      x-foo: bar
      x-foo: baz
      responses:
        '201':
          description: OK
```

With this example `getVendorExtensions()` is returning only one of the two `x-foo` keys.

After the change of this PR, an error is thrown by parsing (which is good because the Yaml is invalid in this case).


Patch:
diff --git a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/ObjectMapperFactory.java b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/ObjectMapperFactory.java
index b6e4282d..1df3fdd5 100644
--- a/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/ObjectMapperFactory.java
+++ b/modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/ObjectMapperFactory.java
@@ -15,7 +15,7 @@ public class ObjectMapperFactory {
     }
 
     protected static ObjectMapper createJson(boolean includePathDeserializer, boolean includeResponseDeserializer) {
-        return create(null, includePathDeserializer, includeResponseDeserializer);
+        return create(createJsonFactory(), includePathDeserializer, includeResponseDeserializer);
     }
 
     public static ObjectMapper createYaml() {
@@ -23,11 +23,11 @@ public class ObjectMapperFactory {
     }
 
     protected static ObjectMapper createYaml(boolean includePathDeserializer, boolean includeResponseDeserializer) {
-        return create(new YAMLFactory(), includePathDeserializer, includeResponseDeserializer);
+        return create(createYamlFactory(), includePathDeserializer, includeResponseDeserializer);
     }
 
     private static ObjectMapper create(JsonFactory jsonFactory, boolean includePathDeserializer, boolean includeResponseDeserializer) {
-        ObjectMapper mapper = jsonFactory == null ? new ObjectMapper(createJsonFactory()) : new ObjectMapper(jsonFactory);
+        ObjectMapper mapper = new ObjectMapper(jsonFactory);
 
         mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
         mapper.configure(SerializationFeature.FAIL_ON_EMPTY_BEANS, false);
@@ -42,4 +42,10 @@ public class ObjectMapperFactory {
           .enable(StreamReadFeature.STRICT_DUPLICATE_DETECTION)
           .build();
     }
+
+    private static JsonFactory createYamlFactory() {
+        return YAMLFactory.builder()
+          .enable(StreamReadFeature.STRICT_DUPLICATE_DETECTION)
+          .build();
+    }
 }


Test patch:
diff --git a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
index 8e8e31a2..531083c0 100644
--- a/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
+++ b/modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
@@ -180,7 +180,6 @@ public class OpenAPIV3ParserTest {
         assertNotNull(apispec);
     }
 
-
     @Test
     public void testIssue339() throws Exception {
         OpenAPIV3Parser openAPIV3Parser = new OpenAPIV3Parser();
@@ -522,7 +521,6 @@ public class OpenAPIV3ParserTest {
         Assert.assertEquals(s.getPattern(),"^[A-Z]+$"); //ERROR: got null
     }
 
-
     @BeforeClass
     private void setUpWireMockServer() throws IOException {
         this.wireMockServer = new WireMockServer(WireMockConfiguration.wireMockConfig().dynamicPort());
@@ -867,7 +865,6 @@ public class OpenAPIV3ParserTest {
         Assert.assertEquals("1516042231144", date.getExample().toString());
     }
 
-
     @Test
     public void testRefPaths() throws Exception {
         String yaml = "openapi: '3.0.0'\n" +
@@ -1234,10 +1231,8 @@ public class OpenAPIV3ParserTest {
 
         assertEquals(((Map) openAPI.getExtensions().get("x-some-vendor")).get("sometesting"), "bye!");
         assertEquals(openAPI.getPaths().get("/foo").getExtensions().get("x-something"), "yes, it is supported");
-       
     }
 
-
     @Test
     public void testIssue292WithCSVCollectionFormat() {
         String yaml =
@@ -1898,7 +1893,7 @@ public class OpenAPIV3ParserTest {
         assertEquals(parameter.getIn(), "path");
         assertEquals(parameter.getName(), "playerId");
     }
-  
+
     @Test
     public void testIssue884() {
         ParseOptions parseOptions = new ParseOptions();
@@ -1910,7 +1905,7 @@ public class OpenAPIV3ParserTest {
         assertEquals(operationId, "getRepository");
         assertNotNull(userRepository.getHeaders());
     }
-  
+
     @Test
     public void testLinkIssue() {
         ParseOptions parseOptions = new ParseOptions();
@@ -2067,10 +2062,10 @@ public class OpenAPIV3ParserTest {
 
     @Test
     public void shouldParseApiWithParametersUsingContentvsSchema() {
-    	// Tests that the content method of specifying the format of a parameter
-    	// gets resolved.
-    	// Test checks if an API's single parameter of array type gets fully resolved to 
-    	// referenced definitions.
+        // Tests that the content method of specifying the format of a parameter
+        // gets resolved.
+        // Test checks if an API's single parameter of array type gets fully resolved to 
+        // referenced definitions.
         String location = "src/test/resources/issue-1078/api.yaml";
         ParseOptions options = new ParseOptions();
         options.setResolve(true);
@@ -2261,7 +2256,7 @@ public class OpenAPIV3ParserTest {
     }
 
     @Test
-    public void testDuplicateHttpStatusCodes() {
+    public void testDuplicateHttpStatusCodesJson() {
         final String location = "src/test/resources/duplicateHttpStatusCodes.json";
 
         final ParseOptions options = new ParseOptions();
@@ -2276,6 +2271,22 @@ public class OpenAPIV3ParserTest {
 
     }
 
+    @Test
+    public void testDuplicateHttpStatusCodesYaml() {
+        final String location = "src/test/resources/duplicateHttpStatusCodes.yaml";
+
+        final ParseOptions options = new ParseOptions();
+        options.setResolve(true);
+
+        final OpenAPIV3Parser parser = new OpenAPIV3Parser();
+        final SwaggerParseResult result = parser.readLocation(location, null, options);
+        assertNull(result.getOpenAPI());
+        List<String> messages = result.getMessages();
+        assertEquals(1, messages.size());
+        assertEquals(messages.get(0), "Duplicate field '200' in `src/test/resources/duplicateHttpStatusCodes.yaml`");
+
+    }
+
     private static int getDynamicPort() {
         return new Random().ints(10000, 20000).findFirst().getAsInt();
     }
diff --git a/modules/swagger-parser-v3/src/test/resources/duplicateHttpStatusCodes.yaml b/modules/swagger-parser-v3/src/test/resources/duplicateHttpStatusCodes.yaml
new file mode 100644
index 00000000..d4d001f7
--- /dev/null
+++ b/modules/swagger-parser-v3/src/test/resources/duplicateHttpStatusCodes.yaml
@@ -0,0 +1,22 @@
+openapi: 3.0.1
+info:
+  title: title
+  description: This is a sample server
+  license:
+    name: Apache-2.0
+    url: http://www.apache.org/licenses/LICENSE-2.0.html
+  version: 1.0.0
+servers:
+- url: https://api.absolute.org/v2
+  description: An absolute path
+paths:
+  /whatever:
+    get:
+      summary: Some operation
+      description: Some operation
+      operationId: doWhatever
+      responses:
+        "200":
+          description: OK
+        "200":
+          description: duplicate HTTP status code
\ No newline at end of file
diff --git a/modules/swagger-parser-v3/src/test/resources/oas3.yaml.template b/modules/swagger-parser-v3/src/test/resources/oas3.yaml.template
index 97ca74a1..01b9c2bc 100644
--- a/modules/swagger-parser-v3/src/test/resources/oas3.yaml.template
+++ b/modules/swagger-parser-v3/src/test/resources/oas3.yaml.template
@@ -37,7 +37,6 @@ security:
   - tokenAuth: []
 info:
   description: 'This is a sample server Petstore'
-  version: 1.0.0
   title: Sample Pet Store App
   termsOfService: http://swagger.io/terms/
   x-info: info extension
diff --git a/modules/swagger-parser-v3/src/test/resources/petstore.yaml b/modules/swagger-parser-v3/src/test/resources/petstore.yaml
index d9260ddf..c33acc89 100644
--- a/modules/swagger-parser-v3/src/test/resources/petstore.yaml
+++ b/modules/swagger-parser-v3/src/test/resources/petstore.yaml
@@ -450,7 +450,6 @@ paths:
       summary: Logs user into the system
       description: ''
       operationId: loginUser
-      security: []
       parameters:
         - name: username
           in: query