Skip to content

http: fix first body chunk fast case for UTF-16 #12747

New issue

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

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

Already on GitHub? # to your account

Closed

Conversation

addaleax
Copy link
Member

http.OutgoingMessage tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: #11788

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

http

CI: https://ci.nodejs.org/job/node-test-commit/9509/

@nodejs/http

`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: nodejs#11788
@addaleax addaleax added http Issues or PRs related to the http subsystem. lts-watch-v4.x labels Apr 29, 2017
@nodejs-github-bot nodejs-github-bot added the http Issues or PRs related to the http subsystem. label Apr 29, 2017
@mscdex
Copy link
Contributor

mscdex commented May 1, 2017

LGTM

@addaleax
Copy link
Member Author

addaleax commented May 3, 2017

Landed in 55c95b1

@addaleax addaleax closed this May 3, 2017
@addaleax addaleax deleted the http-outgoing-first-chunk-singlebyte branch May 3, 2017 13:33
addaleax added a commit that referenced this pull request May 3, 2017
`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: #11788
PR-URL: #12747
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
anchnk pushed a commit to anchnk/node that referenced this pull request May 6, 2017
`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: nodejs#11788
PR-URL: nodejs#12747
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@jasnell jasnell mentioned this pull request May 11, 2017
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
3 tasks
gibfahn pushed a commit that referenced this pull request Jun 18, 2017
`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: #11788
PR-URL: #12747
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@gibfahn
Copy link
Member

gibfahn commented Jun 18, 2017

Landed this on v6.x, LMK if I shouldn't have.

Seems worth backporting, #11788 mentioned that this was broken on 5.x and up.

gibfahn pushed a commit that referenced this pull request Jun 20, 2017
`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: #11788
PR-URL: #12747
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
`http.OutgoingMessage` tried to send the first chunk together
with the headers by concatenating them together as a string, but the
list of encodings for which that works was incorrect.

Change it from a blacklist to a whitelist.

Fixes: #11788
PR-URL: #12747
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
# for free to join this conversation on GitHub. Already have an account? # 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.

8 participants