swagger-api / swagger-parser

Swagger Spec to Java POJOs
http://swagger.io
Apache License 2.0
778 stars 525 forks source link

Possible regression of issue #1066 - Inaccurate reporting of duplicate parameter values when parsing schema #1492

Open jwgcooke opened 3 years ago

jwgcooke commented 3 years ago

I am seeing an issue very similar to this: https://github.com/swagger-api/swagger-parser/issues/1066

using the sample schema:

{
  "openapi": "3.0.0",
  "info": {
    "title": "Test",
    "version": "1.0"
  },
  "paths": {
    "/foo": {
      "post": {
        "operationId": "foo",
        "parameters": [
          {
            "in": "header",
            "name": "x-something",
            "schema": {
              "type": "string"
            },
            "required": true
          },
          {
            "in": "header",
            "name": "x-something-else",
            "schema": {
              "type": "string"
            },
            "required": false
          }
        ],
        "responses": {
          "201": {
            "description": "Success–resource created."
          }
        }
      }
    },
    "/foo/bar": {
      "post": {
        "operationId": "foo2",
        "parameters": [
          {
            "$ref": "#/paths/~1foo/post/parameters/0"
          },
          {
            "$ref": "#/paths/~1foo/post/parameters/1"
          }
        ],
        "responses": {
          "201": {
            "description": "Success–resources created."
          }
        }
      }
    }
  }
}

when trying to parse this file I am getting the same error described in #1066, This issue was originally filed against the Atlassian Validator: https://bitbucket.org/atlassian/swagger-request-validator/issues/307/inaccurate-reporting-of-duplicate

They were able to reproduce this using 2.0.23 of the swagger-parser which they use to drive the parsing of the schemas.

sdoeringNew commented 3 years ago

Is referencing to a different path a thing?

By looking at OpenAPIDeserializer.java it isn't.

private Parameter getParameterDefinition(Parameter parameter) {
    if (parameter.get$ref() == null) {
        return parameter;
    }
    Object parameterSchemaName = extractSimpleName(parameter.get$ref()).getLeft();
    return Optional.ofNullable(components)
        .map(Components::getParameters)
        .map(parameters -> parameters.get(parameterSchemaName))
        .orElse(parameter);
}

Parameter references will be only looked within components. Furthermore the name of the parameter is either "0" or "1". The numbers will not be handled as indices in arrays.

The parse warning "There are duplicate parameter values" is only a subsequent error to the wrong parameter defintion above.

jwgcooke commented 3 years ago

The sample is a combined/de-referenced schema. I will go back to the vendor of the library that is handling the de-referencing and get their inputs on this as well. That being said my understanding of JSON Pointer syntax is that the path ref is actually valid syntax for pointing to specific array elements.

https://tools.ietf.org/html/rfc6901 (actual spec for json pointer) https://www.baeldung.com/json-pointer (concrete examples)

There is also nothing I see in the OpenAPI spec that specifically mandates the $ref be to an item under "#/components" (although in general this makes total sense).

sdoeringNew commented 3 years ago

On a sidenote... The Swagger Editor handles this referencing without any problems and certainly gives no warnings.

jwgcooke commented 3 years ago

As I expected the vendor for the other library indicated that they above example is indeed valid as per the OpenAPI spec and the JSON Pointer spec. They are using : https://github.com/APIDevTools/json-schema-ref-parser

To handle the de-referencing and (as you also indicate based on the swagger editor) this SHOULD be considered as a valid file.