search

adobe / openwhisk-action-utils

Utilities for OpenWhisk Actions
Apache License 2.0
2 stars 3 forks source link

Setting multiple cookies fails using expressify #213

Closed icaraps closed 2 years ago

icaraps commented 2 years ago

Setting multiple cookies works fine without expressify ✅ Setting a single cookie works fine with expressify ✅ Setting multiple cookies doesn't work with expressify ❌

Please have a look at the following code for an action. 

const express = require('express');
const { expressify } = require('@adobe/openwhisk-action-utils');

function main(params) {
  // This is working fine
  // return {
  //   headers: {
  //     'Set-Cookie': [
  //       'cookie-a=1',
  //       'cookie-b=2'
  //     ],
  //     'Content-Type': 'text/html'
  //   },
  //   statusCode: 200,
  //   body: '<html><body>OK</body></html>'
  // }

  const app = express();
  app.get('/', async (req, res) => {

    // Setting multiple cookies in an array is failing. No cookies are set. 
    res.setHeader('Set-Cookie', ['cookie-a=1', 'cookie-b=2']);

    // Setting single cookie works
    // res.setHeader('Set-Cookie', 'cookie-a=1');
    res.send('ok');
  });

  return expressify(app)(params)
}

exports.main = main;
icaraps commented 2 years ago

https://www.npmjs.com/package/serverless-http used by expressify only supports AWS provider and therefore handles multivalue parameters with multiValueHeaders see https://aws.amazon.com/fr/blogs/compute/support-for-multi-value-parameters-in-amazon-api-gateway/ for details.

A simple Wworkaround is to verify if the property is set and adapt the response eg

const res = await expressify(app)(params);

  if (res.multiValueHeaders) {
    res.headers = {
      ...res.headers,
      ...res.multiValueHeaders
    };
    delete res.multiValueHeaders;
  }

  return res;

I imagine another solution would be to add new provider openwhisk to http-serverless to handle it.

trieloff commented 2 years ago

@icaraps as we don't use cookies in Helix, this will probably not get a lot of attention from us, but we'd review a PR, should it come to that.

tripodsan commented 2 years ago

we don't use cookies in Helix

yes, we do, in helix-admin where we set the auth cookie. and it is actually a problem with helix-fetch (any maybe helix-universal) that multiheaders can't be used. so we have to encode multiple cookie values into 1 cookie

tripodsan commented 2 years ago

also see: https://fetch.spec.whatwg.org/#headers-class

https://stackoverflow.com/questions/63204093/how-to-get-set-multiple-set-cookie-headers-using-fetch-api/63254504#63254504

github-actions[bot] commented 2 years ago

:tada: This issue has been resolved in version 4.4.8 :tada:

The release is available on:

Your semantic-release bot :package::rocket: