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

anyOf/oneOf/allOf should not imply type=object #186

Closed
JacobLey opened this issue Dec 17, 2019 · 2 comments
Closed

anyOf/oneOf/allOf should not imply type=object #186

JacobLey opened this issue Dec 17, 2019 · 2 comments

Comments

@JacobLey
Copy link
Collaborator

JacobLey commented Dec 17, 2019

Hello,

I think I've hit an unintentional backwards incompatibility caused by this PR/commit: bc44495

In my case, I have an input that I accept as a number or a string (using allOf) to represent a timestamp. When provided a string, the JSON parsing fails, but I believe it should not be parsed in the first place, as I did not indicate it was an object.

Parsing is performed here: https://github.com/cdimascio/express-openapi-validator/blob/master/src/middlewares/openapi.request.validator.ts#L156

This example route should help clarify:

/my/get/endpoint:
  get:
    operationId: my.get
    summary: Failing Timestamp Get
    description: This endpoint will fail if query's "time" is an ISO string
    parameters:
      - name: time
        in: query
        description: Limit results to ones created before time
        schema:
          title: Input Timestamp
          description: Value passed to Javascript's `new Date()`.
          example: '2019-06-24T12:34:56.789Z'
          anyOf:
            - title: Number Timestamp
              type: integer
              description: Unix milliseconds
              example: 1234567890123
              nullable: true
            - type: string
              description: ISO Timestamp
              pattern: \S
    responses:
      200:
        description: My Response
        content:
        application/json:
          schema:
            type: object

When time is 2019-06-24T12:34:56.789Z I get an error Unexpected number in JSON at position 4

Based on the OpenAPI spec I don't think I would expect a JSON.parse to be attempted here. Unless I am incorrect? In which case I'd appreciate some guidance around how to properly format my schema.

I believe we should either only be parsing when type = object, or one of the any/one/all is of type object.

Or we could always attempt JSON.parse, but not fail on error (i.e. wrap in try/catch) and let the validator detect if parsing should have succeeded (e.g. non-json string doesn't match object schema)

The latter option feels better to me, as it would support both non-json+json strings for the same parameter.

I'm happy to take a swing on a PR for this if my suggested solution is agreeable.

@cdimascio
Copy link
Owner

@JacobLey, thanks for jumping on this. im on board with your approach. Will review the PR soon.

cdimascio added a commit that referenced this issue Dec 18, 2019
Only enforce query object parsing when specified, always validate #186
@cdimascio
Copy link
Owner

Thanks, @JacobLey. This is fixed in v3.2.3

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

2 participants