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

http: fix non-string header value concatenation #4460

Closed

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Dec 28, 2015

Since headers are stored in an empty literal object ({}) instead of an object created with Object.create(null), care must be taken with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing strings instead.

Fixes: #4456

@mscdex mscdex force-pushed the fix-http-non-string-header-concat branch from 82d1159 to 7878d43 Compare December 28, 2015 23:43
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Dec 28, 2015
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: nodejs#4456
@mscdex mscdex force-pushed the fix-http-non-string-header-concat branch from 7878d43 to 070b617 Compare December 28, 2015 23:44
@mscdex
Copy link
Contributor Author

mscdex commented Dec 28, 2015

/cc @nodejs/http

@mscdex
Copy link
Contributor Author

mscdex commented Dec 29, 2015

@indutny
Copy link
Member

indutny commented Dec 29, 2015

LGTM

4 similar comments
@Trott
Copy link
Member

Trott commented Dec 29, 2015

LGTM

@cjihrig
Copy link
Contributor

cjihrig commented Dec 29, 2015

LGTM

@MylesBorins
Copy link
Contributor

LGTM

@JungMinu
Copy link
Member

LGTM

mscdex added a commit that referenced this pull request Dec 30, 2015
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: #4456
PR-URL: #4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@mscdex
Copy link
Contributor Author

mscdex commented Dec 30, 2015

Landed in 2a1ef97.

@mscdex mscdex closed this Dec 30, 2015
@mscdex mscdex deleted the fix-http-non-string-header-concat branch December 30, 2015 17:41
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Jan 6, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: nodejs#4456
PR-URL: nodejs#4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: #4456
PR-URL: #4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: #4456
PR-URL: #4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: nodejs#4456
PR-URL: nodejs#4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: nodejs#4456
PR-URL: nodejs#4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Since headers are stored in an empty literal object ({}) instead
of an object created with Object.create(null), care must be taken
with property names inherited from Object. Currently there are
only functions inherited, so we can safely check for existing
strings instead.

Fixes: nodejs#4456
PR-URL: nodejs#4460
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants