lifeomic / axios-fetch

A WebAPI Fetch implementation backed by an Axios client
MIT License
172 stars 30 forks source link

Fix header class detection #119

Closed ecl1ps closed 2 years ago

ecl1ps commented 2 years ago

When you create a new object by using Object.create(null) then this new instance doesn't inherit from anything. It means Object.create(null).constructor === undefined (https://stackoverflow.com/a/15518712/14750967). And that is the error we are encountering and this PR is fixing.

That is the way how apollo/client@3.7 is preparing headers which are then passed to axios-fetch https://github.com/apollographql/apollo-client/blob/9134aaf3b6fc398b2d82439b5b63848b533ae4c9/src/link/http/selectHttpOptionsAndBody.ts#L203.

This is a regression caused by the following cleanup commit https://github.com/lifeomic/axios-fetch/commit/33d696262a27b4a6a5e88c3d1fe4558c5a864776.

Added test to reproduce this problem.

loscm commented 2 years ago

Requested review from a random @lifeomic/security team member, @bishopb, due to term: regression.

This review is not blocking and is for broader awareness. Consider if this change requires deeper security review and ask when it is necessary.

mdlavin commented 2 years ago

Thank you for your contribution! It's perfect ❤️ . Clear description, fix and a testcase. I'm running the tests now and I'll merge when it passes

mdlavin commented 2 years ago

Oh bummer. I forgot we require commit signing. @ecl1ps any chance you could force push a signed commit with the same change?

ecl1ps commented 2 years ago

Done, signed. But it seems I am too late :D

mdlavin commented 2 years ago

Your change was so good, and I couple other people reported it. I cheated and merged without a signature. Thanks again

ecl1ps commented 2 years ago

Glad to help