-
Notifications
You must be signed in to change notification settings - Fork 850
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
Store HTTP response headers in case-insensitive array #531
Conversation
@brandur-stripe Also, could you check if stripe-go is affected by this issue (since it's the only other library that uses HTTP/2)? (My guess is that it's not because the |
|
||
use ArrayAccess; | ||
|
||
class CaseInsensitiveArray implements ArrayAccess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could put a comment block in here that mentions that this exists to provide case insensitive access to response headers because their case may vary depending on whether the request came back over HTTP or HTTP/2 (which mandates lowercase)? I think it may be helpful in reasoning about why it's here in the future.
Nice implementation!
Yeah, it looks like you found it already, but the // Header maps header keys to values. If the response had multiple
// headers with the same key, they may be concatenated, with comma
// delimiters. (RFC 7230, section 3.2.2 requires that multiple headers
// be semantically equivalent to a comma-delimited sequence.) When
// Header values are duplicated by other fields in this struct (e.g.,
// ContentLength, TransferEncoding, Trailer), the field values are
// authoritative.
//
// Keys in the map are canonicalized (see CanonicalHeaderKey).
Header Header And CanonicalHeaderKey returns the canonical format of the header key s. The canonicalization converts the first letter and any letter following a hyphen to upper case; the rest are converted to lowercase. For example, the canonical key for "accept-encoding" is "Accept-Encoding". If s contains a space or invalid header field bytes, it is returned without modifications. So we're okay there. Definitely a good thing to keep an eye out for when we start getting to other implementations though. r? @ob-stripe |
I added a comment as requested, ptal @brandur-stripe. (And thanks for confirming that stripe-go is unaffected!) |
Thanks OB! LGTM. |
Released as 6.19.2. |
r? @brandur-stripe @remi-stripe
cc @stripe/api-libraries
Store HTTP response headers in case-insensitive array. HTTP headers are supposed to be case-insensitive, but since we stored them in a regular PHP array, they were actually case-sensitive.
This is particularly problematic since the API will return headers with different cases depending on whether HTTP/2 is used or not (since HTTP/2 requires that all headers be in lowercase).
Fixes #529.