redocly-cli icon indicating copy to clipboard operation
redocly-cli copied to clipboard

`allOf` in oas3 is handled incorrectly under Windows making examples not fail the mediatype validation

Open CommCody opened this issue 4 years ago • 7 comments

Describe the bug

allOf in oas3 is handled incorrectly making examples fail the validation

To Reproduce Steps to reproduce the behavior:

  1. Given this .redocly.yaml file
lint:
  extends:
    - recommended
  rules:
    info-contact: off
  1. And this OpenAPI file: (openapi.yaml)
openapi: "3.0.3"
info:
  title: bugtest
  version: "1.0"
  description: Demo
  license:
    name: DEMO
    url: https://demo.com
servers:
  - url: http://demo.com/api

components:
  examples:
    DemoResponse:
      description: Demo
      value:
        name: "demo"
        demo: "Demo"

  schemas:
    Demo:
      type: object
      required:
        - name
        - demo
      properties:
        name:
          type: string
          maxLength: 100
        demo:
          type: string

    DemoAllOf:
      allOf:
      - type: object
        required:
          - name
        properties:
          name:
            type: string
            maxLength: 100
      - type: object
        required:
          - demo
        properties:
          demo:
            type: string

paths:
  /demo:
    summary: "Demo with schema NO allOf and NO baseRef"
    get:
      summary: "Get demo no refs"
      operationId: getDemoNoRefs
      description: "Returns Demo No Refs"
      responses:
        200:
          description: Demo No Refs
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/Demo"
              examples:
                Demo1:
                  $ref: "#/components/examples/DemoResponse"

  /demoAllOf:
    summary: " Demo with schema allOf and NO baseRef"
    get:
      summary: "Get demo no refs"
      operationId: getDemoAllOf
      description: "Returns Demo refs with baseRef"
      responses:
        200:
          description: Demo refs with baseRef
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/DemoAllOf"
              examples:
                Demo:
                  $ref: "#/components/examples/DemoResponse"
  1. Run this command: npx @redocly/openapi-cli lint openapi.yaml
  2. See error On Windows 10: NONE On github ubuntu-latest: warnings from rule: no-invalid-media-type-examples because allOf is not correctly handled

Expected behavior No warnings should be generated on ubuntu-latest or WSL 2 with Ubuntu 20.04 (other linux versions not tested)

Logs If applicable, add logs to help explain your problem.

OpenAPI definition If applicable, add an OpenAPI definition and .redocly.yaml configuration file that helps reproduce the problem. At a minimum, please state the specification version(s) you're using (e.g. 2.0, 3.0, 3.1).

openapi-cli Version(s) What version of openapi-cli are you using? latest (with npx)

Node.js Version(s) What version of node.js are you using? Latest LTS Version: 14.15.1 (includes npm 6.14.8)

Additional context Add any other context about the problem here.

CommCody avatar Nov 28 '20 08:11 CommCody

@CommCody in this rule, we decided to disallow additional properties by default (like if each schema had additionalProperties: false). This is a very common mistake people make (do not set additionalProperties: false).

You can configure this behaviour in the .redocly.yaml:

lint:
  rules:
    no-invalid-media-type-examples:
      severity: warn
      disallowAdditionalProperties: false

See: https://redoc.ly/docs/cli/built-in-rules/#no-invalid-media-type-examples

It is strange it does not show any warnings in Windows. We have to check this.

RomanHotsiy avatar Nov 28 '20 08:11 RomanHotsiy

But in my opinion are both schemas Demo and DemoAllOf the same, so I don't understand which properties are considered to be additional? Because for DemoAllOf both properties (one from each constituent part) are considered "additional" which is VERY strange, since in oas3 I cannot define anything else next to allOf

CommCody avatar Nov 28 '20 09:11 CommCody

Hi @RomanHotsiy thanks for the quick response. I've done some further investigation into allOf, Combining schemas - allOf at json-schema.org was particularly enlightening. I now better understand why your validator gives errors given your approach. It seems like my biggest misunderstanding was regarding the fact that the component schema of the allOf don't merge and are not considered as a whole. I had expected this to work more in a mixin like fashion.

PS: After rereading in the oas specs, I can't actually find a requirement saying that it is illegal to have further declarations next to allOf in a schema definition (so I've hidden my previous comment as it is irrelevant)

CommCody avatar Nov 28 '20 19:11 CommCody

Sorry not Closing:

  1. The difference between windows 10 & wsl2 ubuntu is a bug
  2. There is also weird behaviour in the transitivity of additionalProperties:

This gives one warning:

DemoAllOf:
      allOf:
      - type: object
        required:
          - name
        properties:
          name:
            type: string
            maxLength: 100
        additionalProperties: true
      required:
        - demo
      properties:
        demo:
          type: string
      additionalProperties: false

This gives two warnings:

    DemoAllOf:
      allOf:
      - type: object
        required:
          - name
        properties:
          name:
            type: string
            maxLength: 100
        additionalProperties: true
      - type: object
        required:
          - demo
        properties:
          demo:
            type: string
        additionalProperties: true
      additionalProperties: false

CommCody avatar Nov 28 '20 19:11 CommCody

Can you show the difference in the output of wsl2 Ubuntu and windows 10?

I don’t understand what you mean by the transitivity of additionalProperties?

adamaltman avatar Dec 02 '20 05:12 adamaltman

@CommCody is the problem with windows10 still relevant. I was testing the given definition on my windows 10 machine and I 'm getting 2 warnings

windows10

slavikbez avatar Oct 30 '21 11:10 slavikbez

@CommCody this is not just a "Windows" problem, it appears that validator incorrectly handles "allOf" cases

slavikbez avatar Nov 01 '21 11:11 slavikbez

Confirm, reproduces for me.

chaporgin avatar Dec 23 '22 14:12 chaporgin

@antonyc which version of Redocly CLI are you using? Could you also provide a minimal reproducible example?

tatomyr avatar Dec 28 '22 10:12 tatomyr

does not reproduce on the newest versions, thank you!

chaporgin avatar Apr 17 '23 14:04 chaporgin

Closing this as fixed then. Feel free to reopen if I'm missing something.

tatomyr avatar Apr 20 '23 09:04 tatomyr