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

Validation Parameters, vague errors that have regressed ( I believe ) #1511

Closed
ponelat opened this issue Sep 27, 2017 · 6 comments · Fixed by #1985
Closed

Validation Parameters, vague errors that have regressed ( I believe ) #1511

ponelat opened this issue Sep 27, 2017 · 6 comments · Fixed by #1985

Comments

@ponelat
Copy link
Member

ponelat commented Sep 27, 2017

Q A
Bug or feature request? Bug ( regression )
Which Swagger/OpenAPI version? 2.0
Which Swagger-Editor version? swaggerEditor/Ui: "3.1.9/g95b3f38a-dirty" / version: "3.2.2" / gitRevision: "g774a066"
How did you install Swagger-Editor? Custom + Using Online version
Which broswer & version? Chrome/59
Which operating system? Linux Mint 18

Demonstration API definition

swagger: '2.0'
info:
  version: '1'
  title: oneOf validation fail
  description: ok

paths:
  /models/{name}:
    get:
      summary: Extra `example` field in parameter
      description: ok
      parameters: 
      - in: path
        name: name
        type: string
        required: true
        example: ok
      responses:
        200:
          description: Ok

Expected Behavior

Validation should show that the "example" key is not allowed in "path" parameters.

Current Behavior

Two validation errors occur...

should NOT have additional properties
additionalProperty: type, example, in, name, required
Jump to line 13
Schema error at paths['/models/{name}'].get.parameters[0].in
should be equal to one of the allowed values
allowedValues: body, header, formData, query
Jump to line 13

Possible Solution

Hack the structural validation lib, to better handle oneOf cases?
Ie: in: path then enforce validation for path items.
Or catch the strings ( this was done before and has regressed ) and transform the error message.
And perhaps lastly, add custom validation to the cases where oneOf appear, and remove the bad structural messages.

Context ( as a user )

Frustration on how to fix the spec, given the vague somewhat incorrect validation messages.

@webron
Copy link
Contributor

webron commented Sep 27, 2017

Yeah, that's a regression caused by changing the JSON Schema validator. We really need to address this.

@shockey shockey self-assigned this Sep 27, 2017
@shockey shockey added this to the Friday, November 10 milestone Nov 3, 2017
@shockey shockey removed this from the Friday, November 10 milestone Nov 17, 2017
@freegnu
Copy link

freegnu commented Nov 27, 2017

adding required: true to the parameter definition fixes the problem. Removing it from any path parameter definition causes the error. Maybe a better default for path parameters?

@shockey
Copy link
Contributor

shockey commented Nov 27, 2017

@freegnu that would be nice, but would require a change to the OpenAPI specification!

It currently reads:

Parameter.required

Determines whether this parameter is mandatory. If the parameter location is "path", this property is REQUIRED and its value MUST be true. Otherwise, the property MAY be included and its default value is false.

If we just transparently added required: true in the UI/Editor, it would be out of line with the specification and confusing to end-users. It may also break other Swagger/OpenAPI tools that don't transparently add required: true.

That said, I don't think it'd be a terrible idea to change the specification to something like if the parameter location is "path", this property's default value is true, and MAY explicitly be set to true.

Feel free to bring it up with the OpenAPI folks here: https://github.com/OAI/OpenAPI-Specification/issues

@bmschwa
Copy link

bmschwa commented Nov 23, 2018

Anyone still seeing this? I'm having the same problem today (1 year after the last entry).... Or have an example of a way to work around it? for us swagger Novices, a oneOf workaround example would be helpful

@dsandor
Copy link

dsandor commented Dec 20, 2018

@bmschwa Hi Brian, I just came across this issue today and found a solution. It seems much of the examples I see ask you to specify the parameters in such a way that does not pass validation. Here is a working example that got me past this issue.

FWIW, I really wish the validation failures provided links to working examples. This would mitigate a lot of frustration because there is so much incorrect or no longer valid examples online.

/my/path/{id}:
  get:
    parameters:
    -name: id
      in: path
      required: true
      type: string

. . .

For me the change was to specify type: string instead of the long form where I was specifying the schema for the type. I think that is what threw off validation for me.

The documentation here: https://swagger.io/docs/specification/describing-parameters/ tells us to describe the path parameters with schema which does not work in the editor. You must (as of today) specify the type on a single line in short form e.g. type: integer or type: string`.

@shockey
Copy link
Contributor

shockey commented Apr 12, 2019

I've opened a pull request (#1985) that will close this issue.

Here's what Swagger Editor reports with my changes:

Structural error at […]parameters.0
should NOT have additional properties
additionalProperty: example

