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.

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

Multiple Cookie: headers vs RFC 9113 #2541

Closed
bagder opened this issue May 17, 2023 · 18 comments · Fixed by #2753
Closed

Multiple Cookie: headers vs RFC 9113 #2541

bagder opened this issue May 17, 2023 · 18 comments · Fixed by #2753
Labels

Comments

@bagder
Copy link

bagder commented May 17, 2023

In the original 6265 as well as in the latest 6265bis draft, this paragraph exists:

When the user agent generates an HTTP request, the user agent MUST NOT attach more than one Cookie header field.

This is directly contradicted by RFC 7540 and then RFC 9113 section 8.2.3:

To allow for better compression efficiency, the Cookie header field MAY be split into separate header fields, each with one or more cookie-pairs

I personally suspect that trying to split the header up into multiples is a recipe for disaster and interop problems even if done only over HTTP/2, but maybe I'm just a pessimist.

Would it be sensible to maybe address this contradiction (better) in the bis document?

@bagder bagder changed the title Multiple Cookie: headers vs RFC 7540 Multiple Cookie: headers vs RFC 9113 May 17, 2023
@bagder
Copy link
Author

bagder commented May 17, 2023

HTTP/3, RFC 9114 section 4.2.1 repeats the HTTP/2 statement as well:

To allow for better compression efficiency, the Cookie header field MAY be split into separate header fields, each with one or more cookie-pairs

Both these HTTP RFCs also clarify that:

If there are multiple Cookie header fields after decompression, these MUST be concatenated into a single octet string using the two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ") before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application.

But still, that is for the receiving end and does not really restrict the sender when using HTTP/2 or HTTP/3.

@reschke
Copy link
Contributor

reschke commented May 17, 2023

I agree that the inconsistency is bad.

That said, why do you believe that using multiple field instances can cause an interop problem?

@davidben
Copy link
Contributor

I'm fairly sure Chromium does this, so if there were interop problems, we'd probably have noticed by now. :-)

The phrasing is a little odd and probably could be improved. One resolution is to say this is an HPACK-specific encoding thing and not actually sending multiple fields. The full text from 9113 says:

To allow for better compression efficiency, the Cookie header field MAY be split into separate header fields, each with one or more cookie-pairs. If there are multiple Cookie header fields after decompression, these MUST be concatenated into a single octet string using the two-octet delimiter of 0x3b, 0x20 (the ASCII string "; ") before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application.

I.e. this field splitting starts and ends its life in HTTP/2.

@bagder
Copy link
Author

bagder commented May 17, 2023

A concern:

Some servers have a maximum request header field size limit of around 8K. curl makes sure to never send a Cookie: header longer than that as it risks getting a 400 back and basically blocking that particular request without it being very obvious to the user. This is a limit RFC6265(bis) does not mention but probably should because it is one of the more important ones.

By splitting up the header into multiple ones, the total size of the sent cookies should probably still remain smaller than 8K since there is a risk that there is a h2/3=>h1 conversion done that will pass those cookies on in a single header and thus be affected by this limit.

I.e. this field splitting starts and ends its life in HTTP/2.

It would if HTTP/3 didn't have the same wording so it also applies there.

@bagder
Copy link
Author

bagder commented May 17, 2023

why do you believe that using multiple field instances can cause an interop problem?

Because one RFC says we MUST NOT send them and another says we MAY. I bet there is a server side or two out there that then will not appreciate getting multiple such header fields, perhaps because the h2/h3 special case is not taken care of. But this is just be being worried, I don't know for sure this is the case.

@davidben
Copy link
Contributor

By splitting up the header into multiple ones, the total size of the sent cookies should probably still remain smaller than 8K since there is a risk that there is a h2/3=>h1 conversion done that will pass those cookies on in a single header and thus be affected by this limit.

Agreed that such a limit should apply to the total here. I think that would follow from treating this as a funny HPACK encoding of a single field, rather than semantically sending multiple fields. That's not precisely how it's described today, but it results in the exact same behavior on the wire. I.e. we can still get the compression benefits of compressing individual cookies separately, and resolve the slightly awkward phrasing.

@guoye-zhang
Copy link
Contributor

Safari started splitting Cookies in h2 and h3 a few years ago, it actually improved website compatibility where the h2 load balancer had a per-field size limit but the ultimate h1 origin did not.

@reschke
Copy link
Contributor

reschke commented May 18, 2023

Because one RFC says we MUST NOT send them and another says we MAY. I bet there is a server side or two out there that then will not appreciate getting multiple such header fields, perhaps because the h2/h3 special case is not taken care of. But this is just be being worried, I don't know for sure this is the case.

