stoplightio / prism

Turn any OpenAPI2/3 and Postman Collection file into an API server with mocking, transformations and validations.
https://stoplight.io/open-source/prism
Apache License 2.0
4.32k stars 350 forks source link

OAS3 fails to generate text/plain response when no Accept header passed #500

Closed chris-miaskowski closed 5 years ago

chris-miaskowski commented 5 years ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce

====test====
Generate simple plain text response
====spec====
openapi: "3.0.1"
paths:
  "/path":
      get:
        responses:
          '200':
            content:
              text/plain:
                schema:
                  type: string
====server====
mock -p 4010
====command====
curl -i -H "Accept:" http://localhost:4010/path
====expect====
HTTP/1.1 200 OK
content-type: text/plain

string

Actual body equals to ""

Environment (remove any that are not applicable): v3.0.3

XVincentX commented 5 years ago

Yeah ok this is a bit articulated.

What happens is:

  1. The Accept: is not a valid media type
  2. Prism treats this as "no media type provided"
  3. When no media type is provided, it defaults to application/json
  4. No application/json is provided in this document
  5. Since no media type is provided, it returns an empty text/plain

It might make sense to look for a text/plain content in the operation before returning an empty one? //cc @philsturgeon

philsturgeon commented 5 years ago

The actual output is expected output. We default to JSON because this is primarily an API tool and most people are honestly rather likely to be using JSON. Defaulting to text/plain is not beneficial for this sort of tool. We only do it when a response status is defined but no schema or example exists so we have to return something.

XVincentX commented 5 years ago

All right!

taasan commented 5 years ago

I'm trying to mock a real-world proprietary API that returns only text/plain and run into this problem. It would be nice to have a command-line option or x-something-something option in swagger for default media type.

philsturgeon commented 5 years ago

Maybe a --default-content-type=text/plan switch, which defaults to application/json? This way our hardcoded choice to default to JSON can be replaced with a option that is in the control of the user. That would solve the use case for you @taasan, and help @chris-miaskowski with his example.

What do you think @XVincentX ?

pytlesk4 commented 5 years ago

Why not just pick one from the produces array? If JSON doesn't exist, then pick the first one or a random one?

XVincentX commented 5 years ago

I am against this flag. I've checked out the specs a bit and:

https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

A request without any Accept header field implies that the user agent will accept any media type in response.

Fundamentally no Accept === Accept: */*; we could normalise this somewhere in Prism and let it use the regular flow. In this case, we could also remove the application/json fallback (or we can keep it, I'd need to do some tests)

That said, I still think that doing explicitly Accept: mean "nothing is suitable for me" and so you get a 406, because whatever I'll send you, you wouldn't be ok with it.

I'm trying to mock a real-world proprietary API that returns only text/plain and run into this problem.

@taasan If your API is only returning text/plain all you need to do is explicitly put text/plain in the produces array (OAS2) or write your OAS3 document with text/plain content types.

Can you articulate a little bit with a minimal OpenAPI document?

taasan commented 5 years ago

swagger.yaml

openapi: 3.0.1
info:
  title: text/plain API
  version: 1.0.0
paths:
  /:
    get:
      summary: Send
      operationId: send
      responses:
        200:
          description: OK
          content:
            text/plain:
              schema:
                type: string
                example: "0\nOK"

HTTP request gives no output. RFC 2616 says:

If no Accept header field is present, then it is assumed that the client accepts all media types.

telnet localhost 4010
Trying ::1...
Connected to localhost.
Escape character is '^]'.
GET /

HTTP/1.1 200 OK
access-control-allow-origin: *
content-type: text/plain
content-length: 0
Date: Thu, 24 Oct 2019 12:15:47 GMT
Connection: close

Connection closed by foreign host.

Prism log

success The request passed the validation rules. Looking for the best response
note    No mediaType provided. Fallbacking to the default media type (application/json)
success Responding with the requested status code 200
XVincentX commented 5 years ago

@taasan yeah ok you're falling in the first case, where we should probably just take your Accept absence for a Accept: */* //cc @philsturgeon

philsturgeon commented 5 years ago

Don't look at 2616 it is old and dead.

If Accept: === */* in 723* then sure, that is fine, but what does it mean when Accept is missing? Is the lack of an Accept header the same as the lack of a value in the header?

Btw our "application/json fallback" is for when you have multiple responses types defined but are not specifying an Accept, because we here at Stoplight know that most people are using JSON, and we are doing folks a favour. This doesn't need to be in the HTTP spec, we are making a choice which is additive to the spec.

@pytlesk4 the downside to "if they dont specify anything just grab the first thing" is that when a 2nd thing is added, the thing they were doing before now does not work and that's not great. But, that's pretty much what Accept: */* means, so in any situation where / is explicitly or implicitly used we will do that.

pytlesk4 commented 5 years ago

@pytlesk4 the downside to "if they dont specify anything just grab the first thing" is that when a 2nd thing is added, the thing they were doing before now does not work and that's not great. But, that's pretty much what Accept: */* means, so in any situation where / is explicitly or implicitly used we will do that.

@philsturgeon I understand that I just think its better than failing, that is a pretty technical thing to understand. But that is my opinion. I am thinking of this from a user perspective, prism is failing when something is defined, it is potentially confusing.

Sounds like a solution is reached though, so that is awesome :).

taasan commented 5 years ago

From RFC 7231

A request without any Accept header field implies that the user agent will accept any media type in response.

XVincentX commented 5 years ago

@philsturgeon So what do we do here?

Are we ok that the lack of Accept === Accept: */* and Accept: === 406? If so, we can easily tame this since it's almost a single line of code.

philsturgeon commented 5 years ago

@XVincentX it seems like we collectively reached the conclusion that no Accept, and Accept: are the same thing? Let's do that.

XVincentX commented 5 years ago

Ok — we'll treat both cases as "the client accepts / ".