samchon / nestia

NestJS Helper Libraries + TypeScript OpenAPI generator
https://nestia.io/
MIT License
1.71k stars 89 forks source link

`response_headers_to_object` can be improved #863

Open ninthsun91 opened 3 months ago

ninthsun91 commented 3 months ago

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.

https://github.com/samchon/nestia/blob/fbe753e45807c3a92d54957c4643bb8423c89352/packages/fetcher/src/internal/FetcherBase.ts#L256

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.