buffer.indexOf is incorrect in utf16le encoding for odd byteOffset

[hkjackey]

Version: v11.10.1
Platform: Windows 10 (64-bits)
Subsystem: buffer
File Encoding: UTF8

Please consider the following 2 lines of code:
let buf = Buffer.from('\u6881\u6882\u6881', 'utf16le');
console.log(buf.indexOf('\u6881', 1, 'utf16le'));

In this example, the expected correct output should be 4.
However, the result is 0.

[abhinavsagar]

It would be much better if you showed exactly what the error log displays.

[mkopa]

@hkjackey ucs2 (utf16le) encoding is always two bytes. So byteOffset must by multiple of 2.
Look at the code: https://github.com/nodejs/node/blob/master/src/node_buffer.cc#L945 you see offset has been divided by 2 and result multiplied by 2.
If you put offset equal 1 you have 1 div 2 mul 2 == 0.

[addaleax]

Currently, indexOf with UTF16-LE is odd anyway in another respect as well, namely that we only search for the needle at even indices:

> Buffer.from('00aaaa', 'hex').indexOf('\uaaaa', 'utf16le')
-1
> Buffer.from('0000aaaa', 'hex').indexOf('\uaaaa', 'utf16le')
2

I’m not sure if this is “correct”, because some people might expect this?

/cc @nodejs/buffer

[RReverser]

I think it's correct as we wouldn't really want to break codepoint boundaries (although need to recheck if same holds for UTF-8 or any other encoding).

[seishun]

You mean code unit boundaries. This isn't relevant for UTF-8 because a UTF-8 code unit is a byte.

I suggest closing as expected behavior.

[RReverser]

You mean code unit boundaries. This isn't relevant for UTF-8 because a UTF-8 code unit is a byte.

No, I do mean a codepoint (1-4 bytes in case of UTF-8).

[seishun]

In UTF-16 a codepoint can be represented by one or two 16-bit code units. I don't think Buffer.indexOf has any checks for that.

[RReverser]

I don't think Buffer.indexOf has any checks for that.

Hmm that's another good question (although personally I'd prefer it to check if it doesn't).

[hkjackey]

@hkjackey ucs2 (utf16le) encoding is always two bytes. So byteOffset must by multiple of 2.
Look at the code: https://github.com/nodejs/node/blob/master/src/node_buffer.cc#L945 you see offset has been divided by 2 and result multiplied by 2.
If you put offset equal 1 you have 1 div 2 mul 2 == 0.

@mkopa Thanks for the reply.

I agree that utf16le encoding is always two bytes, but buffer is not.
"buffer" can be intended to store a mix of many things.
For example, the beginning 7 bytes of a buffer is used for some other things,
then starting from the 7th byte is used to store utf16le string.
In this case, an odd byteOffset is needed.

I regarded the current behavior as a bug because according to the NodeJS specification:
https://nodejs.org/api/buffer.html#buffer_buf_indexof_value_byteoffset_encoding
it does not mention that odd byteOffset is not allowed.

I would accept the current behavior not a bug if the NodeJS specification mentioned that.

[hkjackey]

Hi everybody!
Below I want to persuade that returning -1 for odd byteOffset in utf16le can be really weird.

Please consider the following 5 lines of code:
let buf = Buffer.alloc(7);
let str = '\u6881\u6882\u6881';
console.log(buf.write(str, 1, 'utf16le')); //6 (successfully written)
console.log(buf); //<Buffer 00 81 68 82 68 81 68>
console.log(buf.indexOf(str, 1, 'utf16le')); //-1 (NOT found)

I write a string at odd byteOffset 1 but finding it using indexOf at the same position
give me -1 which means "NOT found".
If anyone still regard the above behavior valid, then I have nothing to say.

[seishun]

That might be a breaking change - if the UTF-16 data starts at an even offset, indexOf might find a code unit composed of an odd byte and an even byte that happen to match the input value. For instance, this test breaks.

However, the behavior in the issue description is clearly a bug - the offset should be rounded up to an even value, which is a simple fix. Thoughts?

[addaleax]

the offset should be rounded up to an even value, which is a simple fix. Thoughts?

I would disagree – I think buffer.indexOf(str, offset, enc) should work the same way that buffer.slice(offset).indexOf(str, enc) works (with an adjusted return value).