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

Fastboot causes double decoding #284

Open
arschmitz opened this issue Jul 10, 2019 · 2 comments
Open

Fastboot causes double decoding #284

arschmitz opened this issue Jul 10, 2019 · 2 comments
Labels

Comments

@arschmitz
Copy link

arschmitz commented Jul 10, 2019

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

@marcoow
Copy link
Member

marcoow commented Aug 2, 2019

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 marcoow added the bug label Aug 2, 2019
@arschmitz
Copy link
Author

@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

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

No branches or pull requests

2 participants