Use HeaderMap instead of plain Map for headers

[vtipparam]

The expressMiddleware handler uses a plain Map for the headers that results in case sensitive lookup on the headers. This makes the header Apollo-Query-Plan-Experimental ineffective when the ApolloServer is used with expressMiddleware handler.

Resolves apollographql/federation#2337

[netlify]

✅ Deploy Preview for apollo-server-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	9cd9f38
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-server-docs/deploys/63c87266226c300008698183
😎 Deploy Preview	https://deploy-preview-7313--apollo-server-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 9cd9f38:

Sandbox	Source
Apollo Server Typescript	Configuration
Apollo Server	Configuration

[trevor-scheer]

Thanks for the PR! Would you please add a changeset by running npx changeset (just a patch since this is a bugfix).

[glasser]

Huh! @trevor-scheer do you understand why this wasn't a compilation failure?

The field HTTPGraphQLRequest.headers is defined to be a HeaderMap, and class HeaderMap extends Map<string, string>. Does TypeScript interpret this as meaning that headers can be anything that has the public API of HeaderMap (which doesn't include anything more than Map) I guess? I didn't realize that...

[glasser]

Yeah I suppose so: https://www.typescriptlang.org/docs/handbook/2/classes.html#relationships-between-classes

Do we want to make some change here (somehow compatibly?) to make this issue less possible?

[vtipparam]

Huh! @trevor-scheer do you understand why this wasn't a compilation failure?

I was wondering about that as well. Magic of Duck Typing in Typescript?

[trevor-scheer]

Yeah, maybe some additional property on the HeaderMap will give us the differentiation we're expectingto have.

[theJC]

FYI, any of the apollo-server-integrations until each one has switched over to use HeaderMap AND cuts a new release, consumers will likely still be using case-sensitve map for headers.

1. At Indeed we use Koa, and therefore we use apollo-server-integration-koa. We hit this case sensitive header issue, but fortunately we got lucky that this was fixed by @trevor-scheer in chore(deps): update all non-major dependencies apollo-server-integrations/apollo-server-integration-koa#84 -- but we didnt have access to this fix until fortunately release v0.3.0 was cut a few days ago (I believe for other reasons) and allowed us to pull in the fix to this issue.

2. The hapi integration looks to still be using new Map(): /~https://github.com/apollo-server-integrations/apollo-server-integration-hapi/blob/f9ef226df0dcb3d8ebf2189629dd368c6b6b2d9b/src/index.ts#L99 and may still have this issue.

3. Maybe all the integrations need to be reviewed to make sure this is corrected for everyone, and releases get cut rather than waiting for significant other changes to be made to finally get a release cut.

4. Should there/is there a plan to make sure best practices/important changes information (like this one) that is made in the built-in express middleware is disseminated so that all the frameworks fix the same issue if they are effected?

[trevor-scheer]

Hey @theJC. These integrations are intentionally (somewhat) disparate and community-owned, so while at some level I help maintain them in a pinch / as-needed, the hope is that community members who are interested in both Apollo Server and their respective framework of choice maintain them, respond to issues, etc. It's not a perfect system but I do keep an eye on them and step in if needed.

That being said - we do export an integration testsuite and typings which act as a guide for integration authors to stay in step with expected behaviors. As long as authors keep their dev dependencies up-to-date, use TypeScript, and run our testsuite in CI, we can reasonably expect things should be working as intended.

In the specific HeaderMap case, this was ultimately a typings issue which slipped through the cracks. The fix I landed here is a good example of how we disseminate important changes (in this case through typings) - so as the integrations update their dependencies, they should see a new error if applicable.

The important part for this to work is that we remain vigilant with appropriate testsuite additions and typings fixes as applicable. This is imperfect, but it's pretty good. And to be fair to this project, AS4 has successfully made these integrations very simple and the pattern repeatable so overall I'm not too worried about this being a recurring or frequent problem.

If you see any places where we can improve this process or have anything in mind, I'd be happy to hear ideas! And if you'd like to participate / help improve any of the integrations you're of course invited to do so.