Guidance needed: HeaderMap doesn't allow setting multiple values for a header (needed for set-cookie)

[magicmark]

Hi!

When creating a custom Apollo Server Plugin, in the willSendResponse method, we might want to copy the response headers from subgraphs back down the the user's request - particularly for set-cookie.

(This problem also exists in theory for x-forwarded-for too fwiw)

HeaderMap is defined as a simple subclass of Map<string, string>: https://github.com/apollographql/apollo-server/blob/6ff88e87c52f4d6734c36fda96be07d4f8ad80cc/packages/server/src/utils/HeaderMap.ts#L1

So we can't do response.http.headers.append('set-cookie', 'foo') (this worked in prior versions of apollo, where I think we were using more of minifetch/node-fetch for this layer?)

It's also unclear how to serialize multiple set-cookie values into a single string - the cookie npm module (defacto standard, used by express) explicitly (correctly) doesn't support this https://github.com/jshttp/cookie/issues/129#issuecomment-957344975

I would suggest either: 1) provide guidance on how to concat and encode multiple set-cookie values into a single string (is this supported? I can't find where this is defined in spec) 2) Change HeaderMap: Map<string, string> => Map<string, string | Array<string>>?

Thanks!

[glasser]

This is an unfortunate outcome of having decided to base our API on what we thought of as the most "standardized" JS HTTP API (Fetch)... when that API specifically does not allow setting set-cookie headers and thus doesn't run into this problem.

I know this might not be the most compelling answer, but one answer would be to (assuming you're using expressMiddleware; similar approaches exist for other frameworks) add the Express res object to your context value in your context function, and call its methods directly instead of returning the set-cookie via the HTTPGraphQLResponse abstraction.

[magicmark]

Gotcha, thanks!

fwiw in case this helps future readers, my tests for this now look something like this:

import httpMocks from 'node-mocks-http';
...

test('forwards cookies', async () => {
    nock('http://example.com')
        .post('/foo/graphql', { query: '{hello}', variables: {} })
        .reply(200, { data: { hello: 'world' } }, [
            // Headers can be set multiple times!
            'Set-Cookie',
            'foo=bar; Max-Age=100',
            'Set-Cookie',
            'baz=qux',
        ]);

    nock('http://example.com')
        .post('/bar/graphql', { query: '{goodbye}', variables: {} })
        .reply(200, { data: { goodbye: 'yellow brick road' } }, { 'Set-Cookie': 'hello=world; Domain=example.com' });

    const gateway = new ApolloGateway({
        buildService: ({ name }) => new MyCusomServiceThatCopiesCookies(),
        supergraphSdl: schema,
    });

    const query = /* GraphQL */ `
        {
            hello
            goodbye
        }
    `;

    const { req, res } = httpMocks.createMocks();

    const server = new ApolloServer({
        gateway,
        plugins: [ForwardCookies],
    });

    const response = await server.executeOperation({ query }, { contextValue: { req, res } });
    expect(res.getHeaders()).toMatchObject({
        'set-cookie': ['foo=bar; Max-Age=100', 'baz=qux', 'hello=world; Domain=example.com'],
    });
});