Update request cookie parser to handle zero or more spaces between semicolons #7312
Conversation
Can we also find some precedent in any other HTTP implementations? e.g. Netty, Pekko, the JDK. |
Sure thing! I forked both pekko and netty and added tests. I found that both actually handle zero or more spaces after semicolons, do you think we should do the same here?
|
Aha, thanks for checking that! Yes, while we're here I suppose we can handle that case too 🙃 Mandatory speech from Ross in #7104 (comment). |
mrdziuban
changed the title
Done! Ross' comments definitely resonate, I would much prefer to fix whatever is going wrong on my user's end that results in multiple spaces, but my debugging so far leads me to believe it's an issue in a proprietary corporate firewall so my chances of that seem slim 😬 |
| val cookieString = (RequestCookie.parser ~ ( | ||
| (char(';') *> char(' ').rep0).soft *> RequestCookie.parser | ||
| ).rep0).map { case (head, tail) => | ||
| Cookie(NonEmptyList(head, tail)) | ||
| } | ||
|
|
||
| // We also see trailing semi-colons in the wild, and grudgingly tolerate them here |
There was a problem hiding this comment.
Maybe we can add a comment similar to this one esp. since the implementation no longer matches the comment above.
mrdziuban
force-pushed
the
request-cookie-multi-space
branch
from
e9a3d36
to
e5dc421
Compare
This updates the cookie header parser to handle zero or more spaces between semicolons, whereas the current code only handles a single space. For example, these are both currently invalid, but are considered valid with these changes:
While this technically goes against the HTTP spec, I have seen it happen (and just had to fix a bug in production because of it 😅) and there seems to be precedent for going a little off spec, e.g. by handling trailing semicolons in request cookies and by handling zero or more spaces in
Set-Cookieheaders.