mainmatter/ember-cookies

Fastboot causes double decoding

arschmitz opened this issue · 2 comments

when reading cookies in with fastboot they are being uri decoded twice causing an error if the cookie contains a %. This happens because fastboot uses the cookie package to parse the headers and add to the request object https://github.com/ember-fastboot/fastboot/blob/5a42df54ce3359a33589173d9c79a2236e619ba1/src/fastboot-request.js#L52 cookie calls decodeURIComponent by default when parsing https://www.npmjs.com/package/cookie#decode the value is then decoded again by ember-cookies https://github.com/simplabs/ember-cookies/blob/master/addon/services/cookies.js#L207.

a simple reproduction is

if (!this.fastboot.isFastBoot) {
  this.cookies.write('broken', '%')
}

this.cookies.read('broken')

I know there is a raw option i cannot use this since i'm using ember-cimple-auth. However i also don't think the current behavior is correct regardless since it would require it to be encoded on write and raw on read. I also don't think it would ever be the correct behavior to double decode something since this would change the format unexpectedly if someone intentionally double encoded something for some reason or if it similarly contained a restricted character.

I think the correct fix would be to never use decodeURIComponent when in fastboot

I am happy to submit a PR for this if you agree with the approach

for now my workaround is

_decodeValue(value, raw) {
  if (isNone(value) || raw) {
    return value;
  } else {
    try {
      return decodeURIComponent(value)
    } catch (error) {
      return value
    }
  }
}

This is obviously not great though since it overrides a private method and does not actually handle the double encoding problem but it solved my particular situation

Thanks for the report @arschmitz and sorry for the slow response. The solution you're suggesting (never calling decodeURIComponent when in FastBoot) seems correct. I think you'd just have to get that working correctly with the _fastBootCookiesCache where cookies are read from potentially and those reads would not go through cookie.parse and then.

@marcoow no problem on slow response its open source I know how it is. Ill do a PR for this when i have a few minutes