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

Split cookies on semicolons, when followed by valid cookie characters #12

Merged
merged 1 commit into from
Aug 19, 2022

Conversation

rpkyle
Copy link
Contributor

@rpkyle rpkyle commented May 20, 2021

The fixes proposed in #9 were addressed instead via e8289f8, but the following error continues to appear intermittently as of reqres 0.2.3.9000:

ERROR: 'names' attribute [11] must be the same length as the vector [10]

The proposed change is to initially parse the cookie string via stri_split_regex instead of stri_split_fixed, with splitting only performed on ; if followed by a valid cookie character (as defined in RFC 6265).

Internally we have also identified a potential issue with handling of nameless cookies -- those with valid values but no names, as described here: httpwg/http-extensions#159.

This arises because splitting on the name/value pairs in this case results in an attribute vector for the names object whose length is odd (see L441, of the parse_cookies function, paraphrased here):

> cookie <- c("", "1234", "foo", "5678")
> structure(
+      as.list(url_decode(cookies[c(FALSE, TRUE)])),
+      names = cookies[c(TRUE, FALSE)]
+  )
Error in attributes(.Data) <- c(attributes(.Data), attrib) : 
  'names' attribute [3] must be the same length as the vector [2]

The end result is that c("", "foo") is passed via names, but only the second name is valid since the first is dropped.

Our temporary fix was to drop nameless cookies, but it's probably preferable to retain them since IE, Chrome and Firefox treat them as valid.

@thomasp85 thomasp85 merged commit 4dfaab3 into thomasp85:main Aug 19, 2022
@thomasp85
Copy link
Owner

Thank you

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

Successfully merging this pull request may close these issues.

None yet

2 participants