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

response_headers_to_object can be improved #863

Open
ninthsun91 opened this issue Apr 4, 2024 · 0 comments
Open

response_headers_to_object can be improved #863

ninthsun91 opened this issue Apr 4, 2024 · 0 comments

Comments

@ninthsun91
Copy link
Contributor

I think response_headers_to_object is not good enough. Actually it is quite uncomfortable when multiple cookies with options are set by Set-Cookie header.

const response_headers_to_object = (

Problems

When multiple cookies are set without any option, then it's fine.

For example,

res.setCookie('cookie1', 'qwe123');
res.setCookie('cookie2', 'asd123');

this will be converted to:

'set-cookie': [
    'cookie1=qwe123',
    'cookie2=asd123'
]

However, when cookie security options are set along like this:

res.setCookie('cookie1', 'qwe123', {
    httpOnly: true,
    secure: true,
    sameSite: 'lax',
    path: '/',
    expires: new Date(),
});
res.setCookie('cookie2', 'asd123', {
    httpOnly: true,
    secure: true,
    sameSite: 'lax',
    path: '/',
    expires: new Date(),
});

now things get messy

'set-cookie': [
    'cookie1=qwe123',
    'HttpOnly',
    'Secure',
    'SameSite=Lax',
    'Path=/',
    'Expires=Fri, 05 Apr 2024 08:31:47 GMT',
    'cookie2=asd123',
    'HttpOnly',
    'Secure',
    'SameSite=Lax',
    'Path=/',
    'Expires=Fri, 05 Apr 2024 08:31:47 GMT',
]

This is not comfortable as it is hard to tell which cookie each elements belong to.

Suggestions

Fortunately, response_headers_to_object is parsing multiple cookies one by one

key: set-cookie
value: cookie1=qwe123; Path=/; Expires=Fri, 05 Apr 2024 12:31:46 GMT; HttpOnly; Secure; SameSite=Lax
key: set-cookie
value: cookie2=asd123; Path=/; Expires=Fri, 05 Apr 2024 12:31:46 GMT; HttpOnly; Secure; SameSite=Lax

So, I would suggest either

1. pushing cookie strings

const response_headers_to_object = (
  headers: Headers,
): Record<string, string | string[]> => {
  const output: Record<string, string | string[]> = {};
  headers.forEach((value, key) => {
    if (key === "set-cookie") {
      output[key] ??= [];
      (output[key] as string[]).push(value);
    } else output[key] = value;
  });
  return output;
};
'set-cookie': [
    'cookie1=qwe123; Path=/; Expires=Fri, 05 Apr 2024 12:31:46 GMT; HttpOnly; Secure; SameSite=Lax',
    'cookie2=asd123; Path=/; Expires=Fri, 05 Apr 2024 12:31:46 GMT; HttpOnly; Secure; SameSite=Lax'
]

So, let people handle their cookie themselves,
or

2. construct as object

const response_headers_to_object = (
  headers: Headers,
): Record<string, string | string[] | Record<string, any>> => {
  const output: Record<string, string | string[] | Record<string, any>> = {};
  headers.forEach((value, key) => {
    if (key === "set-cookie") {
      output[key] ??= {};
      const parsedCookie = parseCookies([value]);
      Object.assign(output[key], parsedCookie);
    } else output[key] = value;
  });
  return output;
};

type CookieOptions = CookieOption['options'] & { value: string };
export const parseCookies = (cookies: string[]) => {
  return cookies.reduce(
    (acc, cookie) => {
      const parts = cookie.split(';');
      const keyValue = parts.shift();
      if (keyValue === undefined) return acc;

      const [key, value] = keyValue.split('=');
      acc[key] = parts.reduce(
        (acc, part) => {
          const splits = part.trim().split('=');
          const optionKey = splits[0].replace(/[A-Z]/, (match) => {
            return match.toLowerCase();
          }) as keyof CookieOption['options'];
          const optionValue = splits[1];
          switch (optionKey) {
            case 'expires':
              acc.expires = new Date(optionValue);
              break;
            case 'httpOnly':
            case 'secure':
              acc[optionKey] = true;
              break;
            default:
              acc[optionKey] = optionValue as any;
          }
          return acc;
        },
        { value: value.trim() } as CookieOptions,
      );

      return acc;
    },
    {} as Record<string, CookieOptions>,
  );
};
'set-cookie': {
    cookie1: {
      value: 'qwe123',
      path: '/',
      expires: 2024-04-05T09:14:04.000Z,
      httpOnly: true,
      sameSite: 'Lax'
    },
    cookie2: {
      value: 'asd123',
      path: '/',
      expires: 2024-04-05T09:14:04.000Z,
      httpOnly: true,
      sameSite: 'Lax'
    }
  },

and possibly better if you could provide type-safe getter.

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

No branches or pull requests

1 participant