Understood and agreed. So yes, that "MUST NOT" doesn't make any sense, and it's inconsistent with the base HTTP spec. Would be interesting to find out how that got there.

@bagder
Copy link
Author

bagder commented May 18, 2023

Would be interesting to find out how that got there.

I can't recall the discussions about it on the http-state list back in the day (man, it's been over a decade already!) and I could not find any relevant threads when I casually searched the archives.

@reschke
Copy link
Contributor

reschke commented May 18, 2023

Changed from SHOULD NOT to MUST NOT between draft 09 and 10:

https://author-tools.ietf.org/iddiff?url1=draft-ietf-httpstate-cookie-09&url2=draft-ietf-httpstate-cookie-10&difftype=--html

@MikeBishop
Copy link
Contributor

I think the wording is intended to be internal to HPACK/QPACK -- that is, there can only ever be one Cookie field as far as HTTP is concerned, but Cookie is special-cased to be split when encoded. To deal with the special case, any occurrence of multiple values gets coalesced. I don't see this as a contradiction.

@sbingler
Copy link
Collaborator

sbingler commented Nov 3, 2023

Are there any remaining concerns before I close this issue?

I agree with Mike's interpretation that this isn't a contradiction. The spec mandates that UAs generate a single header field which the server expects on the other end. I don't believe that another layer transparently muxing/demuxing this field is an issue.

@bagder
Copy link
Author

bagder commented Nov 3, 2023

I agree that Mike's interpretation is the sensible one to draw from this. I disagree that this is what the specs actually say in the written words.

@sbingler
Copy link
Collaborator

sbingler commented Nov 8, 2023

I can see the potential for confusion here. This seems to stem from 6265bis's tendency to re-implement other specs requirements, but in a somewhat inferior way.

The cookie spec probably shouldn't care how the HTTP request is specifically formed so long as the invariant of a single cookie-string from UA -> Server is maintained by the time the server's cookie processing code receives it.

#2084 would probably help here.

In the meantime, I'd prefer if we didn't special case actions such as 9113's cookie header field splitting, so maybe some rephrasing could avoid the issue?

Here's a rough draft of what I mean. My intention here is to make it clear that the UA (more specifically the cookie code) should create a single cookie-string to attach as the cookie: header's value. This would lead to a single header in the default case, but should still allow for header splitting.

5.7.1. The Cookie Header Field
The user agent includes stored cookies in the Cookie HTTP request header field.

A user agent MAY omit the Cookie header field in its entirety. For example, the user agent might wish to block sending
cookies during "third-party" requests from setting cookies (see Section 7.1).

If the user agent does attach a Cookie header field to an HTTP request, the user agent MUST generate a single
cookie-string and the user agent MUST compute the cookie-string following the algorithm defined in Section 5.7.3,
where the retrieval's URI is the request-uri, the retrieval's same-site status is computed for the HTTP request as defined in
Section 5.2, and the retrieval's type is "HTTP".

@MikeBishop
Copy link
Contributor

9113 and 9114 have to special-case Cookies for one simple reason: they're a semicolon-delimited list rather than a comma-delimited list. 9110 already has generic rules for comma-delimited lists; the text in question applies the same logic to Cookies despite the different delimiter and points out that for compression reasons it's particularly useful to split Cookie fields. (It might have been worth noting that other list fields might see similar gains from being split and recombined across the compression boundary, too.)

I think @sbingler's proposed text on generation is fine -- there should always be exactly one Cookie header generated from UA->server, though a lower layer might mux it before compression. After decompression, demuxing is mandatory when being passed into a context that doesn't know to expect being split.

However, there's that exception in both RFCs: "...before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application." This text clearly supposes that anything version-specific which comes after RFC 7540 will know that Cookies may have been split; perhaps this document should prescribe tolerance of receiving that on the server side. It's then an implementation concern whether that tolerance takes the form of recombining them before processing.

@sbingler
Copy link
Collaborator

sbingler commented Mar 6, 2024

This text clearly supposes that anything version-specific which comes after RFC 7540 will know that Cookies may have been split; perhaps this document should prescribe tolerance of receiving that on the server side.

Which document specifically?

@MikeBishop
Copy link
Contributor

Sorry, let me clarify antecedents:

This text [found in RFCs 7540, 9113, and 9114] clearly supposes that anything version-specific which comes after RFC 7540 will know that Cookies may have been split; perhaps [6265bis] should prescribe tolerance of receiving that on the server side.

That is, perhaps the Cookie spec should require that servers be able to handle Cookie fragments having been split into multiple Cookie headers, since this document has the opportunity to be aware of HTTP/2+ behavior.

@sbingler
Copy link
Collaborator

sbingler commented Mar 7, 2024

Thanks for clarifying, that makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

7 participants