Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix and test for issue 1758 #1760

Merged
merged 5 commits into from
Jul 8, 2022
Merged

fix and test for issue 1758 #1760

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
707adcb
fix and test for issue 1758
gracekarina Jun 25, 2022
d70bfb5
added more scenarios for refs validation
gracekarina Jun 29, 2022
ec4e0e4
adding examples scenario
gracekarina Jun 29, 2022
ee37090
rephrase error message
gracekarina Jul 8, 2022
344f4e5
Merge branch 'master' into issue1758
gracekarina Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions modules/swagger-parser-v3/src/main/java/io/swagger/v3/parser/util/OpenAPIDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,6 +958,9 @@ public PathItem getPathItem(ObjectNode obj, String location, ParseResult result)
} else {
pathItem.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/pathItems"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would detail what is the expected type, like "$ref target "+ref.textValue() +" is not of expected type pathItem");. Same applies to similar code below

}
if(result.isOpenapi31()){
String value = getString("summary", obj, false, location, result);
if (StringUtils.isNotBlank(value)) {
Expand Down Expand Up @@ -1535,6 +1538,9 @@ public Link getLink(ObjectNode linkNode, String location, ParseResult result) {
} else {
link.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/links"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if (result.isOpenapi31()) {
String desc = getString("description", linkNode, false, location, result);
if (StringUtils.isNotBlank(desc)) {
Expand Down Expand Up @@ -1659,6 +1665,9 @@ public Callback getCallback(ObjectNode node, String location, ParseResult result
} else {
callback.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/callbacks"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
return callback;
} else {
result.invalidType(location, "$ref", "string", node);
Expand Down Expand Up @@ -1905,10 +1914,13 @@ public Parameter getParameter(ObjectNode obj, String location, ParseResult resul
parameter = new Parameter();
String mungedRef = mungedRef(ref.textValue());
if (mungedRef != null) {
parameter.set$ref(mungedRef);
} else {
parameter.set$ref(ref.textValue());
}
parameter.set$ref(mungedRef);
}else {
parameter.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/parameters")||ref.textValue().startsWith("#/components/headers"))) {
gracekarina marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cdoe style, would replace meters")||ref.textVal with meters") || ref.textVal

result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if (result.isOpenapi31()) {
String desc = getString("description", obj, false, location, result);
if (StringUtils.isNotBlank(desc)) {
Expand Down Expand Up @@ -2115,6 +2127,10 @@ public Header getHeader(ObjectNode headerNode, String location, ParseResult resu
} else {
header.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/parameters")||ref.textValue().startsWith("#/components/headers"))) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cdoe style, would replace meters")||ref.textVal with meters") || ref.textVal

result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}

if (result.isOpenapi31()) {
String desc = getString("description", headerNode, false, location, result);
if (StringUtils.isNotBlank(desc)) {
Expand Down Expand Up @@ -2277,6 +2293,9 @@ public SecurityScheme getSecurityScheme(ObjectNode node, String location, ParseR
} else {
securityScheme.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/securitySchemes"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if (result.isOpenapi31()) {
String desc = getString("description", node, false, location, result);
if (StringUtils.isNotBlank(desc)) {
Expand Down Expand Up @@ -2698,6 +2717,9 @@ at the moment path passed as string (basePath) from upper components can be both
if(schema.get$ref().startsWith("#/components/schemas")){// it's internal
String refName = schema.get$ref().substring(schema.get$ref().lastIndexOf("/")+1);
localSchemaRefs.put(refName,location);
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/schemas"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
return schema;
} else {
Expand Down Expand Up @@ -3161,6 +3183,9 @@ public Example getExample(ObjectNode node, String location, ParseResult result)
} else {
example.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/examples"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if(result.isOpenapi31()){
String value = getString("summary", node, false, location, result);
if (StringUtils.isNotBlank(value)) {
Expand Down Expand Up @@ -3300,6 +3325,9 @@ public ApiResponse getResponse(ObjectNode node, String location, ParseResult res
} else {
apiResponse.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/responses"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if(result.isOpenapi31()){
String value = getString("description", node, false, location, result);
if (StringUtils.isNotBlank(value)) {
Expand Down Expand Up @@ -3550,6 +3578,9 @@ public RequestBody getRequestBody(ObjectNode node, String location, ParseResult
} else {
body.set$ref(ref.textValue());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/requestBodies"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
if (result.isOpenapi31()) {
String desc = getString("description", node, false, location, result);
if (StringUtils.isNotBlank(desc)) {
Expand Down Expand Up @@ -3806,6 +3837,9 @@ public Schema getJsonSchema(ObjectNode node, String location, ParseResult result
} else {
schema.set$ref(ref.asText());
}
if(ref.textValue().startsWith("#/components") && !(ref.textValue().startsWith("#/components/schemas"))) {
result.warning(location, "$ref target "+ref.textValue() +" is not of expected type");
}
} else {
result.invalidType(location, "$ref", "string", node);
}
Expand Down
21 changes: 20 additions & 1 deletion modules/swagger-parser-v3/src/test/java/io/swagger/v3/parser/test/OpenAPIV3ParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,26 @@ public class OpenAPIV3ParserTest {
protected int serverPort = getDynamicPort();
protected WireMockServer wireMockServer;

@Test
public void testIssue1758() throws Exception{
ParseOptions options = new ParseOptions();
SwaggerParseResult result = new OpenAPIV3Parser().readLocation("src/test/resources/issue1758.yaml", null, options);

Assert.assertNotNull(result);
Assert.assertNotNull(result.getOpenAPI());
assertEquals(result.getMessages().size(),9);
assertTrue(result.getMessages().contains("paths.'/path1'.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).parameters.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).responses.default.three.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).requestBody.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).responses.200.user.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).responses.200.content.'application/json'.schema.$ref target #/components/parameters/pet is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).responses.200.content.'application/json'.examples.one.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).responses.400.$ref target #/components/schemas/xFoo is not of expected type"));
assertTrue(result.getMessages().contains("paths.'/foo'(get).callbacks.$ref target #/components/schemas/xFoo is not of expected type"));

}

@Test(description = "Test for not setting the schema type as default")
public void testNotDefaultSchemaType() {
ParseOptions options = new ParseOptions();
Expand Down Expand Up @@ -516,7 +536,6 @@ public void testIssueSameRefsDifferentModelValid() {
options.setResolveFully(true);

final SwaggerParseResult openAPI = parser.readLocation("src/test/resources/same-refs-different-model-valid.yaml", null, options);
Yaml.prettyPrint(openAPI);
assertEquals(openAPI.getMessages().size(), 0);
}

Expand Down
55 changes: 55 additions & 0 deletions modules/swagger-parser-v3/src/test/resources/issue1758.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
openapi: 3.0.3
info:
title: Missing validation rule for schemas in Headers.
version: 1.0.0
servers:
- url: /
paths:
/path1:
$ref: '#/components/schemas/xFoo'
/foo:
get:
description: ok
parameters:
- $ref: '#/components/schemas/xFoo'
requestBody:
$ref: '#/components/schemas/xFoo'
responses:
default:
description: ok
headers:
three:
$ref: '#/components/schemas/xFoo'
'200':
description: successful operation
content:
application/json:
schema:
type: array
items:
$ref: '#/components/parameters/pet'
examples:
one:
$ref: '#/components/schemas/xFoo'
links:
user:
$ref: '#/components/schemas/xFoo'
'400':
$ref: '#/components/schemas/xFoo'
callbacks:
mainHook:
$ref: "#/components/schemas/xFoo"
components:
schemas:
xFoo:
type: string
description: This isn't validated correctly
parameters:
pet:
name: X-pet
in: header
required: false
schema:
type: string
format: uuid