Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix child process sending UTF8. #4999

Closed
wants to merge 1 commit into from

4 participants

@loganfsmyth

Currently the child process messages are decoded as ASCII, which breaks when UTF8 characters are stringified. As mentioned in this StackOverflow question:
http://stackoverflow.com/questions/15378871/node-js-child-process-send-utf-8-char-issue

I'm happy to add more to this like tests if someone points me in the right direction. Or if there was a reason this was ascii, maybe there is some other solution.

Basic repro steps:

var n = require('child_process').fork(...);
n.send('中文');
@bnoordhuis

LGTM but will need a couple of regression tests in test/simple/test-child-process-* (feel free to add one or more new files.)

Please see the contributing guide, in particular the sections about the commit log and the CLA. Thanks!

@Nodejs-Jenkins

Can one of the admins verify this patch?

@loganfsmyth

I still need to verify if using toString here is reasonable, or if this should be using StringDecoder. I isn't obvious whether offset and length are guaranteed to delimit complete characters.

@bnoordhuis bnoordhuis referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@loganfsmyth

Looks like @bnoordhuis wrote the test for me, sorry I didn't get a chance this week. Can this be closed then?

@TooTallNate
Owner

Yup. Closing.

@loganfsmyth loganfsmyth deleted the branch
@bnoordhuis bnoordhuis referenced this pull request from a commit in bnoordhuis/node
@bnoordhuis bnoordhuis child_process: fix sending utf-8 to child process
In process#send() and child_process.ChildProcess#send(), use 'utf8' as
the encoding instead of 'ascii' because 'ascii' mutilates non-ASCII
input.

It worked by accident in v0.8 but not in v0.10 because the high bits
are now stripped when converting Buffers to ASCII strings. See commit
96a314b for details.

Fixes #4999 and #5011.
a26a2e4
@bnoordhuis bnoordhuis referenced this pull request from a commit
@bnoordhuis bnoordhuis child_process: fix sending utf-8 to child process
In process#send() and child_process.ChildProcess#send(), use 'utf8' as
the encoding instead of 'ascii' because 'ascii' mutilates non-ASCII
input. Correctly handle partial character sequences by introducing
a StringDecoder.

Sending over UTF-8 no longer works in v0.10 because the high bit of
each byte is now cleared when converting a Buffer to ASCII. See
commit 96a314b for details.

Fixes #4999 and #5011.
44843a6
@bnoordhuis bnoordhuis referenced this pull request from a commit
@bnoordhuis bnoordhuis child_process: fix sending utf-8 to child process
In process#send() and child_process.ChildProcess#send(), use 'utf8' as
the encoding and correctly handle partial character sequences by
introducing a StringDecoder. Before this commit, it used 'ascii' and
partial sequences were dropped or corrupted.

This is a back-port of commit 44843a6 from the v0.10 branch.

Fixes #4999 and #5011.
84bb0ec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 13, 2013
  1. @loganfsmyth
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  lib/child_process.js
View
2  lib/child_process.js
@@ -325,7 +325,7 @@ function setupChannel(target, channel) {
channel.buffering = false;
channel.onread = function(pool, offset, length, recvHandle) {
if (pool) {
- jsonBuffer += pool.toString('ascii', offset, offset + length);
+ jsonBuffer += pool.toString('utf8', offset, offset + length);
var i, start = 0;
Something went wrong with that request. Please try again.