shockey added a commit to shockey/swagger-editor that referenced this issue Apr 17, 2019
shockey added a commit to shockey/swagger-editor that referenced this issue Apr 17, 2019
shockey added a commit that referenced this issue Apr 18, 2019
* adopt @webron's OpenAPI 3.0 schema from OAI/OpenAPI-Specification#1270

permalink: https://github.com/OAI/OpenAPI-Specification/blob/92e15eba1d4591ebfe8c11898c48241e72854381/schemas/v3.0/schema.yaml

* add ajv-errors

* address error messages for #1808's Swagger 2.0 example

clarifies the schema and adds custom error messages for unclear error conditions

* address error messages for #1808's OpenAPI 3.0 example

* restrict underlying JSON Schema `type` field to simple types only (for #1832)

* fix limitation in JSON Pointer conversion helper

* add clear `not` error message (for #1489)

* add additionalProperties message (for #1394)

* add ajv-keywords

* use `switch` to intelligently identify inline vs referenced content (for #1853)

* use `switch` to XOR `schema` and `content` (for #1853)

* use `switch` to pivot security scheme based on type

(for #1672)

* use switch to fall-through to inline security scheme validation (for #1672)

* rewrite more Reference oneOfs (for #1519)

* add custom message for `Schema.required` type error (for #1519)

* rewrite Response/Reference oneOf (for #1489)

* use switch in ParameterLocation validation (for #1797)

* define pivot key switches for SecurityDefinitions (for #1711)

* give helpful `format: uri` messages for SecurityDefinitions (for #1711)

* eliminate NonBodyParameter; pivot on `Parameter.in` with a switch (for #1511)

* oneOf -> switch for Parameters.items reference

* (for #1711)

* remove redundant semantic validator (for #1511)

* adjust wording of custom error message (for #1853)

* add regression tests for all related issues

* revert to expect@^1.20.2

* linter fixes

* fix messaging flaw for #1832

* improve messaging for #1394

* use literal key for `$ref` in Reference Object

* remove commented legacy data from OAS3 schema

* remove superfluous quotation marks

* normalize test case paths to `/`

* normalize openapi fields to 3.0.0

* drop unused `paths` information

* ensure clear errors for 3.0 Parameter style/content exclusivity

* add `required` assertions to switch statements that pivot on a key's value

this prevents false positives when the pivot key is missing entirely

* remove stray space
shockey added a commit to shockey/swagger-editor that referenced this issue May 23, 2019
…i#1985)

* adopt @webron's OpenAPI 3.0 schema from OAI/OpenAPI-Specification#1270

permalink: https://github.com/OAI/OpenAPI-Specification/blob/92e15eba1d4591ebfe8c11898c48241e72854381/schemas/v3.0/schema.yaml

* add ajv-errors

* address error messages for swagger-api#1808's Swagger 2.0 example

clarifies the schema and adds custom error messages for unclear error conditions

* address error messages for swagger-api#1808's OpenAPI 3.0 example

* restrict underlying JSON Schema `type` field to simple types only (for swagger-api#1832)

* fix limitation in JSON Pointer conversion helper

* add clear `not` error message (for swagger-api#1489)

* add additionalProperties message (for swagger-api#1394)

* add ajv-keywords

* use `switch` to intelligently identify inline vs referenced content (for swagger-api#1853)

* use `switch` to XOR `schema` and `content` (for swagger-api#1853)

* use `switch` to pivot security scheme based on type

(for swagger-api#1672)

* use switch to fall-through to inline security scheme validation (for swagger-api#1672)

* rewrite more Reference oneOfs (for swagger-api#1519)

* add custom message for `Schema.required` type error (for swagger-api#1519)

* rewrite Response/Reference oneOf (for swagger-api#1489)

* use switch in ParameterLocation validation (for swagger-api#1797)

* define pivot key switches for SecurityDefinitions (for swagger-api#1711)

* give helpful `format: uri` messages for SecurityDefinitions (for swagger-api#1711)

* eliminate NonBodyParameter; pivot on `Parameter.in` with a switch (for swagger-api#1511)

* oneOf -> switch for Parameters.items reference

* (for swagger-api#1711)

* remove redundant semantic validator (for swagger-api#1511)

* adjust wording of custom error message (for swagger-api#1853)

* add regression tests for all related issues

* revert to expect@^1.20.2

* linter fixes

* fix messaging flaw for swagger-api#1832

* improve messaging for swagger-api#1394

* use literal key for `$ref` in Reference Object

* remove commented legacy data from OAS3 schema

* remove superfluous quotation marks

* normalize test case paths to `/`

* normalize openapi fields to 3.0.0

* drop unused `paths` information

* ensure clear errors for 3.0 Parameter style/content exclusivity

* add `required` assertions to switch statements that pivot on a key's value

this prevents false positives when the pivot key is missing entirely

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

Successfully merging a pull request may close this issue.

6 participants