OAI / OpenAPI-Specification

The OpenAPI Specification Repository
https://openapis.org
Apache License 2.0
28.82k stars 9.07k forks source link

[oas3] Default 'explode' for cookie parameters? #1528

Closed jwalton closed 3 months ago

jwalton commented 6 years ago

According to the spec, "When style is form, the default value [for explode] is true. For all other styles, the default value is false." However, according to this documentation, in the "Cookie Parameters" section, there are "*"s indicating the default serialization, and it is style: form, explode: false.

So which is it? Do cookies use explode by default or not?

hkosova commented 6 years ago

The Specification is the source of truth - so the cookies are not exploded by default.

I've fixed the swagger.io doc. If you notice any other info on the Swagger website that contradicts the info in the OpenAPI Specification, please file an issue in the https://github.com/swagger-api/swagger.io repository. Thanks!

jwalton commented 6 years ago

What does explode even mean for a cookie, though? If I have a list in a cookie "foo", does this mean I have a Cookie header with multiple copies of the cookie?

Cookie: foo=1; foo=2

RFC 6265 doesn't disallow this, and some clients will even send multiple copies of the same cookie in certain weird circumstances (if you set a cookie "foo" with the domain "*.myserver.com", and then set a second cookie "foo" with the domain "baz.myserver.com"). But... That'd be a weird way to pass parameters around. :P

jwalton commented 6 years ago

And, cookies default to "form", but they aren't really "form". An array encoded in "form" with explode: true is encoded as: 'color=blue&color=black&color=brown' (this is the example from the spec) but if you tried to literally set the cookie header to:

Cookie: color=blue&color=black&color=brown

A cookie parser would turn that into {color: "blue&color=black&color=brown"}, which is... ugh.

webron commented 6 years ago

Does this discourage you from using cookies as parameters?

jwalton commented 6 years ago

Using cookies as parameters is weird, and I know there's a history of debate about whether to include them in the spec or not in the first place. I'm not using cookies as parameters; I'm writing an implementation of the spec. If someone is using my implementation, they expect it to work correctly, even if they are using cookie parameters. So I need to work out what "correctly" is.

If we compare the table for cookies and headers, you can see they are very nearly identical, except that "header" uses style: simple and cookie uses style: form. But simple and form aren't anything like each other.

As far as I can tell, to correctly implement the spec, if the cookie parameter is {style: form, explode: false}, then I want to parse the cookie with a standard cookie parser, and treat the value as 'simple' encoded. If the cookie parameter is {style: form, explode: true} (which is the default for cookie parameters) then I want to treat it the same as if explode is false for the non-array/object case, and I have no idea what to do in the array/object case.

So, in short, treat cookie parameters like simple, even though they are from and don't even support simple in the first place.

This seems like an error in the spec; cookie parameters should only support 'simple', not only support 'form', and then "exploded" cookies make sense, just like they do for header parameters.

webron commented 6 years ago

Not quite. In simple the name of the parameter isn't included in the 'rendered' version, making it suitable for path parameters where the parameter name is not needed (it's based on location) or header parameters, where the name of the header itself is the parameter name.

In form on the other hand, the name of the parameter is included. That makes it suitable for query parameter where the name is used as the identifier, and, well, cookie parameters as there is technically support for multiple cookie parameters, but there can be only one cookie header.

I get the potential issue with parsing the cookie 'naturally', however, using cookies to transfer such information is not ideal anyway. If you have proposals for improvement, we'd love to hear.

jwalton commented 6 years ago

Well... it depends on what you count as the "rendered" version. If you have a header pameter, for example:

X-MyHeader: 5

The "name" of the parameter is right there; it's "X-MyHeader", so I could argue that this is really form, right? Semantically, that's not right, because the "value" of this header is "5", and that's the bit we want to parse.

With a cookie header, I'd make the same argument though:

Cookie: id=5; session=blahblahblah

Even though "id" appears in this cookie header, I would argue that semantically there is an "id" cookie, and it's value is 5, just like in the "header" case.

