Skip to content

Commit 79e600c

Browse files
authored
fixed NPE in OpenAPI validation, added test to run official JSON Schema test suite (needs to be downloaded seperately) (membrane#1906)
* fixed NPE in OpenAPI validation, added test to run official JSON Schema test suite (needs to be downloaded seperately) * refactor * added documentation * refactor * replaced 'System.out' by 'LOG' * small JSON Schema fixes - fixed NPE - support "type":"null" in schema - do not accept "123.45" as a valid number, when this appears in a JSON - JSON Schema 'pattern' is not anchored * added tests * fixed openapi validation example: json schema patterns are not anchored * refactor
1 parent 5a638f2 commit 79e600c

File tree

16 files changed

+360
-12
lines changed

16 files changed

+360
-12
lines changed

core/src/main/java/com/predic8/membrane/core/openapi/validators/AbstractBodyValidator.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ protected ValidationErrors validateBodyAccordingToMediaType(ValidationContext ct
4343
// Use the value of the OpenAPI spec for comparison, so it can not
4444
// be influenced from the outside.
4545
if ( APPLICATION_JSON_CONTENT_TYPE.match(mediaType)) {
46-
if (mediaTypeObj.getSchema().get$ref() != null) {
47-
ctx.schemaType(mediaTypeObj.getSchema().get$ref());
46+
if (mediaTypeObj.getSchema() != null && mediaTypeObj.getSchema().get$ref() != null) {
47+
ctx = ctx.schemaType(mediaTypeObj.getSchema().get$ref());
4848
}
4949
errors.add(new SchemaValidator(api, mediaTypeObj.getSchema()).validate(ctx, message.getBody()));
5050
} else if(isXML(mediaType)) {

core/src/main/java/com/predic8/membrane/core/openapi/validators/ArrayValidator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public String canValidate(Object value) {
4747

4848
@Override
4949
public ValidationErrors validate(ValidationContext ctx, Object value) {
50-
ctx.schemaType("array");
50+
ctx = ctx.schemaType("array");
5151

5252
ValidationErrors errors = new ValidationErrors();
5353
Schema itemsSchema = schema.getItems();

core/src/main/java/com/predic8/membrane/core/openapi/validators/IJSONSchemaValidator.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
interface IJSONSchemaValidator {
2020

21+
String NULL = "null";
2122
String NUMBER = "number";
2223
String ARRAY = "array";
2324
String OBJECT = "object";
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2022 predic8 GmbH, www.predic8.com
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.predic8.membrane.core.openapi.validators;
18+
19+
import com.fasterxml.jackson.databind.node.NullNode;
20+
21+
public class NullValidator implements IJSONSchemaValidator {
22+
23+
@Override
24+
public String canValidate(Object value) {
25+
if (value == null) {
26+
return NULL;
27+
}
28+
if (value instanceof NullNode) {
29+
return NULL;
30+
}
31+
return null;
32+
}
33+
34+
/**
35+
* Check, if value is null.
36+
*/
37+
@Override
38+
public ValidationErrors validate(ValidationContext ctx, Object value) {
39+
if (value == null) {
40+
return null;
41+
}
42+
if (value instanceof NullNode) {
43+
return null;
44+
}
45+
return ValidationErrors.create(ctx.schemaType(NULL), String.format("%s is not null.", value));
46+
}
47+
}

core/src/main/java/com/predic8/membrane/core/openapi/validators/NumberValidator.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,17 @@
1717
package com.predic8.membrane.core.openapi.validators;
1818

1919
import com.fasterxml.jackson.databind.*;
20+
import com.fasterxml.jackson.databind.node.TextNode;
2021

2122
import java.math.*;
2223

2324
import static java.lang.Double.*;
2425

26+
/**
27+
* When numbers appear in parameters, they enter as Strings (which is OK).
28+
*
29+
* If numbers appear in a JSON string "123.45", this is a TextNode (which is not OK). (See JSON Schema Spec.)
30+
*/
2531
public class NumberValidator implements IJSONSchemaValidator {
2632

2733
@Override
@@ -30,6 +36,9 @@ public String canValidate(Object value) {
3036
if (value instanceof Number) {
3137
return NUMBER;
3238
}
39+
if (value instanceof TextNode) {
40+
return null;
41+
}
3342
if (value instanceof JsonNode jn) {
3443
new BigDecimal((jn).asText());
3544
return NUMBER;
@@ -49,6 +58,9 @@ public String canValidate(Object value) {
4958
@Override
5059
public ValidationErrors validate(ValidationContext ctx, Object value) {
5160
try {
61+
if (value instanceof TextNode) {
62+
return ValidationErrors.create(ctx.schemaType(NUMBER), String.format("%s is not a number.", value));
63+
}
5264
if (value instanceof JsonNode jn) {
5365
// Not using double prevents from losing fractions
5466
new BigDecimal(jn.asText());

core/src/main/java/com/predic8/membrane/core/openapi/validators/SchemaValidator.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,16 @@ public ValidationErrors validate(ValidationContext ctx, Object obj) {
107107
}
108108

109109
private boolean isNullable() {
110-
return (schema.getNullable() != null && schema.getNullable()) || schema.getTypes().contains("null");
110+
return (schema.getNullable() != null && schema.getNullable()) || (schema.getTypes() != null && schema.getTypes().contains("null")) ||
111+
(schema.getType() != null && schema.getType().equals("null"));
111112
}
112113

113114
private ValidationErrors validateByType(ValidationContext ctx, Object value) {
114115

115116
String type = schema.getType();
116117

117118
if (schemaHasNoTypeAndTypes(type)) {
118-
return null;
119+
return validateMultipleTypes(List.of("string", "number", "integer", "boolean", "array", "object", "null"), ctx, value);
119120
}
120121

121122
// type in schema has only one type
@@ -183,6 +184,7 @@ private static String getType(Object obj) {
183184
private ValidationErrors validateSingleType(ValidationContext ctx, Object value, String type) {
184185
try {
185186
return switch (type) {
187+
case NULL -> new NullValidator().validate(ctx, value);
186188
case NUMBER -> new NumberValidator().validate(ctx, value);
187189
case "integer" -> new IntegerValidator().validate(ctx, value);
188190
case "string" -> new StringValidator(schema).validate(ctx, value);
@@ -202,6 +204,7 @@ private boolean schemaHasNoTypeAndTypes(String type) {
202204

203205
private static List<IJSONSchemaValidator> getValidatorClasses() {
204206
return List.of(
207+
new NullValidator(),
205208
new IntegerValidator(),
206209
new NumberValidator(),
207210
new StringValidator(null),

core/src/main/java/com/predic8/membrane/core/openapi/validators/StringValidator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,6 @@ private String getEnumValues() {
204204
}
205205

206206
private boolean matchRegexPattern(String v) {
207-
return Pattern.compile(schema.getPattern()).matcher(v).matches();
207+
return Pattern.compile(schema.getPattern()).matcher(v).find();
208208
}
209209
}

core/src/test/java/com/predic8/membrane/core/openapi/validators/IntegerTest.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,20 @@ public void invalidMaximumInBody() {
106106
assertTrue(e.getMessage().contains("maximum"));
107107
}
108108

109+
@Test
110+
public void invalidMaximumWithoutTypeInBody() {
111+
ValidationErrors errors = validator.validate(Request.post().path("/integer").body(new JsonBody(getNumbers("maximum-without-type",new BigDecimal(13)))));
112+
assertEquals(1,errors.size());
113+
ValidationError e = errors.get(0);
114+
assertTrue(e.getMessage().contains("maximum"));
115+
}
116+
117+
@Test
118+
public void validMaximumWithoutTypeInBody() {
119+
ValidationErrors errors = validator.validate(Request.post().path("/integer").body(new JsonBody(getNumbers("maximum-without-type",new BigDecimal(5)))));
120+
assertEquals(0,errors.size());
121+
}
122+
109123
private JsonNode getNumbers(String name, BigDecimal n) {
110124
ObjectNode root = om.createObjectNode();
111125
root.put(name,n);
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
package com.predic8.membrane.core.openapi.validators;
2+
3+
import com.fasterxml.jackson.core.JsonProcessingException;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
6+
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
7+
import com.predic8.membrane.core.openapi.OpenAPIValidator;
8+
import com.predic8.membrane.core.openapi.model.Body;
9+
import com.predic8.membrane.core.openapi.model.Request;
10+
import com.predic8.membrane.core.openapi.serviceproxy.OpenAPIRecord;
11+
import com.predic8.membrane.core.openapi.serviceproxy.OpenAPISpec;
12+
import com.predic8.membrane.core.util.URIFactory;
13+
import jakarta.mail.internet.ParseException;
14+
import org.jetbrains.annotations.NotNull;
15+
import org.jetbrains.annotations.Nullable;
16+
import org.junit.jupiter.api.Disabled;
17+
import org.junit.jupiter.api.Test;
18+
import org.slf4j.Logger;
19+
import org.slf4j.LoggerFactory;
20+
21+
import java.io.ByteArrayInputStream;
22+
import java.io.File;
23+
import java.io.IOException;
24+
import java.nio.charset.StandardCharsets;
25+
import java.util.HashMap;
26+
import java.util.List;
27+
import java.util.Map;
28+
import java.util.concurrent.atomic.AtomicReference;
29+
30+
import static com.google.common.collect.ImmutableMap.of;
31+
import static com.predic8.membrane.core.openapi.model.Request.post;
32+
import static com.predic8.membrane.core.openapi.util.TestUtils.om;
33+
import static com.predic8.membrane.core.openapi.util.TestUtils.parseOpenAPI;
34+
import static java.util.regex.Pattern.quote;
35+
import static org.junit.jupiter.api.Assertions.assertEquals;
36+
37+
/**
38+
* Runs the OpenAPI validator with the official tests from the JSON Schema Test Suite.
39+
*
40+
* The test suite needs to be downloaded manually.
41+
*/
42+
public class JsonSchemaTestSuiteTests {
43+
private final static Logger log = LoggerFactory.getLogger(JsonSchemaTestSuiteTests.class);
44+
public final String TEST_SUITE_BASE_PATH = "git\\JSON-Schema-Test-Suite\\tests\\draft6";
45+
46+
private final ObjectMapper objectMapper = new ObjectMapper();
47+
private final ObjectMapper yamlMapper = new ObjectMapper(new YAMLFactory().disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER));
48+
49+
int correct, incorrect, ignored;
50+
51+
@Disabled("The test requires a manual download. It also fails.")
52+
@Test
53+
void jsonSchema() throws IOException, ParseException {
54+
runTestsFoundInDirectory(TEST_SUITE_BASE_PATH);
55+
log.info("correct = {}", correct);
56+
log.info("incorrect = {}", incorrect);
57+
log.info("ignored = {}", ignored);
58+
59+
assertEquals(0, incorrect);
60+
}
61+
62+
private void runTestsFoundInDirectory(String baseDir) throws IOException, ParseException {
63+
File base = new File(baseDir);
64+
if (!base.exists()) {
65+
throw new RuntimeException("Please download the tests from https://github.com/json-schema-org/JSON-Schema-Test-Suite/ and adjust the base path here.");
66+
}
67+
for (File file : base.listFiles()) {
68+
if (!file.getName().endsWith(".json"))
69+
continue;
70+
runTestsFromFile(file);
71+
}
72+
}
73+
74+
private void runTestsFromFile(File file) throws IOException, ParseException {
75+
log.info("Testing file: {}", file.getName());
76+
77+
List<?> tests = objectMapper.readValue(file, List.class);
78+
for (Object t : tests) {
79+
Map test = (Map) t;
80+
String description = test.get("description").toString();
81+
Object schema = test.get("schema");
82+
List<?> testRuns = (List<?>) test.get("tests");
83+
84+
log.info("- description = {}", description);
85+
log.info(" schema = {}", om.writeValueAsString(schema));
86+
87+
String openapi = generateOpenAPIForSchema(schema);
88+
89+
String ignoredReason = computeIgnoredReason(openapi, description);
90+
91+
OpenAPIValidator validator = null;
92+
if (ignoredReason == null)
93+
validator = new OpenAPIValidator(new URIFactory(), new OpenAPIRecord(parseOpenAPI(
94+
new ByteArrayInputStream(openapi.getBytes(StandardCharsets.UTF_8))),new OpenAPISpec()));
95+
96+
for (Object tr : testRuns)
97+
runSingleTestRun((Map) tr, ignoredReason, validator);
98+
}
99+
}
100+
101+
private @NotNull String generateOpenAPIForSchema(Object schema) throws JsonProcessingException {
102+
Map openapi = new HashMap(of("openapi", "3.1.0", "paths", of("/test", of("post", of(
103+
"requestBody", of("content", of("application/json", of("schema", schema))),
104+
"responses", of("200", of("description", "OK")))))));
105+
106+
moveTypeDefinitions(schema, openapi, "definitions");
107+
moveTypeDefinitions(schema, openapi, "$defs");
108+
109+
return replaceReferences(replaceReferences(yamlMapper.writeValueAsString(openapi), "definitions"), "$defs");
110+
}
111+
112+
private String replaceReferences(String openApiString, String rootKey) {
113+
return openApiString.replaceAll("#/" + quote(rootKey) + "/", "#/components/schemas/");
114+
}
115+
116+
private static void moveTypeDefinitions(Object schema, Map openApi, String rootKey) {
117+
if (schema instanceof Map && ((Map) schema).containsKey(rootKey)) {
118+
openApi.put("components", of("schemas", ((Map) schema).get(rootKey)));
119+
log.warn(" warning: The schema contains definitions. They have been moved from #/" + rootKey + "/ to #/components/schemas/ .");
120+
}
121+
}
122+
123+
124+
private static @Nullable String computeIgnoredReason(String openapi, String description) {
125+
if (openapi.contains("http://"))
126+
return "the official test code seems to start a webserver on localhost:1234, which we do not support (yet).";
127+
if (description.equals("Location-independent identifier"))
128+
return "'$ref':'#foo' is used, which we do not support.";
129+
if (description.contains("empty tokens in $ref json-pointer"))
130+
return "the name of a definition is the empty string.";
131+
if (description.equals("relative pointer ref to array"))
132+
return "prefix reference migration not implemented";
133+
if (description.equals("relative pointer ref to object"))
134+
return "reference migration not implemented";
135+
return null;
136+
}
137+
138+
private void runSingleTestRun(Map tr, String ignoredReason, OpenAPIValidator validator) throws JsonProcessingException, ParseException {
139+
Map testRun = tr;
140+
141+
log.info(" - testRun = {}", om.writeValueAsString(testRun));
142+
143+
String description = testRun.get("description").toString();
144+
String body = objectMapper.writeValueAsString(testRun.get("data"));
145+
Boolean valid = (Boolean)testRun.get("valid");
146+
147+
log.info(" testRun.description = {}", description);
148+
log.info(" testRun.body = {}", body);
149+
log.info(" testRun.shouldBeValid = {}", valid);
150+
151+
if (ignoredReason != null) {
152+
ignored++;
153+
log.info(" Test: Ignored. ({})", ignoredReason);
154+
return;
155+
}
156+
157+
Request<Body> post = post().path("/test").mediaType("application/json");
158+
ValidationErrors errors = null;
159+
try {
160+
errors = validator.validate(post.body(body));
161+
162+
log.info(" validation result = {}", errors);
163+
} catch (Exception e) {
164+
e.printStackTrace();
165+
}
166+
167+
if (errors != null && errors.isEmpty() == valid) {
168+
log.info(" Test: OK");
169+
correct++;
170+
} else {
171+
log.warn(" Test: NOT OK!");
172+
incorrect++;
173+
}
174+
}
175+
}

0 commit comments

Comments
 (0)