Repository: denoland/deno
Pull request: fix(ext/node): fix buffer.indexOf for ucs2 encoding

This PR fixes `buffer.indexOf` when the encoding is 'ucs2'.

Currently `indexOf` searches in the heystack at each byte, but it should only search at every 2-byte step when 'ucs2' specified in the encoding parameter of `indexOf`

towards #29589

Changed files:
filename: ext/node/polyfills/internal_binding/buffer.ts
patch: @@ -9,6 +9,7 @@ export function indexOfNeedle(
   source: Uint8Array,
   needle: Uint8Array,
   start = 0,
+  step = 1,
 ): number {
   if (start >= source.length) {
     return -1;
@@ -17,7 +18,7 @@ export function indexOfNeedle(
     start = Math.max(0, source.length + start);
   }
   const s = needle[0];
-  for (let i = start; i < source.length; i++) {
+  for (let i = start; i < source.length; i += step) {
     if (source[i] !== s) continue;
     const pin = i;
     let matched = 1;
@@ -100,9 +101,11 @@ function indexOfBuffer(
     throw new Error(`Unknown encoding code ${encoding}`);
   }
 
+  const isUcs2 = encoding === Encodings.UCS2;
+
   // If the encoding is UCS2 and haystack or needle has a length less than 2, the search will always fail
   // https://github.com/nodejs/node/blob/fbdfe9399cf6c660e67fd7d6ceabfb106e32d787/src/node_buffer.cc#L1067-L1069
-  if (encoding === Encodings.UCS2) {
+  if (isUcs2) {
     if (buffer.length < 2 || targetBuffer.length < 2) {
       return -1;
     }
@@ -128,7 +131,7 @@ function indexOfBuffer(
     return byteOffset <= targetBuffer.length ? byteOffset : targetBuffer.length;
   }
 
-  return indexOfNeedle(targetBuffer, buffer, byteOffset);
+  return indexOfNeedle(targetBuffer, buffer, byteOffset, isUcs2 ? 2 : 1);
 }
 
 function indexOfNumber(

filename: tests/node_compat/config.jsonc
patch: @@ -24,7 +24,6 @@
       "test-blocklist.js",
       "test-buffer-arraybuffer.js",
       "test-buffer-backing-arraybuffer.js",
-      "test-buffer-indexof.js",
       "test-buffer-tostring-range.js",
       "test-child-process-exec-abortcontroller-promisified.js",
       "test-child-process-exec-encoding.js",

filename: tests/node_compat/test/parallel/test-buffer-indexof.js
patch: @@ -2,7 +2,7 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 18.12.1
+// Taken from Node 23.9.0
 // This file is automatically generated by `tests/node_compat/runner/setup.ts`. Do not modify this file manually.
 
 'use strict';
@@ -83,9 +83,7 @@ assert.strictEqual(b.indexOf(Buffer.from('f'), 5), 5);
 assert.strictEqual(b.indexOf(Buffer.from('f'), -1), 5);
 assert.strictEqual(b.indexOf(Buffer.from('f'), 6), -1);
 
-// TODO(Soremwar)
-// Enable again once encoding is taking into account when evaluating indexOf
-// assert.strictEqual(Buffer.from('ff').indexOf(Buffer.from('f'), 1, 'ucs2'), -1);
+assert.strictEqual(Buffer.from('ff').indexOf(Buffer.from('f'), 1, 'ucs2'), -1);
 
 // Test invalid and uppercase encoding
 assert.strictEqual(b.indexOf('b', 'utf8'), 1);
@@ -294,10 +292,7 @@ assert.strictEqual(-1, asciiString.indexOf('\x2061'));
 assert.strictEqual(asciiString.indexOf('leb', 0), 3);
 
 // Search in string containing many non-ASCII chars.
-const allCodePoints = [];
-for (let i = 0; i < 65534; i++) allCodePoints[i] = i;
-const allCharsString = String.fromCharCode.apply(String, allCodePoints) +
-    String.fromCharCode(65534, 65535);
+const allCharsString = Array.from({ length: 65536 }, (_, i) => String.fromCharCode(i)).join('');
 const allCharsBufferUtf8 = Buffer.from(allCharsString);
 const allCharsBufferUcs2 = Buffer.from(allCharsString, 'ucs2');
 
@@ -345,28 +340,26 @@ assert.strictEqual(Buffer.from('aaaaa').indexOf('b', 'ucs2'), -1);
   }
 }
 
-// TODO(Soremwar)
-// Enable again once encoding is taking into account when evaluating indexOf
-// {
-//   // Find substrings in Usc2.
-//   const lengths = [2, 4, 16];  // Single char, simple and complex.
-//   const indices = [0x5, 0x65, 0x105, 0x205, 0x285, 0x2005, 0x2085, 0xfff0];
-//   for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) {
-//     for (let i = 0; i < indices.length; i++) {
-//       const index = indices[i] * 2;
-//       const length = lengths[lengthIndex];
-
-//       const patternBufferUcs2 =
-//           allCharsBufferUcs2.slice(index, index + length);
-//       assert.strictEqual(
-//         index, allCharsBufferUcs2.indexOf(patternBufferUcs2, 0, 'ucs2'));
-
-//       const patternStringUcs2 = patternBufferUcs2.toString('ucs2');
-//       assert.strictEqual(
-//         index, allCharsBufferUcs2.indexOf(patternStringUcs2, 0, 'ucs2'));
-//     }
-//   }
-// }
+{
+  // Find substrings in Usc2.
+  const lengths = [2, 4, 16];  // Single char, simple and complex.
+  const indices = [0x5, 0x65, 0x105, 0x205, 0x285, 0x2005, 0x2085, 0xfff0];
+  for (let lengthIndex = 0; lengthIndex < lengths.length; lengthIndex++) {
+    for (let i = 0; i < indices.length; i++) {
+      const index = indices[i] * 2;
+      const length = lengths[lengthIndex];
+
+      const patternBufferUcs2 =
+          allCharsBufferUcs2.slice(index, index + length);
+      assert.strictEqual(
+        index, allCharsBufferUcs2.indexOf(patternBufferUcs2, 0, 'ucs2'));
+
+      const patternStringUcs2 = patternBufferUcs2.toString('ucs2');
+      assert.strictEqual(
+        index, allCharsBufferUcs2.indexOf(patternStringUcs2, 0, 'ucs2'));
+    }
+  }
+}
 
 [
   () => {},
@@ -605,22 +598,20 @@ pattern = reallyLong.slice(0, 2000000);  // first 2/5ths.
 assert.strictEqual(reallyLong.lastIndexOf(pattern), 0);
 
 // Test truncation of Number arguments to uint8
-// TODO(Soremwar)
-// Enable once multi byte number search is available
-// {
-//   const buf = Buffer.from('this is a test');
-//   assert.strictEqual(buf.indexOf(0x6973), 3);
-//   assert.strictEqual(buf.indexOf(0x697320), 4);
-//   assert.strictEqual(buf.indexOf(0x69732069), 2);
-//   assert.strictEqual(buf.indexOf(0x697374657374), 0);
-//   assert.strictEqual(buf.indexOf(0x69737374), 0);
-//   assert.strictEqual(buf.indexOf(0x69737465), 11);
-//   assert.strictEqual(buf.indexOf(0x69737465), 11);
-//   assert.strictEqual(buf.indexOf(-140), 0);
-//   assert.strictEqual(buf.indexOf(-152), 1);
-//   assert.strictEqual(buf.indexOf(0xff), -1);
-//   assert.strictEqual(buf.indexOf(0xffff), -1);
-// }
+{
+  const buf = Buffer.from('this is a test');
+  assert.strictEqual(buf.indexOf(0x6973), 3);
+  assert.strictEqual(buf.indexOf(0x697320), 4);
+  assert.strictEqual(buf.indexOf(0x69732069), 2);
+  assert.strictEqual(buf.indexOf(0x697374657374), 0);
+  assert.strictEqual(buf.indexOf(0x69737374), 0);
+  assert.strictEqual(buf.indexOf(0x69737465), 11);
+  assert.strictEqual(buf.indexOf(0x69737465), 11);
+  assert.strictEqual(buf.indexOf(-140), 0);
+  assert.strictEqual(buf.indexOf(-152), 1);
+  assert.strictEqual(buf.indexOf(0xff), -1);
+  assert.strictEqual(buf.indexOf(0xffff), -1);
+}
 
 // Test that Uint8Array arguments are okay.
 {