ember-fastboot / ember-cli-fastboot

Server-side rendering for Ember.js apps
http://ember-fastboot.com/
MIT License
852 stars 160 forks source link

multiple cookies are rewritten/concatenated into an invalid single cookie string #893

Closed st-h closed 1 year ago

st-h commented 2 years ago

If the backend server returns multiple cookies headers like:

set-cookie: X-Cookie1=value1
set-cookie: X-Cookie2=value2

fastboot servers returns the following header:

set-cookie: X-Cookie1=value1, X-Cookie2=value2

The issue with that is that browsers do ignore everything but the first cookie (which can lead to issues like authentication no longer working). The MDN page is quite clear about that behaviour: To send multiple cookies, multiple Set-Cookie headers should be sent in the same response..

I had a look at the fastboot code that sets the cookie headers on the fastboot response, but it looked fine to me. So I guess the issue lies where the app hands over the headers to the fastboot code (however, I could not find that code so far).

st-h commented 1 year ago

Turns out the root case here lies in ember data and my own code. ED creates an object that contains all headers. However, the Headers object always returns a string representation of the value. When multiple cookies with the same key exists, they are joined with a comma

function headersToObject(headers: Headers): Dict<unknown> {
  let headersObject = {};

  if (headers) {
    headers.forEach((value, key) => (headersObject[key] = value));
  }

  return headersObject;
}

In my code I was trying to make the cookies available to the browser by putting it into the fastboot response in an ED adapter:

handleResponse(status, headers, payload, requestData) {
  if (this.fastboot.isFastBoot) {
    this.fastboot.response.headers.set('set-cookie', headers['set-cookie']);
  }
}

In order to make this work, I need to parse the string again and append the individual cookie values:

headers['set-cookie'].split(',')
            .map(c => c.trim())
            .forEach(cookie => this.fastboot.response.headers.append('set-cookie', cookie))

I think there isn't anything we can do to improve this, without making substantial changes to ED, so I am just going to close this.