write() writes partial codes of multibyte characters

[trollixx]

I've noticed that partial bytes of UCS-2 symbols are written, while they shouldn't be according to the Node.js documentation.

Here's the code taken from Node.js test:

var f = new Buffer([0, 0, 0, 0, 0]);
assert.equal(f.length, 5);
var size = f.write('あいうえお', 'ucs2');
console.error('bytes written to buffer: %d     (should be 4)', size);
assert.equal(size, 4);
assert.deepEqual(f, new Buffer([0x42, 0x30, 0x44, 0x30, 0x00]));

[feross]

Confirmed this is an issue. PR welcome!

cc @jessetane @eliangcs (who wrote a bunch of the utf16 code) Any thoughts?

[jessetane]

I think the problem is that blitBuffer currently has no way of knowing how many bytes are required to compose a code unit for the given encoding. Maybe this could be solved by passing an additional parameter unitSize to blitBuffer from each encodings' write function?

[feross]

Fixed in #49.

[trollixx]

#49 fixes problem with UCS-2/UTF-16, but what about UTF-8? Characters can be more that one byte long, so again the same situation with partially written data can occur. Basically, length of target buffer and writes()'s length parameter should be checked for every encoding, that allows more than one byte per character.

[jessetane]

Yeah, my patch only ensures whole code units, not necessarily whole code points. So it's not a proper fix for utf8 or even ucs2/utf16 (which may also use surrogate pairs - 4 bytes - to encode things outside the bmp). It might be worth having all of node's tests brought over before trying to do this through as there may even be some differences between the docs and the way node actually does things, for ex: http://blog.nodejs.org/2014/06/16/openssl-and-breaking-utf-8-change/

[jessetane]

Here is a first pass at dealing with partial / invalid code points. It definitely increases compatibility with node, but there are some things about node's behavior I don't understand / could not match, see comments inline:

jessetane@2ad1029

[feross]

@jessetane this looks like a really good start. have you tried looking directly at the node buffer source code to find out exactly what they're doing? If something doesn't make sense, I'd file a bug against https://github.com/iojs/io.js and/or https://github.com/joyent/node.

also, it might make sense for us to get the node buffer tests in the codebase first so we can evaluate how these changes improve/reduce compatibility with node.

[feross]

FYI, this issue was fully fixed in 3.0.0.

[feross]

(And released in browserify 7.1.0)