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.

Sign up for GitHub

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

Jump to bottom

Cookie.fromSetCookieValue doesn't handle double quotes #33327

Closed
tecbot opened this issue Jun 4, 2018 · 10 comments
Closed

Cookie.fromSetCookieValue doesn't handle double quotes #33327

tecbot opened this issue Jun 4, 2018 · 10 comments
Assignees
@sortie
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@tecbot
Copy link

tecbot commented Jun 4, 2018

Hi,

I'm using the HttpClient from the dart:io package within a Flutter project. Some websites returning in the set-cookie headers, cookie values with double quotes, e.g. instagram.com if expiring cookies:
'sessionid=""; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Max-Age=0; Domain=instagram.com; Path=/'.

If calling Cookie.fromSetCookieValue with the example above, the function will throw an error: Invalid character in cookie value, code unit: '34' The value is in this case ""

I found out that the code doesn't handle double quotes. Any plans to implement this behaviour? Go has an implementation for it https://github.com/golang/go/blob/master/src/net/http/cookie.go#L369

I'm happy to contribute, if wanted.

Flutter (Channel dev, v0.5.1, on Mac OS X 10.13.4 17E199, locale de-DE)
Framework revision c7ea3ca377 (6 days ago), 2018-05-29 21:07:33 +0200
Engine revision 1ed25ca7b7
Dart version 2.0.0-dev.58.0.flutter-f981f09760
@kevmoo kevmoo added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io labels Jun 5, 2018
@alexdelorenzo
Copy link

Also encountered this issue with Instagram and other sites.

@alexdelorenzo alexdelorenzo mentioned this issue Jul 6, 2018
Closed
@jonahwilliams
Copy link
Contributor

Unless I am misreading RFC 6265, then a cookie-value that is enclosed in double quotes should be valid, and the current Dart SDK implementation is an oversight.

 cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
 cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                     ; US-ASCII characters excluding CTLs,
                     ; whitespace DQUOTE, comma, semicolon,
                     ; and backslash

@thosakwe
Copy link

Can confirm this occurs; also using Instagram.

dart-bot pushed a commit that referenced this issue Aug 10, 2018
@whesse
Validate cookie values wrapped in double-quotes
a9ad427 
Servers sometimes send headers with cookie values that are encapsulated in double-quotes. Dart should validate values surrounded with double-quotes instead of throwing a FormatException.

This addresses Issue #33327 directly. I applied the solution to this problem that was [solved in Go's code base](https://github.com/golang/go/blob/master/src/net/http/cookie.go#L369).

Closes #33765
#33765

GitOrigin-RevId: 99672dd
Change-Id: Ie95a064611b1aa15aea93f5c8d801ecfc7d996c4
Reviewed-on: https://dart-review.googlesource.com/63920
Reviewed-by: Zach Anderson <zra@google.com>
Commit-Queue: Zach Anderson <zra@google.com>
@Sense545
Copy link

The pull request #33765 fails for cookies with empty value but no quotes:
CookieName=; expires=somedateinthepast

@kevmoo
Copy link
Member

kevmoo commented Jan 24, 2019

This is related to commit a9ad427

@zanderso thoughts?

@sortie sortie added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 25, 2019
@sortie sortie self-assigned this Jan 25, 2019
@sortie
Copy link
Contributor

sortie commented Jan 25, 2019

I put up a fix at https://dart-review.googlesource.com/c/sdk/+/91221

@stuartsoft
Copy link

@sortie any updates on this? Was your review ever merged in?

@sortie
Copy link
Contributor

sortie commented May 10, 2019

Hi @stuartsoft. Nope, the changelist is still in review. I have approval but I wanted to take a final look before finishing it up. Unfortunately I got sidetracked with some other important tasks, but I'll try to get back to this soon.

@sortie
Copy link
Contributor

sortie commented May 22, 2019

The fix in a9ad427 stripped double quotes instead of preserving them as part of the value. I'm working on a fix to preserve the double quotes as part of the value instead of being considered an encoding.

@sortie
Copy link
Contributor

sortie commented May 23, 2019

I've now landed the fix that preserves the double quotes as part of the value instead of stripping them.

@2x2xplz 2x2xplz mentioned this issue Jul 31, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sortie sortie

Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-io P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@kevmoo @tecbot @Sense545 @alexdelorenzo @sortie @stuartsoft @jonahwilliams @thosakwe