I can't just pass the contents of the Cookie header through a 'form' parser, because there's a "; " in there that's going to confuse the form parser (or is going to give me an "id" of "5; session=blahblahblah", since there's no & separating the arguments like there should be in 'form' style). So first I need to run this through a cookie parser, and I get out something that looks like:

{
    id: '5',
    session: 'blahblahblah'
}

So at this point I have the value '5' and I need to parse it. I can't pass this through a 'form' parser either. But I can pass it through a 'simple' parser.

If you encode an {x, y} object in form style it would be x=7&y=hello. So if I had a cookie parameter named "myParam", and it was an object, and I was using form style, I'd expect the encoding to be:

Cookie: myParam=x=7&y=hello

But in order for that to be the case, I have to treat the value of the cookie as the thing that is encoded with form style. Continuing this train of thought to it's logical conclusion, if I have a 'myParam' parameter of type 'integer' and style 'form', I would expect the cookie header to be:

Cookie: myParam=myParam=7

The spec itself has no examples of how to encode a cookie, so going strictly by the spec, this is what I would expect. This doesn't look anything like the example given on swagger.io though.

If I was writing the spec from scratch with no concern for backwards compatibility, I'd say "Cookie parameters only support the "simple" style. To encode a value, use simple encoding rules, and put the result in the cookie value."

(Edited: I just realized that parsing my cookie header example with the form parser wouldn't generate an id of "5; " but "5; session=blahblahblah").

webron commented 6 years ago

Thanks for the elaborate explanation, it helps clarifying a few things.

Before diving into the explanation, I'll say this - the documentation on swagger.io is auxiliary to the spec, and can contain errors. It is not endorsed by the OAI, and I say that as one of the people behind the Swagger project. You can point out errors in documentation there, but you can't use it as a filler to missing spec documentation.

I'm beginning to understand what you're getting at, however, you have one wrong assumption. You have the following part in your response:

If you encode an {x, y} object in form style it would be x=7&y=hello. So if I had a cookie parameter named "myParam", and it was an object, and I was using form style, I'd expect the encoding to be:

Cookie: myParam=x=7&y=hello

That is actually wrong. I assume in this case you mean that explode: true (otherwise there wouldn't be & there). That would theoretically result in:

Cookie: x=7&y=hello

Before you jump - yes, the parameter name disappears (as it does in query parameters), and yes, it's pretty much broken when it comes to cookies.

Now comes the question - what gives? The answer is... (and this is also the answer as to why there are no examples for everything) - we never really thought anyone would go through the trouble of describing objects in cookies in an exploded way (and let's put aside the why for the moment).

But that's not all - you brought up a fairly important point. It might make more sense to use simple as the style for cookies, with the parameter name being the cookie name and so on. There's a potential issue with encoding (not familiar with the cookie RFC to tell at the moment) but that one can be solved. Though as you said, this is a bit too late right now, because it is a breaking change. This is why we had a long review time, asking tool implementers to give it a spin and give us feedback, but alas, nothing came through. Not blaming you or anyone for this, of course - we just chose the wrong process for these kind of changes, and we're learning from our mistakes.

As for the next steps - looking to hear from others in the community and of course, the @OAI/tsc. If there's general agreement that this should change, we can decide how and when.

jwalton commented 6 years ago

If we're using the encoding as described on swagger.io, then yes, and explode is true, the parameter name disappears and it becomes:

Cookie: x=7&y=hello

I mean, first when I run this through a cookie parser I get {x: '7&y=hello'}, and there's no parser in the world that's going to make sense of that. :P

But I think, by the letter of the spec, this is wrong. The spec doesn't say the cookie header is encoded as form data. The spec says if the parameter location is "cookie", this is "used to pass a specific cookie value to the API". So if I have:

parameter:
  in: cookie
  name: 'myParam'
  style: 'form'
  explode: true
  schema:
    type: object

So to encode this, I take my object {x:7, y: 'hello'}, I form encode it to get x=7&y=hello, then I put this in the value of cookie named "myParam", and I get a cookie header:

Cookie: myParam=x=7&y=hello

Similarly, for:

parameter:
  in: cookie
  name: 'myParam'
  style: 'form'
  schema:
    type: number

I encode my parameter using form to get "myParam=7", then I put this in a cookie named "myParam", and I get:

Cookie: myParam=myParam=7

So maybe the best thing to do here is to release an OAS 3.1.0 which adds "simple" to the list of allowed cookie styles, update the docs on swagger.io to show the above as the "form" encoding for cookies (because that's literally what the spec says) and then add some "simple" examples, too.

This leaves the default for cookies as {style: 'form', explode: 'true'}, which is a bit weird because probably no one wants that, but it works.

jwalton commented 6 years ago

Or, to put this more succinctly: RFC 6265 says a cookie is:

cookie-header = "Cookie:" OWS cookie-string OWS
cookie-string = cookie-pair *( ";" SP cookie-pair )
cookie-pair = cookie-name "=" cookie-value

Or, less formally:

Cookie: [cookie-name]=[cookie-value]; [cookie-name]=[cookie-value];...

OAS 3.0.1 says:

"cookie" - used to pass a specific cookie value to the API

So the value part of the cookie, according to OAS3, has to be form encoded. This example from the swagger site:

Cookie: id=5

Is non-conformant, because the cookie-value is the string "5" which is not a form encoded string.

(Edited: hit 'shift-enter' and sent too soon. :P)

webron commented 6 years ago

Not sure what you're referring to as 'form encoding' - it sounds like you're referring to application/x-www-form-urlencoded but that's not the case here. The form style actually refers to https://tools.ietf.org/html/rfc6570#section-3.2.8 - but I agree that with the current wording, assigning it to cookie doesn't make sense and for the current 3.0.X versions it needs to be clarified.

jwalton commented 6 years ago

Yeah, by "a form encoded string", I really meant "a RFC 6570 form-style query expansion". :)

Although, now that I go back an read RFC 6570, in order to be complaint the correct example would actually be:

Cookie id=?id=5

Because the cookie-value needs to start with a "?" to meet RFC 6570.

webron commented 6 years ago

Yup, that's what I referred to as the problem with the current wording.

hkosova commented 6 years ago

Related request: https://github.com/OAI/learn.openapis.org/issues/100 "More and complete examples for serialization styles"

richardwhiuk commented 3 years ago

Similarly, for:

parameter:
  in: cookie
  name: 'myParam'
  style: 'form'
  schema:
    type: number

I encode my parameter using form to get "myParam=7", then I put this in a cookie named "myParam", and I get:

Cookie: myParam=myParam=7

I think this is wrong - I think it would just be:

Cookie: myParam=7

I think to get Cookie: myParam=myParam=7 you would need a spec like:

parameter:
  in: cookie
  name: 'myParam'
  style: 'form'
  schema:
    type: object
    properties:
      myParam:
         type: integer
spacether commented 2 years ago

My preference would also be to have cookie values use style simple serialization. Also we should specify the charsets that are allowed for cookie keys and values. It looks like commas, spaces and equal signs must be uri percent encoded.

rafalkrupinski commented 8 months ago

Should perhaps only string values be accepted?

https://datatracker.ietf.org/doc/html/rfc6265#section-4.2.2

The semantics of individual cookies in the Cookie header are not defined by this document. Servers are expected to imbue these cookies with application-specific semantics.

handrews commented 4 months ago

The way to get around the automatic RFC6570 percent-encoding is to use the content field instead of schema. In that case, style does not matter. You'll need to use a media type (e.g. text/plain) that will work, and follow the cookie RFC's guidance of using base64 encoding for data outside the allowed character set (contentEncoding: base64 in 3.1, format: byte in 3.0).

From what I see here, most of the schema/style/explode options don't work for cookies beyond very simple cases, and we should note that in the specification so folks don't get frustrated trying to align them with the cookie RFC somehow.

handrews commented 3 months ago

PR merged for 3.0.4 and ported to 3.1.1 via PR #3921! This is addressed by the new Appendix D.