From 61a0a0568713700be51027591f573a87e18e989e Mon Sep 17 00:00:00 2001 From: Pavlo Shevchenko Date: Thu, 16 Feb 2023 11:54:35 +0100 Subject: [PATCH 1/2] Add failing test for misleading validation of optional objects --- .../schema/BaseJsonSchemaValidatorTest.java | 14 +++-- .../com/networknt/schema/Issue627Test.java | 2 - .../com/networknt/schema/Issue662Test.java | 60 +++++++++++++++++++ .../resources/issues/662/emptyObject.json | 1 + .../issues/662/objectInvalidValue.json | 5 ++ src/test/resources/issues/662/schema.json | 26 ++++++++ .../resources/issues/662/validObject.json | 5 ++ 7 files changed, 106 insertions(+), 7 deletions(-) delete mode 100644 src/test/java/com/networknt/schema/Issue627Test.java create mode 100644 src/test/java/com/networknt/schema/Issue662Test.java create mode 100644 src/test/resources/issues/662/emptyObject.json create mode 100644 src/test/resources/issues/662/objectInvalidValue.json create mode 100644 src/test/resources/issues/662/schema.json create mode 100644 src/test/resources/issues/662/validObject.json diff --git a/src/test/java/com/networknt/schema/BaseJsonSchemaValidatorTest.java b/src/test/java/com/networknt/schema/BaseJsonSchemaValidatorTest.java index 544a71f74..4c4d63ba6 100644 --- a/src/test/java/com/networknt/schema/BaseJsonSchemaValidatorTest.java +++ b/src/test/java/com/networknt/schema/BaseJsonSchemaValidatorTest.java @@ -30,11 +30,11 @@ */ public class BaseJsonSchemaValidatorTest { - private ObjectMapper mapper = new ObjectMapper(); + private static final ObjectMapper mapper = new ObjectMapper(); protected JsonNode getJsonNodeFromClasspath(String name) throws IOException { InputStream is1 = Thread.currentThread().getContextClassLoader() - .getResourceAsStream(name); + .getResourceAsStream(name); return mapper.readTree(is1); } @@ -46,10 +46,14 @@ protected JsonNode getJsonNodeFromUrl(String url) throws IOException { return mapper.readTree(new URL(url)); } - protected JsonSchema getJsonSchemaFromClasspath(String name) { - JsonSchemaFactory factory = JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V4); + protected static JsonSchema getJsonSchemaFromClasspath(String name) { + return getJsonSchemaFromClasspath(name, SpecVersion.VersionFlag.V4); + } + + protected static JsonSchema getJsonSchemaFromClasspath(String name, SpecVersion.VersionFlag schemaVersion) { + JsonSchemaFactory factory = JsonSchemaFactory.getInstance(schemaVersion); InputStream is = Thread.currentThread().getContextClassLoader() - .getResourceAsStream(name); + .getResourceAsStream(name); return factory.getSchema(is); } diff --git a/src/test/java/com/networknt/schema/Issue627Test.java b/src/test/java/com/networknt/schema/Issue627Test.java deleted file mode 100644 index f51b55b55..000000000 --- a/src/test/java/com/networknt/schema/Issue627Test.java +++ /dev/null @@ -1,2 +0,0 @@ -package com.networknt.schema;public class Issue627Test { -} diff --git a/src/test/java/com/networknt/schema/Issue662Test.java b/src/test/java/com/networknt/schema/Issue662Test.java new file mode 100644 index 000000000..1535f0aa6 --- /dev/null +++ b/src/test/java/com/networknt/schema/Issue662Test.java @@ -0,0 +1,60 @@ +package com.networknt.schema; + +import com.fasterxml.jackson.databind.JsonNode; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.List; +import java.util.Set; + +import static java.util.stream.Collectors.toList; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class Issue662Test extends BaseJsonSchemaValidatorTest { + + private static final String RESOURCE_PREFIX = "issues/662/"; + private static JsonSchema schema; + + @BeforeAll + static void setup() { + schema = getJsonSchemaFromClasspath(resource("schema.json"), SpecVersion.VersionFlag.V7); + } + + @Test + void testNoErrorsForEmptyObject() throws IOException { + JsonNode node = getJsonNodeFromClasspath(resource("emptyObject.json")); + Set errors = schema.validate(node); + assertTrue(errors.isEmpty(), "No validation errors for empty optional object"); + } + + @Test + void testNoErrorsForValidObject() throws IOException { + JsonNode node = getJsonNodeFromClasspath(resource("validObject.json")); + Set errors = schema.validate(node); + assertTrue(errors.isEmpty(), "No validation errors for a valid optional object"); + } + + @Test + void testCorrectErrorForInvalidValue() throws IOException { + JsonNode node = getJsonNodeFromClasspath(resource("objectInvalidValue.json")); + Set errors = schema.validate(node); + List errorMessages = errors.stream() + .map(ValidationMessage::getMessage) + .collect(toList()); + + assertTrue( + errorMessages.contains("$.optionalObject.value: does not have a value in the enumeration [one, two]"), + "Validation error for invalid object property is captured" + ); + assertFalse( + errorMessages.contains("$.optionalObject: object found, null expected"), + "No validation error that the object is not expected" + ); + } + + private static String resource(String name) { + return RESOURCE_PREFIX + name; + } +} diff --git a/src/test/resources/issues/662/emptyObject.json b/src/test/resources/issues/662/emptyObject.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/src/test/resources/issues/662/emptyObject.json @@ -0,0 +1 @@ +{} diff --git a/src/test/resources/issues/662/objectInvalidValue.json b/src/test/resources/issues/662/objectInvalidValue.json new file mode 100644 index 000000000..2245430f4 --- /dev/null +++ b/src/test/resources/issues/662/objectInvalidValue.json @@ -0,0 +1,5 @@ +{ + "optionalObject": { + "value": "invalid" + } +} diff --git a/src/test/resources/issues/662/schema.json b/src/test/resources/issues/662/schema.json new file mode 100644 index 000000000..3155e0fcf --- /dev/null +++ b/src/test/resources/issues/662/schema.json @@ -0,0 +1,26 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "optionalObject": { + "anyOf": [ + { + "type": "null" + }, + { + "type": "object", + "properties": { + "value": { + "enum": [ + "one", + "two" + ] + } + }, + "required": [ + "value" + ] + } + ] + } + } +} diff --git a/src/test/resources/issues/662/validObject.json b/src/test/resources/issues/662/validObject.json new file mode 100644 index 000000000..ce72d612a --- /dev/null +++ b/src/test/resources/issues/662/validObject.json @@ -0,0 +1,5 @@ +{ + "optionalObject": { + "value": "one" + } +} From f75b9559dbd8e121b8bcedcc1e339862ba7111b3 Mon Sep 17 00:00:00 2001 From: Pavlo Shevchenko Date: Thu, 16 Feb 2023 14:13:43 +0100 Subject: [PATCH 2/2] Look up `anyOf` type validators by the full schema path --- .../com/networknt/schema/AnyOfValidator.java | 350 +++++++++--------- 1 file changed, 177 insertions(+), 173 deletions(-) diff --git a/src/main/java/com/networknt/schema/AnyOfValidator.java b/src/main/java/com/networknt/schema/AnyOfValidator.java index f7e719b48..c0cf05685 100644 --- a/src/main/java/com/networknt/schema/AnyOfValidator.java +++ b/src/main/java/com/networknt/schema/AnyOfValidator.java @@ -1,173 +1,177 @@ -/* - * Copyright (c) 2016 Network New Technologies Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.networknt.schema; - -import com.fasterxml.jackson.databind.JsonNode; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import java.util.*; -import java.util.stream.Collectors; - -public class AnyOfValidator extends BaseJsonValidator implements JsonValidator { - private static final Logger logger = LoggerFactory.getLogger(RequiredValidator.class); - private static final String REMARK = "Remaining validation messages report why candidate schemas didn't match"; - private static final String DISCRIMINATOR_REMARK = "and the discriminator-selected candidate schema didn't pass validation"; - - private final List schemas = new ArrayList(); - private final ValidationContext.DiscriminatorContext discriminatorContext; - - public AnyOfValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentSchema, ValidationContext validationContext) { - super(schemaPath, schemaNode, parentSchema, ValidatorTypeCode.ANY_OF, validationContext); - this.validationContext = validationContext; - int size = schemaNode.size(); - for (int i = 0; i < size; i++) { - schemas.add(new JsonSchema(validationContext, - parentSchema.getSchemaPath() + "/" + getValidatorType().getValue() + "/" + i, - parentSchema.getCurrentUri(), - schemaNode.get(i), - parentSchema)); - } - - if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { - this.discriminatorContext = new ValidationContext.DiscriminatorContext(); - } else { - this.discriminatorContext = null; - } - } - - public Set validate(JsonNode node, JsonNode rootNode, String at) { - debug(logger, node, rootNode, at); - - // get the Validator state object storing validation data - ValidatorState state = (ValidatorState) CollectorContext.getInstance().get(ValidatorState.VALIDATOR_STATE_KEY); - - if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { - validationContext.enterDiscriminatorContext(this.discriminatorContext, at); - } - - boolean initialHasMatchedNode = state.hasMatchedNode(); - - Set allErrors = new LinkedHashSet(); - String typeValidatorName = "anyOf/type"; - - // As anyOf might contain multiple schemas take a backup of evaluatedProperties. - Object backupEvaluatedProperties = CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES); - - // Make the evaluatedProperties list empty. - CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, new ArrayList<>()); - - try { - int numberOfValidSubSchemas = 0; - for (JsonSchema schema : schemas) { - state.setMatchedNode(initialHasMatchedNode); - Set errors = new HashSet<>(); - if (schema.getValidators().containsKey(typeValidatorName)) { - TypeValidator typeValidator = ((TypeValidator) schema.getValidators().get(typeValidatorName)); - //If schema has type validator and node type doesn't match with schemaType then ignore it - //For union type, it is a must to call TypeValidator - if (typeValidator.getSchemaType() != JsonType.UNION && !typeValidator.equalsToSchemaType(node)) { - allErrors.add(buildValidationMessage(at, typeValidator.getSchemaType().toString())); - continue; - } - } - if (!state.isWalkEnabled()) { - errors = schema.validate(node, rootNode, at); - } else { - errors = schema.walk(node, rootNode, at, true); - } - - // check if any validation errors have occurred - if (errors.isEmpty()) { - // check whether there are no errors HOWEVER we have validated the exact validator - if (!state.hasMatchedNode()) { - continue; - } - // we found a valid subschema, so increase counter - numberOfValidSubSchemas++; - } - - if (errors.isEmpty() && (!this.validationContext.getConfig().isOpenAPI3StyleDiscriminators())) { - // Clear all errors. - allErrors.clear(); - // return empty errors. - return errors; - } else if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { - if (discriminatorContext.isDiscriminatorMatchFound()) { - if (!errors.isEmpty()) { - errors.add(buildValidationMessage(at, DISCRIMINATOR_REMARK)); - allErrors.addAll(errors); - } else { - // Clear all errors. - allErrors.clear(); - } - return errors; - } - } - allErrors.addAll(errors); - } - - // determine only those errors which are NOT of type "required" property missing - Set childNotRequiredErrors = allErrors.stream().filter(error -> !ValidatorTypeCode.REQUIRED.getValue().equals(error.getType())).collect(Collectors.toSet()); - - // in case we had at least one (anyOf, i.e. any number >= 1 of) valid subschemas, we can remove all other errors about "required" properties - if (numberOfValidSubSchemas >= 1 && childNotRequiredErrors.isEmpty()) { - allErrors = childNotRequiredErrors; - } - - if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators() && discriminatorContext.isActive()) { - final Set errors = new HashSet(); - errors.add(buildValidationMessage(at, "based on the provided discriminator. No alternative could be chosen based on the discriminator property")); - return Collections.unmodifiableSet(errors); - } - } finally { - if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { - validationContext.leaveDiscriminatorContextImmediately(at); - } - if (allErrors.isEmpty()) { - addEvaluatedProperties(backupEvaluatedProperties); - state.setMatchedNode(true); - } else { - CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedProperties); - } - } - return Collections.unmodifiableSet(allErrors); - } - - private void addEvaluatedProperties(Object backupEvaluatedProperties) { - // Add all the evaluated properties. - List backupEvaluatedPropertiesList = (backupEvaluatedProperties == null ? new ArrayList<>() : (List) backupEvaluatedProperties); - backupEvaluatedPropertiesList.addAll((List) CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES)); - CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedPropertiesList); - } - - @Override - public Set walk(JsonNode node, JsonNode rootNode, String at, boolean shouldValidateSchema) { - if (shouldValidateSchema) { - return validate(node, rootNode, at); - } - for (JsonSchema schema : schemas) { - schema.walk(node, rootNode, at, false); - } - return new LinkedHashSet<>(); - } - - @Override - public void preloadJsonSchema() { - preloadJsonSchemas(schemas); - } -} +/* + * Copyright (c) 2016 Network New Technologies Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.networknt.schema; + +import com.fasterxml.jackson.databind.JsonNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.*; +import java.util.stream.Collectors; + +public class AnyOfValidator extends BaseJsonValidator implements JsonValidator { + private static final Logger logger = LoggerFactory.getLogger(RequiredValidator.class); + private static final String DISCRIMINATOR_REMARK = "and the discriminator-selected candidate schema didn't pass validation"; + + private final List schemas = new ArrayList<>(); + private final ValidationContext.DiscriminatorContext discriminatorContext; + + public AnyOfValidator(String schemaPath, JsonNode schemaNode, JsonSchema parentSchema, ValidationContext validationContext) { + super(schemaPath, schemaNode, parentSchema, ValidatorTypeCode.ANY_OF, validationContext); + this.validationContext = validationContext; + int size = schemaNode.size(); + for (int i = 0; i < size; i++) { + schemas.add(new JsonSchema(validationContext, + getChildSchemaPath(i), + parentSchema.getCurrentUri(), + schemaNode.get(i), + parentSchema)); + } + + if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { + this.discriminatorContext = new ValidationContext.DiscriminatorContext(); + } else { + this.discriminatorContext = null; + } + } + + private String getChildSchemaPath(int childIdx) { + return parentSchema.getSchemaPath() + "/" + getValidatorType().getValue() + "/" + childIdx; + } + + public Set validate(JsonNode node, JsonNode rootNode, String at) { + debug(logger, node, rootNode, at); + + // get the Validator state object storing validation data + ValidatorState state = (ValidatorState) CollectorContext.getInstance().get(ValidatorState.VALIDATOR_STATE_KEY); + + if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { + validationContext.enterDiscriminatorContext(this.discriminatorContext, at); + } + + boolean initialHasMatchedNode = state.hasMatchedNode(); + + Set allErrors = new LinkedHashSet<>(); + + // As anyOf might contain multiple schemas take a backup of evaluatedProperties. + Object backupEvaluatedProperties = CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES); + + // Make the evaluatedProperties list empty. + CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, new ArrayList<>()); + + try { + int numberOfValidSubSchemas = 0; + for (int i = 0; i < schemas.size(); ++i) { + JsonSchema schema = schemas.get(i); + state.setMatchedNode(initialHasMatchedNode); + Set errors; + String typeValidatorName = getChildSchemaPath(i) + "/type"; + if (schema.getValidators().containsKey(typeValidatorName)) { + TypeValidator typeValidator = ((TypeValidator) schema.getValidators().get(typeValidatorName)); + //If schema has type validator and node type doesn't match with schemaType then ignore it + //For union type, it is a must to call TypeValidator + if (typeValidator.getSchemaType() != JsonType.UNION && !typeValidator.equalsToSchemaType(node)) { + allErrors.add(buildValidationMessage(at, typeValidator.getSchemaType().toString())); + continue; + } + } + if (!state.isWalkEnabled()) { + errors = schema.validate(node, rootNode, at); + } else { + errors = schema.walk(node, rootNode, at, true); + } + + // check if any validation errors have occurred + if (errors.isEmpty()) { + // check whether there are no errors HOWEVER we have validated the exact validator + if (!state.hasMatchedNode()) { + continue; + } + // we found a valid subschema, so increase counter + numberOfValidSubSchemas++; + } + + if (errors.isEmpty() && (!this.validationContext.getConfig().isOpenAPI3StyleDiscriminators())) { + // Clear all errors. + allErrors.clear(); + // return empty errors. + return errors; + } else if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { + if (discriminatorContext.isDiscriminatorMatchFound()) { + if (!errors.isEmpty()) { + errors.add(buildValidationMessage(at, DISCRIMINATOR_REMARK)); + allErrors.addAll(errors); + } else { + // Clear all errors. + allErrors.clear(); + } + return errors; + } + } + allErrors.addAll(errors); + } + + // determine only those errors which are NOT of type "required" property missing + Set childNotRequiredErrors = allErrors.stream().filter(error -> !ValidatorTypeCode.REQUIRED.getValue().equals(error.getType())).collect(Collectors.toSet()); + + // in case we had at least one (anyOf, i.e. any number >= 1 of) valid subschemas, we can remove all other errors about "required" properties + if (numberOfValidSubSchemas >= 1 && childNotRequiredErrors.isEmpty()) { + allErrors = childNotRequiredErrors; + } + + if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators() && discriminatorContext.isActive()) { + final Set errors = new HashSet(); + errors.add(buildValidationMessage(at, "based on the provided discriminator. No alternative could be chosen based on the discriminator property")); + return Collections.unmodifiableSet(errors); + } + } finally { + if (this.validationContext.getConfig().isOpenAPI3StyleDiscriminators()) { + validationContext.leaveDiscriminatorContextImmediately(at); + } + if (allErrors.isEmpty()) { + addEvaluatedProperties(backupEvaluatedProperties); + state.setMatchedNode(true); + } else { + CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedProperties); + } + } + return Collections.unmodifiableSet(allErrors); + } + + private void addEvaluatedProperties(Object backupEvaluatedProperties) { + // Add all the evaluated properties. + List backupEvaluatedPropertiesList = (backupEvaluatedProperties == null ? new ArrayList<>() : (List) backupEvaluatedProperties); + backupEvaluatedPropertiesList.addAll((List) CollectorContext.getInstance().get(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES)); + CollectorContext.getInstance().add(UnEvaluatedPropertiesValidator.EVALUATED_PROPERTIES, backupEvaluatedPropertiesList); + } + + @Override + public Set walk(JsonNode node, JsonNode rootNode, String at, boolean shouldValidateSchema) { + if (shouldValidateSchema) { + return validate(node, rootNode, at); + } + for (JsonSchema schema : schemas) { + schema.walk(node, rootNode, at, false); + } + return new LinkedHashSet<>(); + } + + @Override + public void preloadJsonSchema() { + preloadJsonSchemas(schemas); + } +}