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

Deal with string/binary and string/byte top-level schemas #193

Open
honzajavorek opened this Issue Jul 24, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@honzajavorek
Copy link
Member

honzajavorek commented Jul 24, 2018

Swagger allows to specify request or response body schemas with type/format string/byte and string/binary. The intention of the spec writers is probably to allow describing pure binary and base64 payloads, but in the realm of JSON Schema, such description implies a valid JSON string (i.e. "blah blah") with its contents being of a particular format. Currently we treat Swagger schemas as JSON Schemas and there are compatibility issues caused by what format values are supported - #99 - but this is slightly different, because I think nobody really meant there should be payloads like this:

"âPNGIHDRdlú±9bKGDˇˇˇ†ΩßìpHYs..."

(It was supposed to be a beginning of a PNG image in binary, but I had to delete some bytes so GitHub wouldn't break the code block 😬)

Instead, the intention was probably that following Swagger describes a /binary route responding with some raw bytes in the body:

paths:
  /binary:
    get:
      responses:
        200:
          description: Representation
          examples:
            "application/octet-stream": "..."
          schema:
            type: string
            format: binary

Even if subsequent tooling ignores the format description, it is lead by the schema to assume the payload should be a valid JSON string.

The Swagger adapter should somehow deal with the situation, as it is Swagger-specific and ruins the agnosticism of the subsequent tooling. One approach, probably the easiest, would be to drop top level string/binary and string/bytes schemas and leave the body description without any assertions for the subsequent tooling. Nested schemas of such types/formats are okay, as the nested nature already implies a structure like JSON.

@honzajavorek

This comment has been minimized.

Copy link
Member

honzajavorek commented Jul 24, 2018

Just a note, this is not an issue in request bodies. Following works, for some reason:

paths:
  /image.png:
    put:
      parameters:
        - name: binary
          in: body
          required: true
          schema:
            type: string
            format: binary
@kylef

This comment has been minimized.

Copy link
Member

kylef commented Jul 24, 2018

I think there are two steps we should take to resolve this issue:

  • Do not provide a JSON schema for binary types in the parse result. JSON Schemas are for JSON and they do not make sense in binary cases.
  • Start creating warnings when a user tries to use binary in JSON types. You cannot embed binary inside a JSON structure so it should create a warning and be stripped.
@honzajavorek

This comment has been minimized.

Copy link
Member

honzajavorek commented Nov 28, 2018

@kylef Do I understand correctly that v0.19.1 fixes your first bullet point?

honzajavorek added a commit to apiaryio/dredd that referenced this issue Nov 29, 2018

fix: support 'binary' response schemas in OpenAPI 2
Requires fury-adapter-swagger@0.19.2, solves
apiaryio/fury-adapter-swagger#193 for Dredd

honzajavorek added a commit to apiaryio/dredd that referenced this issue Nov 29, 2018

fix(OpenAPI 2): support 'binary' response schemas
Requires fury-adapter-swagger@0.19.2, solves
apiaryio/fury-adapter-swagger#193 for Dredd

honzajavorek added a commit to apiaryio/dredd that referenced this issue Nov 29, 2018

fix(OpenAPI 2): support 'binary' response schemas
Requires fury-adapter-swagger@0.19.2, solves
apiaryio/fury-adapter-swagger#193 for Dredd

honzajavorek added a commit to apiaryio/dredd that referenced this issue Nov 29, 2018

fix(openapi2): support 'binary' response schemas
Requires fury-adapter-swagger@0.19.2, solves
apiaryio/fury-adapter-swagger#193 for Dredd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment