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.

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

Exclude OpenAPI Examples from Response Validation #931

Open
mayer-maximilian opened this issue Jun 19, 2024 · 0 comments
Open

Exclude OpenAPI Examples from Response Validation #931

mayer-maximilian opened this issue Jun 19, 2024 · 0 comments

Comments

@mayer-maximilian
Copy link

Describe the bug

When response validation is enabled, and my spec contains several examples with a field called id containing the same string value, the express-openapi-validator in version 5.2.0 produces an unexpected error. This is not the case for request validation.

OpenAPI examples are removed from the apiDoc before request validation, but not before response validation. Some weird behavior in ajv seems to trigger the error when compiling the schema.

It probably makes sense to remove examples before response validation as well.

To Reproduce

Use the following OpenAPI spec with version 5.2.0 of theexpress-openapi-validator (this isn't a problem in 4.8.x):

openapi: '3.0.0'

info:
  version: 1.0.0
  title: Validation Error
  description: A test spec that triggers an validation error on identical id fields in examples.

servers:
  - url: http://{base_url}/{env_id}
    variables:
      base_url:
        default: my.app.com
        description: server
      env_id:
        default: v1
        description: path selector
paths:
  /ping:
    post:
      description: |
        ping then pong!
      operationId: ping
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Response"
              examples:
                response1:
                  summary: "Response 1"
                  value:
                    id: 'Some_ID'
                    message: 'Message 1'
                response2:
                  summary: "Response 2"
                  value:
                    id: 'Some_ID'
                    message: 'Message 2'

components:
  schemas:
    Response:
      required:
        - id
        - message
      properties:
        id:
          type: string
        message:
          type: string

Invoke response validation as follows:

const apiSpec = path.join(__dirname, 'openapi.yml');

app.use(
  OpenApiValidator.middleware({
    apiSpec,
    validateResponses: true
  }),
);

Setting validateResponse to false and validateRequest to true will not produce the error.

Actual behavior
the express-openapi-validator produces the following error:

Error Message: "reference \"Some_ID\" resolves to more than one schema" (for full stack trace, see below)

Expected behavior
Validation of the response, without producing the described error.

Examples and context

Version of express-openapi-validator: 5.2.0

It seems like ajv currently suffers from a compatibility issue regarding properties named id, where the id property is used to differentiate JSON schemas generated by avj. If a property id only defines a new sub-schema, this is no problem. However, when populated with a concrete string value, two schemas having the same value will collide during compilation, resulting in the following error when validating responses:

"error": {
    "message": "reference \"<ID_STRING>\" resolves to more than one schema",
    "stack": [
      "Error: reference \"<ID_STRING>\" resolves to more than one schema",
      "    at ambiguos (/Users/usr/project/node_modules/ajv/lib/compile/resolve.ts:147:12)",
      "    at Ajv.addRef (/Users/usr/project/node_modules/ajv/lib/compile/resolve.ts:115:38)",
      "    at /Users/usr/project/node_modules/ajv/lib/compile/resolve.ts:106:59",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:69:5)",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:83:9)",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:83:9)",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:83:9)",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:83:9)",
      "    at _traverse (/Users/usr/project/node_modules/json-schema-traverse/index.js:83:9)",
      "    at module.exports (/Users/usr/project/node_modules/json-schema-traverse/index.js:14:3)",
      "    at Ajv.getSchemaRefs (/Users/usr/project/node_modules/ajv/lib/compile/resolve.ts:102:3)",
      "    at Ajv._addSchema (/Users/usr/project/node_modules/ajv/lib/core.ts:713:37)",
      "    at Ajv.compile (/Users/usr/project/node_modules/ajv/lib/core.ts:384:22)",
      "    at ResponseValidator.buildValidators (/Users/usr/project/node_modules/@xxx/api-validation/node_modules/express-openapi-validator/src/middlewares/openapi.response.validator.ts:302:8)",
      "    at ResponseValidator._getOrBuildValidator (/Users/usr/project/node_modules/@xxx/api-validation/node_modules/express-openapi-validator/src/middlewares/openapi.response.validator.ts:113:12)",
      "    at /Users/usr/project/node_modules/@xxx/api-validation/node_modules/express-openapi-validator/src/middlewares/openapi.response.validator.ts:64:26",
    ]
  }

Luckily, it is rather rare that schema properties are defined with a fixed string. However, this is not true for schema examples. In our case, we had several examples using the same value for a property called id. This resulted in the above error when performing response validation, but not request validation. Digging in the code, it turns out examples are actually removed from the generated schema in the RequestValidator, but not for the ResponseValidator (see this line in the RequestValidator vs this function in the ResponseValidator).

As examples should never matter in the validation of responses, we suggest to remove examples in this case as well. In fact, probably removing the examples earlier, e.g., in this function, would be a good idea, if there is no use case I am missing.

If desired, I can create a pull request for this, but I am not sure if I will be able to take all edge cases into account.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant