Discriminator validation fails with NPE

[Sergey Kolbasov]

Hello there

First, thanks for the library. It's awesome to have an ability to check client/server implementation against the API contract straight in specs.

But I met with a problem that is a blocker for us atm.
According to the spec: https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#discriminatorObject

Object with discriminator might have no properties field:

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  discriminator:
    propertyName: pet_type

If I understand correctly, in the current implementation of Discriminator.checkValue there is a call to get properties node that returns null in case if field is missing and the very next line turns it into NPE.

[James Navin]

Thanks for raising this!

[Sergey Kolbasov]

@jnavin Sure thing. If you could give me a hint in what direction I should follow, I can try to land a PR to fix it.

[James Navin]

I had a quick look at the problem over the weekend. Its a bit more complicated than it first appears. The current discriminator validation as implemented works for the inheritance case using allOf that was supported in swagger v2. It looks like the use case for it has been expanded in openapi 3.x to now serve as a 'hint' for oneOf and anyOf. I missed that subtlety when I updated the lib.

I think the best way to unblock you right now is probably to disable the discriminator validation when there are no properties and/or when using oneOf / anyOf.

I am curious though - how are you using the discriminator in your case?

[Brandon Chapman]

I'm encountering the same issue. We are using a "oneOf" list of components and a mapping. Basically the same as the example here under the "Mapping Type Names" section.

https://swagger.io/docs/specification/data-models/inheritance-and-polymorphism/

[Brandon Chapman]

Looks like there was a unit test for this at some point but it is not present in master. https://bitbucket.org/atlassian/swagger-request-validator/pull-requests/82/added-discriminatormapping/diff

[Sergey Kolbasov]

@jnavin

Our case is quite similar to one in the documentation example:

MyResponseType:
  oneOf:
  - $ref: '#/components/schemas/Cat'
  - $ref: '#/components/schemas/Dog'
  - $ref: '#/components/schemas/Lizard'
  - $ref: 'https://gigantic-server.com/schemas/Monster/schema.json'
  discriminator:
    propertyName: pet_type
    mapping:
      dog: '#/components/schemas/Dog'
      monster: 'https://gigantic-server.com/schemas/Monster/schema.json'

[Zuki Sarusi]

we are having the same problem... any updates?

[Joe Danks]

Just hit this too. Any suggestions for work around?

[Dejan Mitrovic]

If it helps, I have this small patch that avoids the NPE.

diff --git a/swagger-request-validator-core/src/main/java/com/atlassian/oai/validator/schema/keyword/Discriminator.java b/swagger-request-validator-core/src/main/java/com/atlassian/oai/validator/schema/keyword/Discriminator.java
index e5bf096..78911af 100644
--- a/swagger-request-validator-core/src/main/java/com/atlassian/oai/validator/schema/keyword/Discriminator.java
+++ b/swagger-request-validator-core/src/main/java/com/atlassian/oai/validator/schema/keyword/Discriminator.java
@@ -74,6 +74,10 @@ public class Discriminator {
                 return;
             }

+            if (tree.getNode().get("oneOf") != null) {
+                return; // no need to check for 'properties', 'required' etc.
+            }
+
             final JsonNode properties = tree.getNode().get("properties");
             final List<String> propertyNames = stream(properties.fieldNames()).collect(toList());
             if (!properties.has(discriminatorFieldName)) {
@@ -206,8 +210,11 @@ public class Discriminator {

             });

-            if (mappingNode != null && mappingNode.get(discriminatorNode.textValue()) != null) {
-                discriminatorNode = mappingNode.get(discriminatorNode.textValue());
+            final Boolean useMappingNode = mappingNode != null && mappingNode.get(discriminatorNode.textValue()) != null;
+            if (useMappingNode) {
+                mappingNode.fields().forEachRemaining(e -> {
+                    validDiscriminatorValues.put(e.getKey(), e.getValue());
+                });
             }

             if (!validDiscriminatorValues.containsKey(discriminatorNode.textValue())) {
@@ -219,6 +226,10 @@ public class Discriminator {
                 );
             }

+            if (useMappingNode) {
+                discriminatorNode = mappingNode.get(discriminatorNode.textValue());
+            }
+
             final ListProcessingReport subReport = new ListProcessingReport(report.getLogLevel(), LogLevel.FATAL);
             final JsonPointer ptr = pointerToDiscriminator(data, discriminatorNode);
             final FullData newData = data.withSchema(schemaTree.setPointer(ptr));
@@ -234,15 +245,15 @@ public class Discriminator {
                         .putArgument("schema", ptr.toString())
                         .put("report", subReport.asJson()));
             }
-
         }

         private JsonPointer pointerToDiscriminator(final FullData data, final JsonNode discriminatorNode) {
+            final String discriminatorNodeText = sanitizeDiscriminatorNode(discriminatorNode.textValue());
             // Swagger 2.0 used 'definitions' while OpenAPI uses 'components/schemas'
             if (data.getSchema().getBaseNode().has("components")) {
-                return JsonPointer.of("components", "schemas", discriminatorNode.textValue());
+                return JsonPointer.of("components", "schemas", discriminatorNodeText);
             }
-            return JsonPointer.of("definitions", discriminatorNode.textValue());
+            return JsonPointer.of("definitions", discriminatorNodeText);
         }

         private JsonNode definitionsNode(final FullData data) {
@@ -254,6 +265,14 @@ public class Discriminator {
             return baseNode.get("definitions");
         }

+        private String sanitizeDiscriminatorNode(final String discriminatorNodeText) {
+            if (discriminatorNodeText.startsWith("#/")) {
+                final int n = discriminatorNodeText.lastIndexOf('/');
+                return discriminatorNodeText.substring(n + 1);
+            }
+            return discriminatorNodeText;
+        }
+
         @Override
         public String toString() {
             return keyword;

[Martin Soubeyrand]

@James Navin how could we move forward with this one?

[James Navin]

If you are able to raise a PR to address the problem I will happily review it. Otherwise I will try to prioritize this issue when I next get time to focus on this project, but that may not be for another few weeks.

[Leonard Osang]

Issue #254 was marked as a duplicate of this issue.

[Leonard Osang]

Created a PR for this here: https://bitbucket.org/atlassian/swagger-request-validator/pull-requests/156/166-discriminator-oneof/diff

It only covers the combination of oneOf and discriminator though.

[James Navin]

Available in v2.8.3

[James Navin]

Ive resolved this issue, as @{557057:2ddb174e-56e8-4433-97d4-f844b9f34b85} has fixed the NPE with oneOf. If you encounter it again please feel free to re-open.