Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix response headers usage with upcoming node changes #3174

Closed
wants to merge 1 commit into from

Conversation

mscdex
Copy link

@mscdex mscdex commented Jan 14, 2017

No description provided.

// Support changes in node with how response headers are stored
var headerNames = this.res._headerNames || {};
var keys = Object.keys(resHeaders);
if (keys.length > 0 && headerNames[keys[0]] === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a whole lot of Node.js http mock libraries out there (https://github.com/howardabrams/node-mocks-http comes to mind) and they have never mocked the _headerNames object. I think this would cause the usage of those modules to change the behavior (probably break) people's tests for Express projects that use them since this code would detect those mocks as the new style of headers and then copy all the header values as undefined, right?

@dougwilson dougwilson added this to the 4.15 milestone Feb 21, 2017
@dougwilson dougwilson mentioned this pull request Feb 21, 2017
22 tasks
@dougwilson
Copy link
Contributor

I just landed the change that landed in the send module here in the 4.15 branch to be included in the next release.

dougwilson added a commit that referenced this pull request Feb 21, 2017
@mscdex
Copy link
Author

mscdex commented Feb 28, 2017

Closing this as now more public API methods exist and there is now a docs-only deprecation of the private properties/methods.

@mscdex mscdex closed this Feb 28, 2017
@expressjs expressjs deleted a comment from brneto Feb 7, 2020
@expressjs expressjs deleted a comment from fukayatsu Feb 7, 2020
@expressjs expressjs locked as resolved and limited conversation to collaborators Feb 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants