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.

Sign up for GitHub

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

Jump to bottom

[v12.x backport] http: don't throw on Uint8Arrays for http.ServerResponse#write #33488

Closed
wants to merge 2 commits into from
Closed

[v12.x backport] http: don't throw on Uint8Arrays for http.ServerResponse#write #33488

wants to merge 2 commits into from

Conversation

rexagod
Copy link
Member

@rexagod rexagod commented May 20, 2020 •
edited

Don't throw errors on Uint8Arrays and added test for all
valid types.

PR-URL: #33155
Fixes: #33379
Refs: #29829
Reviewed-By: Robert Nagy ronagy@icloud.com
Reviewed-By: Anna Henningsen anna@addaleax.net
Reviewed-By: Zeyu Yang himself65@outlook.com
Reviewed-By: James M Snell jasnell@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added http Issues or PRs related to the http subsystem. v12.x labels May 20, 2020
@rexagod
http: don't throw on Uint8Arrays for http.ServerResponse#write
d2a920c 
Don't throw errors on Uint8Arrays and added test for all
valid types.

Backport-PR-URL: nodejs#33488
PR-URL: nodejs#33155
Fixes: nodejs#33379
Refs: nodejs#29829
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@rexagod
Copy link
Member Author

rexagod commented May 20, 2020 •
edited

Backport-PR-URL specified in d2a920c (I was unable to trigger builds since I do not have the permission to do so, so I'm specifying the commit here, which is the same that that was force-pushed above).

@BridgeAR BridgeAR mentioned this pull request May 23, 2020
Merged
@BridgeAR
Copy link
Member

Ideally, all commits are backported directly from the following major version. I suggest to wait with this until the backport for v14 landed and see how and if there's need for a manual backport afterwards.

@codebytere codebytere force-pushed the v12.x-staging branch 2 times, most recently from b64963e to 3b1d9b3 Compare July 21, 2020 18:34
@nodejs-github-bot
Copy link
Collaborator

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

MylesBorins pushed a commit that referenced this pull request Aug 18, 2020
@rexagod @MylesBorins
http: don't throw on Uint8Arrays for http.ServerResponse#write
8069f8d 
Don't throw errors on Uint8Arrays and added test for all
valid types.

Backport-PR-URL: #33488
PR-URL: #33155
Fixes: #33379
Refs: #29829
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Member

landed in 8069f8d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielleadams danielleadams danielleadams approved these changes

@BethGriggs BethGriggs BethGriggs approved these changes

@shisama shisama shisama approved these changes

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.

None yet
7 participants
@rexagod @BridgeAR @nodejs-github-bot @MylesBorins @danielleadams @BethGriggs @shisama