Skip to content

backport: http: fix connection upgrade checks#9681

Closed
mscdex wants to merge 1 commit intonodejs:v4.x-stagingfrom
mscdex:backport-v4-pr-8238
Closed

backport: http: fix connection upgrade checks#9681
mscdex wants to merge 1 commit intonodejs:v4.x-stagingfrom
mscdex:backport-v4-pr-8238

Conversation

@mscdex
Copy link
Contributor

@mscdex mscdex commented Nov 18, 2016 •
edited
Loading

Checklist
  • make -j8 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().

This is a backport of #8238.

CI: https://ci.nodejs.org/job/node-test-pull-request/4894/

@mscdex
http: fix connection upgrade checks
69119c6 
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 added http Issues or PRs related to the http subsystem. v4.x labels Nov 18, 2016
@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. v4.x labels Nov 18, 2016
@mscdex mscdex mentioned this pull request Nov 18, 2016
Merged
3 tasks
MylesBorins
MylesBorins approved these changes Nov 18, 2016
View reviewed changes
Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

rubber stamp LGTM if CI is green

@mscdex
Copy link
Contributor Author

mscdex commented Nov 18, 2016 •
edited
Loading

I'm not sure what's up with the freebsd11 node failure, it looks like it has an old compiler installed or something?

OS X tty test failure is unrelated.

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 19, 2016

it hasn't ever been working on v4

just been rolling with it

On Fri, Nov 18, 2016, 3:38 PM Brian White notifications@github.com wrote:

I'm not sure what's up with the freebsd-11 node, it looks like it has an
old compiler installed or something?

—
You are receiving this because you commented.

Reply to this email directly, view it on GitHub
#9681 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecVyRnCkzAtx8Mowj-3VW7PcM4YCaMks5q_gy8gaJpZM4K24xT
.

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 22, 2016

one last ci: https://ci.nodejs.org/job/node-test-pull-request/4934/

@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 22, 2016

looking good
landed in 50a4471

@mscdex mscdex deleted the backport-v4-pr-8238 branch November 22, 2016 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@MylesBorins MylesBorins MylesBorins approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

http Issues or PRs related to the http subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mscdex @MylesBorins @nodejs-github-bot