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

OutgoingMessage._send misperforms ucs2 encoding of first chunk #11788

Closed
kyriosli opened this issue Mar 10, 2017 · 1 comment
Closed

OutgoingMessage._send misperforms ucs2 encoding of first chunk #11788

kyriosli opened this issue Mar 10, 2017 · 1 comment
Labels
http Issues or PRs related to the http subsystem.

Comments

@kyriosli
Copy link

kyriosli commented Mar 10, 2017

  • Version: master, at least since 5.x, maybe earlier
  • Platform: any
  • Subsystem: http

I figured out that http.OutgoingMessage tries to send the first chunk together with the headers by concating them together as a string (here), but ucs2 is not in the judgement list so it is treated same as binary or utf8, which causes the headers sent as ucs2 at the same time.

Example code:

require('http').createServer(function (req, res) {
    res.setHeader('content-type', 'text/plain; charset=utf-16');
    res.write('helloworld', 'ucs2');
    res.end();
}).listen(8080);

And try explaining the output:

$ echo -e "GET / HTTP/1.1\r\n\r\n" | netcat localhost 8080 | hexdump -C
00000000  48 00 54 00 54 00 50 00  2f 00 31 00 2e 00 31 00  |H.T.T.P./.1...1.|
00000010  20 00 32 00 30 00 30 00  20 00 4f 00 4b 00 0d 00  | .2.0.0. .O.K...|
00000020  0a 00 63 00 6f 00 6e 00  74 00 65 00 6e 00 74 00  |..c.o.n.t.e.n.t.|
00000030  2d 00 74 00 79 00 70 00  65 00 3a 00 20 00 74 00  |-.t.y.p.e.:. .t.|
00000040  65 00 78 00 74 00 2f 00  70 00 6c 00 61 00 69 00  |e.x.t./.p.l.a.i.|
00000050  6e 00 0d 00 0a 00 44 00  61 00 74 00 65 00 3a 00  |n.....D.a.t.e.:.|
00000060  20 00 46 00 72 00 69 00  2c 00 20 00 31 00 30 00  | .F.r.i.,. .1.0.|
00000070  20 00 4d 00 61 00 72 00  20 00 32 00 30 00 31 00  | .M.a.r. .2.0.1.|
00000080  37 00 20 00 31 00 30 00  3a 00 34 00 36 00 3a 00  |7. .1.0.:.4.6.:.|
00000090  34 00 36 00 20 00 47 00  4d 00 54 00 0d 00 0a 00  |4.6. .G.M.T.....|
000000a0  43 00 6f 00 6e 00 6e 00  65 00 63 00 74 00 69 00  |C.o.n.n.e.c.t.i.|
000000b0  6f 00 6e 00 3a 00 20 00  6b 00 65 00 65 00 70 00  |o.n.:. .k.e.e.p.|
000000c0  2d 00 61 00 6c 00 69 00  76 00 65 00 0d 00 0a 00  |-.a.l.i.v.e.....|
000000d0  54 00 72 00 61 00 6e 00  73 00 66 00 65 00 72 00  |T.r.a.n.s.f.e.r.|
000000e0  2d 00 45 00 6e 00 63 00  6f 00 64 00 69 00 6e 00  |-.E.n.c.o.d.i.n.|
000000f0  67 00 3a 00 20 00 63 00  68 00 75 00 6e 00 6b 00  |g.:. .c.h.u.n.k.|
00000100  65 00 64 00 0d 00 0a 00  0d 00 0a 00 31 00 34 00  |e.d.........1.4.|
00000110  0d 00 0a 00 68 00 65 00  6c 00 6c 00 6f 00 77 00  |....h.e.l.l.o.w.|
00000120  6f 00 72 00 6c 00 64 00  0d 00 0a 00 30 0d 0a 0d  |o.r.l.d.....0...|
@addaleax addaleax added the http Issues or PRs related to the http subsystem. label Mar 10, 2017
addaleax added a commit to addaleax/node that referenced this issue Apr 29, 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
@addaleax
Copy link
Member

PR to fix this: #12747

anchnk pushed a commit to anchnk/node that referenced this issue 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>
gibfahn pushed a commit that referenced this issue 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 pushed a commit that referenced this issue 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 issue 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>
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

No branches or pull requests

2 participants