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

RangeError when creating a cookie with an empty vale #35189

Closed
tomyeh opened this issue Nov 16, 2018 · 4 comments
Closed

RangeError when creating a cookie with an empty vale #35189

tomyeh opened this issue Nov 16, 2018 · 4 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-duplicate Closed in favor of an existing report library-_http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@tomyeh
Copy link

tomyeh commented Nov 16, 2018

Dart 2.1
MacOS

Test Code:

new Cookie("foo", "")

Exception:

RangeError (index): Invalid value: Valid value range is empty: 0
#0      _StringBase.[] (dart:core/runtime/libstring_patch.dart:237:55)
#1      _Cookie._validate (dart:_http/http_headers.dart:989:14)
#2      new _Cookie (dart:_http/http_headers.dart:844:5)
#3      new Cookie (dart:_http:1105:54)
#4      _newRemeCookie (package:quire/server/s

Cause:

_validate doesn't honer empty case

@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) library-_http labels Nov 16, 2018
@hoylen
Copy link
Contributor

hoylen commented Jan 21, 2019

Hi tomyeh,

I came across this issue too, when running the stable Dart v2.1.0 release.

Just looking at your fix for this issue, https://github.com/tomyeh/sdk/commit/8d1d9bafa30d3ea8ef05ec4416ce1b3d5d251167, and thought it might need to handle the situation where the value is a one-character string containing just a double quote (i.e. value == '"').

It that situation, the code will throw a RangeError, because the if statement will be true (treating that character as both the opening and closing double-quote) and value.substring(1, value.length - 1) will try to call substring(1, 0) and fail.

Looking at the cookie grammar in RFC 6265, I think that is not an error, but is a valid cookie-value. It doesn't match the second alternative in the definition of a cookie-value, so it must match the first.

 cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )

That is, treating the one-character string " no differently from how it would treat the four-character strings"foo or bar".

P.S. There is probably a meta-discussion about what the "value" of a cookie really is. The Dart implementation currently treats the double-quotes as a part of the string value -- which I think is correct, since the RFC says "The semantics of the cookie-value are not defined...". So it needs to preserve any double quotes, so the application can decide if they are significant or not (i.e. when to produce headers with or without them, and what to do if it receives a cookie with or without them).

korsvanloon added a commit to korsvanloon/auth that referenced this issue Apr 4, 2019
Setting a cookie with an empty string `''` causes a crash.

See: dart-lang/sdk#35189
@korsvanloon
Copy link

If I'm reading the RFC spec correctly, doesn't it say that a cookie-octet cannot be a "?

So you either have 2 bounding double-quotes or none, but never 1.

@hoylen
Copy link
Contributor

hoylen commented Apr 4, 2019

Sorry, you are correct.

Looking at it again, if the cookie-value production didn't match the right alternative (the one where there is a matching pair of double quotes), then I (incorrectly) though the left alternative meant zero or more of any character. But actually *cookie-octet does not match a single double quotes (since the cookie-octet excludes that character).

Anyway, I'm looking forward to being able to create cookies with zero-length values again. Thanks for fixing that.

@sortie
Copy link
Contributor

sortie commented May 28, 2019

Hi, I fixed this bug, it's a duplicate of #35804.

@sortie sortie closed this as completed May 28, 2019
@sortie sortie added the closed-duplicate Closed in favor of an existing report label May 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. closed-duplicate Closed in favor of an existing report library-_http type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants