smollweide / node-mock-server

File based Node REST API mock server
MIT License
255 stars 65 forks source link

[fix] Fix mutation of global headers by using Object.assign #107

Closed kyusu closed 6 years ago

kyusu commented 6 years ago

Hi @smollweide,

sorry, but it's me again. So my colleagues had some fun with another interesting and very subtle bug today. After they added a custom header to the response one of our mocked paths they noticed that this custom headers is now suddenly appearing on all other responses as well. After some digging around they found the issue: In line 177 of the _handleMockRequest method of MockController.js there is the following expression:

        var headers = this.options.headers || {};

So far not a problem. But in line 211 of the very same method there is the following expression:

        this._writeDefaultHeader(res, extend(headers, responseHeaders));

Since headers is only a reference to this.options.headers the custom response headers for this particular response are actually copied to the global headers object (via the extend call).

Our solution was to clone the global headers object via

        var headers = Object.assign({}, this.options.headers);

Best regards, Christian

smollweide commented 6 years ago

Hi @kyusu, thanks you 😄

Release: v0.23.4

greetings, Simon

kyusu commented 6 years ago

Hi @smollweide,

you're welcome. Credit has to go to @dypsilon for finding the bug in the first place. And thanks again for implementing node-mock-server. Despite the two little bugs I really like it.

Regards, Christian

dypsilon commented 6 years ago

@kyusu thank you for creating the pull request! Regards, Tim