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.

Sign up for GitHub

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

Jump to bottom

Falsely Identifying Strings as Octals #936

Closed
raworre opened this issue Jan 20, 2020 · 4 comments · Fixed by #1050 · May be fixed by stoplightio/yaml-ast-parser#2
Closed

Falsely Identifying Strings as Octals #936

raworre opened this issue Jan 20, 2020 · 4 comments · Fixed by #1050 · May be fixed by stoplightio/yaml-ast-parser#2
Assignees
@P0lip
Labels
documentation t/bug Something isn't working

Comments

@raworre
Copy link

raworre commented Jan 20, 2020

Falsely Identifying Strings as Octals

Issue Description

When linting OpenAPI 3.x.x specifications, Spectral lint is not appropriately
parsing strings per the YAML specification. This issue seems to present itself
when strings that are made up of numbers that begin with the digit 0, and
contain the digits 8 or 9, are not quoted in the OpenAPI specification.

Since these are not valid octal strings, Spectral should be parsing these as
the YAML !!str type as opposed to the
!!int type.

Steps to Reproduce

openapi: 3.0.2
tags:
  - name: Test
    description: Test tag
info:
  title: Test API
  version: '0.0.1'
  description: An API for testing
  contact:
    name: Test Contact
servers:
  - url: https://foo-bar-dev.nonprod.jbhunt.com/test
paths:
  /things:
    get:
      operationId: get-things
      description: Fetches some things
      tags: [Test]
      responses:
        '200':
          description: Fetched some things
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/ThingList'
components:
  schemas:
    ThingList:
      type: object
      properties:
        things:
          type: array
          items:
            type: string
      example:
        things:
          - '01'
          - '02'
          - '03'
          - '04'
          - '05'
          - '06'
          - '07'
          - 08
          - 09
          - '010'
          - 01981

When linting the above OpenAPI specification, we get the following error.

OpenAPI 3.x detected

c:/Source/enterprise-apis/apis/test/test.yaml
 35:15  warning  example-value-or-externalValue     Example should have either a `value` or `externalValue` field.
 44:13    error  oas3-valid-content-schema-example  `7` property type should be string
 44:13    error  oas3-valid-schema-example          `7` property type should be string

✖ 3 problems (2 errors, 1 warning, 0 infos, 0 hints)

In the example above, the error at location 44:13 references the array example
item 08.
Spectral seem to be incorrectly interpereting these values as octals rather than
strings.

Expected Behavior

Per the YAML specification, the given example should be valid.

Environment

  • Spectral Version: 5.0.0
  • Windows 10
    • Version: 1909
    • Build: 18363.592
  • Node Version: 10.16.2
  • NPM Version: 6.13.6
@raworre raworre added the t/bug Something isn't working label Jan 20, 2020
@nulltoken
Copy link
Contributor

@raworre Indeed, this seems strange to me as well.

The doco states Please note that by default Spectral supports YAML 1.2 with merge keys extension..

And YAML specs are pretty clear with regards to expected formats:

YAML 1.2 tags (https://yaml.org/spec/1.2/spec.html#id2761292) vs YAML 1.1 tags (https://yaml.org/spec/1.1/#id858600)

And the regexp to validate them

  • YAML 1.2 integer (0 | -? [1-9] [0-9]*)
  • YAML 1.2 octal 0o [0-7]+

@P0lip any idea?

@P0lip
Copy link
Contributor

P0lip commented Jan 21, 2020 •
edited
Loading

@nulltoken yeah, seems indeed to be a bug. On it

EDIT:
We resolve tags according to "Core Schema" rather than "JSON Schema".
We'll want to change it JSON Schema.
See https://yaml.org/spec/1.2/spec.html#id2804923 - the above case is perfectly valid in case of Core Schema

@P0lip P0lip self-assigned this Jan 21, 2020
@P0lip P0lip mentioned this issue Jan 21, 2020
Open
@nulltoken
Copy link
Contributor

nulltoken commented Jan 21, 2020 •
edited
Loading

We resolve tags according to "Core Schema"

@P0lip Dammit! You're right!

Maybe should the doco be updated to reflect that (along with links to the proper parts of the YAML 1.2 specs)?

@P0lip
Copy link
Contributor

P0lip commented Jan 21, 2020

Maybe should the doco be updated to reflect that (along with links to the proper parts of the YAML 1.2 specs)?

Yeah, that's indeed a good idea.
That being said, I'd be still open to switching from Core Schema to JSON Schema, but doing so would result in a higher amount of parsing errors.

@nulltoken nulltoken mentioned this issue Apr 1, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@P0lip P0lip

Labels
documentation t/bug Something isn't working
Projects
None yet
Milestone
No milestone
3 participants
@nulltoken @raworre @P0lip