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 connection upgrade checks #8238

Merged
merged 1 commit into from
Aug 26, 2016

Conversation

mscdex
Copy link
Contributor

@mscdex mscdex commented Aug 23, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • http
Description of change

This commit fixes connection upgrade checks, specifically when headers are passed as an array instead of a plain object to http.request().

Fixes: #8235

@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Aug 23, 2016
@mscdex
Copy link
Contributor Author

mscdex commented Aug 23, 2016

/cc @nodejs/http

@mscdex
Copy link
Contributor Author

mscdex commented Aug 23, 2016

@mscdex
Copy link
Contributor Author

mscdex commented Aug 23, 2016

CI is green except for an unrelated failure on AIX.

@jasnell
Copy link
Member

jasnell commented Aug 23, 2016

LGTM

This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <jasnell@gmail.com>
@mscdex mscdex merged commit 1050594 into nodejs:master Aug 26, 2016
@mscdex mscdex deleted the http-add-headers-guard branch August 26, 2016 14:19
@Fishrock123 Fishrock123 mentioned this pull request Sep 6, 2016
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: #8235
PR-URL: #8238
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

@mscdex this does not land cleanly on v4.x, but we definitely should backport as I have confirmed it is broken on v4.x

Would you be able to backport?

@MylesBorins MylesBorins added this to the v4.6.2 milestone Oct 24, 2016
@MylesBorins MylesBorins modified the milestones: v4.7.0, v4.6.2 Oct 26, 2016
@MylesBorins
Copy link
Contributor

ping @mscdex

mscdex added a commit to mscdex/io.js that referenced this pull request Nov 18, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: nodejs#8235
PR-URL: nodejs#8238
Reviewed-By: James M Snell <jasnell@gmail.com>
@mscdex
Copy link
Contributor Author

mscdex commented Nov 18, 2016

@thealphanerd #9681

MylesBorins pushed a commit that referenced this pull request Nov 22, 2016
This commit fixes connection upgrade checks, specifically when headers
are passed as an array instead of a plain object to http.request()

Fixes: #8235
PR-URL: #8238
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins removed this from the 4.7.0 milestone Nov 22, 2016
@MylesBorins MylesBorins mentioned this pull request Nov 22, 2016
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.

error http request at headers is array
3 participants