Skip to content

v3.2: Provide guidance for the Set-Cookie response header #4748

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

Merged
merged 7 commits into from
Jul 24, 2025

Conversation

handrews
Copy link
Member

@handrews handrews commented Jun 24, 2025

Partially addresses issue #1237

The Set-Cookie response header breaks the normal rules for headers with multiple values and requires special handling.

There are two options here:

  1. Rework how we handle headers to accommodate Set-Cookie in a consistent manner
  2. Treat Set-Cookie as a special case and define how it relates to normal behavior

Since RFC9110 §5.3 advises clients to treat Set-Cookie as a special case, I went with special-casing it. This assumes the approach to header serialization that is described in PR #4648 and discussed in one of its comment threads (specifically, that it does not include the header name which is required for it to be consistent with how style: "simple" and explode are treated elsewhere and are defined in RFC6570).

  • schema changes are included in this pull request
  • schema changes are needed for this pull request but not done yet
  • no schema changes are needed for this pull request

@handrews handrews added this to the v3.2.0 milestone Jun 24, 2025
@handrews handrews added media and encoding Issues regarding media type support and how to encode data (outside of query/path params) headers labels Jun 24, 2025
@handrews handrews requested review from a team as code owners June 24, 2025 00:25
@handrews
Copy link
Member Author

handrews commented Jul 3, 2025

With apologies to @philsturgeon for clearing his approval (its' set to automatically do that if anything changes) I realized we don't needallowReserved: true in that example (an earlier version I tried out did, and it was left over from that). And while I was at it I added a paragraph emphasizing that this only really matters if you need an intermedia multi-value single-string representation. If you just have an array of values and style: simple to map them to HTTP, they end results are the same as for other headers that support multiple values. Set-Cookie is only strange in that you cannot put multiple values on a single line.

@handrews
Copy link
Member Author

handrews commented Jul 4, 2025

OK apparently I was more out-of-it yesterday with this cold than I thought. Now I have added the commit that does things I mentioned in the last comment! 🤦

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm mostly in favour, with some minor editorial comments. Do we need to wait for the dataValue/serializedValue elements to resolve though? I see them in an example here.

@handrews
Copy link
Member Author

@lornajane I have rewritten this to (hopefully) focus more on examples and less on explanations. This should also go well with PR #4800 which defines style example behavior in terms of whether the name is included in the example or not, and explains why it is for forms and is not for headers.

Meh... now there are conflicts, though, I will rebase and force-push, but it will be the same commits. It's probably best to just look at it fresh anyway as the rewrite commit on its own is noisier than just looking at it as one thing.

handrews and others added 5 commits July 18, 2025 16:53
The Set-Cookie response header breaks the normal rules for
headers with multiple values and requires special handling.
Co-authored-by: Phil Sturgeon <[email protected]>
Also remove a stray line from an example that didn't really hurt
but wasn't needed and could have been confusing.
Co-authored-by: Lorna Jane Mitchell <[email protected]>
Explain when Set-Cookie workaround is needed

Also remove a stray line from an example that didn't really hurt
but wasn't needed and could have been confusing.
@lornajane
Copy link
Contributor

I commented this somewhere else too: I don't think we have the serializedValue PR merged yet and I see this one uses the feature (and it's a good use of it!) so let's get that one in before we approve this one.

@ralfhandl ralfhandl requested review from a team July 20, 2025 15:26
Copy link
Contributor

@ralfhandl ralfhandl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 with one nit and two questions

Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@lornajane lornajane dismissed ralfhandl’s stale review July 24, 2025 16:53

Changes were made

@lornajane lornajane merged commit 774f70c into OAI:v3.2-dev Jul 24, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
headers media and encoding Issues regarding media type support and how to encode data (outside of query/path params)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants