feat(node:buffer): implement transcode()

[0xfandom]

What does this PR do?

Implements buffer.transcode(source, fromEncoding, toEncoding), which previously threw Error: Not implemented. Fixes #24235.

Supports the same encodings as Node.js — ascii, latin1/binary, ucs2/utf16le, utf8 — and substitutes ? for code points the target encoding cannot represent (e.g. transcode(Buffer.from('€'), 'utf8', 'ascii') → ?).

The dispatch mirrors Node's src/node_i18n.cc:

• simdutf fast paths for ASCII/Latin-1 → UCS2 and UTF8 ↔ UCS2
• a UTF-16 pivot (decode → re-encode) for the remaining pairs, matching ICU's substitution behavior

ICU's converter headers (unicode/ucnv.h) aren't available in the build's header set, so the pivot is implemented directly rather than via ucnv_convertEx. Error behavior matches Node: a non-Uint8Array source throws ERR_INVALID_ARG_TYPE, and unsupported/unknown encodings throw Unable to transcode Buffer [U_ILLEGAL_ARGUMENT_ERROR] with matching code/errno.

How did you verify your code works?

Added coverage in test/js/node/buffer.test.js (replacing the old "is undefined / Not implemented" assertion):

• the issue repro (€ utf8→ascii = ?)
• the known-answer vectors from Node's test/parallel/test-icu-transcode.js (utf8→latin1/ascii/ucs2, ucs2→utf8 round-trip, ascii/latin1→utf16le)
• Uint8Array source, empty-buffer short-circuit, and the ERR_INVALID_ARG_TYPE / U_ILLEGAL_ARGUMENT_ERROR error paths

Also ran Node's €×4000 round-trip stress assertions against the debug build. The new test passes with the debug build and fails against the release bun (proving it exercises the change); the full buffer.test.js file passes (506 pass, 0 fail).

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

Walkthrough

Replaces the buffer.transcode no-op stub in NodeBufferModule.h with a full implementation. It defines a TranscodeEncoding enum, a UTF-16 pivot decode/encode pipeline with SIMD specializations and ill-formed UTF-8 replacement, a Node-compatible error throw helper, and the 3-argument jsBufferConstructorFunction_transcode host function. Tests are updated to assert correct output, error types, and edge cases.

Changes

Buffer.transcode() implementation

Layer / File(s)	Summary
Encoding type mapping and error infrastructure src/jsc/modules/NodeBufferModule.h	Adds BufferEncodingType.h, JSBufferEncodingType.h, and <unicode/utypes.h> headers. Defines the TranscodeEncoding enum (ASCII, LATIN1, UTF8, UCS2, Unsupported) and a mapper from WebCore::BufferEncodingType. Implements parseTranscodeEncoding accepting only JSString values and throwTranscodeError to emit Node-matching errors with code and errno properties from ICU status codes.
UTF-16 pivot decode/encode pipeline src/jsc/modules/NodeBufferModule.h	Implements UTF-8 ill-formed-sequence replacement helpers using maximal subpart algorithm to produce U+FFFD. Adds decodeToUtf16 dispatching ASCII/LATIN1/UTF8/UCS2 to SIMD or fallback paths. Adds encodeFromUtf16 re-encoding the pivot with ? substitution for unrepresentable code points, using SIMDUTF for UCS2 and UTF8 output. Adds transcodeGeneric and SIMD-specialized helpers for Latin1→UCS2, UTF8→UCS2, and UCS2→UTF8 conversions with UErrorCode reporting.
JS host function, module export wiring, and tests src/jsc/modules/NodeBufferModule.h, test/js/node/buffer.test.js	Implements jsBufferConstructorFunction_transcode to validate JSUint8Array source, reject detached buffers, short-circuit empty input, parse encodings, copy bytes to stable buffer, dispatch to correct path, and return Uint8Array or throw. Updates NodeBuffer export from undefined stub to 3-argument function. Tests assert byte-vector correctness for multiple encoding pairs, Uint8Array source acceptance, empty-buffer short-circuiting, ERR_INVALID_STATE for detached buffers, ERR_INVALID_ARG_TYPE for null source, and U_ILLEGAL_ARGUMENT_ERROR for unsupported encoding combinations with matching code and errno.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(node:buffer): implement transcode()' clearly summarizes the main change—implementing the transcode function for Node.js buffer compatibility.
Description check	✅ Passed	The description fully covers both required template sections: it explains what the PR does (implement transcode with encoding support and error handling) and how it was verified (comprehensive test coverage with specific test cases and validation results).
Linked Issues check	✅ Passed	The implementation fully addresses issue #24235 by implementing the transcode function with support for the four required encodings (ascii, latin1, ucs2, utf8), proper error handling, and the specific case of transcoding UTF-8 Euro symbol to ASCII producing '?'.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the transcode function: NodeBufferModule.h adds the implementation and export, buffer.test.js adds corresponding test coverage, with no unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/modules/NodeBufferModule.h`:
- Around line 421-441: The transcode function reads the byteLength() at line 426
and copies from typedVector() at line 441 without validating that the source
Uint8Array is not detached. Add a detached-view check immediately after the
source parameter validation (after the dynamicDowncast check) to ensure the
Uint8Array has valid backing storage before attempting to read byteLength or
copy data. Reference the detached-view guard pattern used in neighboring isUtf8
and isAscii paths to implement this validation consistently.
- Around line 396-405: The error check on line 402 that treats zero-length UTF-8
conversion results as U_INVALID_CHAR_FOUND is incorrect because line 396
intentionally truncates odd UCS2 byte lengths to get complete char16_t units,
which can legitimately result in zero-length output (e.g., a one-byte input
produces zero char16_t units). Remove or modify the `if (!length)` error
condition to allow zero-length results as valid outcomes, so that inputs with no
complete code units return an empty buffer instead of an error status, making
this behavior consistent with the generic UCS2 conversion path.

In `@test/js/node/buffer.test.js`:
- Around line 2561-2586: The test assertions need tightening to verify stronger
invariants. First, in the empty input test for BufferModule.transcode with empty
buffer, replace the valid encoding strings with invalid ones to actually verify
that encodings are not being validated when input is empty. Second, in the error
handling loop testing unsupported encodings, add assertions beyond just checking
the error message - specifically assert that the error object has the correct
code property (either ERR_INVALID_ARG_TYPE.code or U_ILLEGAL_ARGUMENT_ERROR) and
verify any errno property that the native implementation sets. This ensures each
assertion can actually fail and tests the complete error metadata rather than
just the message string.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6a368d2d-7cd8-4a0a-b710-889cfe96400a

📥 Commits

Reviewing files that changed from the base of the PR and between 0c537fe and 66878e1.

📒 Files selected for processing (2)

• src/jsc/modules/NodeBufferModule.h
• test/js/node/buffer.test.js

[0xfandom]

Addressed the review in cce0575:

• Detached source (major): reject a detached Uint8Array with ERR_INVALID_STATE before reading byteLength/copying, matching the isUtf8/isAscii guard.
• Zero-length ucs2 (minor): ucs2→utf8 now returns an empty Buffer when the source has no complete code units (e.g. a 1-byte input) instead of throwing U_INVALID_CHAR_FOUND.
• Tests: empty-input case now uses invalid encoding names to prove they are not parsed on the fast path; error loop asserts err.errno === 1; added detached-source and odd-length-ucs2 cases.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/jsc/modules/NodeBufferModule.h (1)

396-405: ⚠️ Potential issue | 🔴 Critical

Validate UTF-16LE length before converter call for malformed input safety.

The code at lines 403–404 allocates a destination buffer sized by utf8_length_from_utf16le, then unconditionally calls convert_utf16le_to_utf8 with that buffer. If the input contains malformed UTF-16 surrogates, utf8_length_from_utf16le returns 0, resulting in a zero-sized buffer passed to the converter.

While simdutf's convert_utf16le_to_utf8 is documented to return 0 (not write) for malformed input, it is safer to validate the length before the conversion call rather than rely on this contract post-hoc. Add an early check: if (simdutf::utf8_length_from_utf16le(...) == 0) before allocating and converting. Alternatively, use simdutf::convert_utf16le_to_utf8_with_errors for explicit malformed-input detection.

The same pattern exists at lines 346–349 in the transcodeGeneric path and should be fixed in the same PR.

Test coverage for malformed surrogates in well-formed UCS2 pairs is absent; the existing test at line 2573 covers only odd-length input.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/modules/NodeBufferModule.h` around lines 396 - 405, Add an early
validation check after calling utf8_length_from_utf16le to ensure the returned
length is non-zero before allocating the result buffer and calling
convert_utf16le_to_utf8. Store the result of utf8_length_from_utf16le in a
variable, check if it equals zero and return an appropriate empty buffer if so,
then proceed with the Vector allocation and conversion only if the length is
valid. Apply the same fix to the transcodeGeneric path around lines 346-349
where the identical pattern exists. Additionally, add test coverage to verify
the handling of malformed UTF-16 surrogates in input that is otherwise
well-formed UCS2 pairs, beyond the existing test that only covers odd-length
input.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/jsc/modules/NodeBufferModule.h`:
- Around line 396-405: Add an early validation check after calling
utf8_length_from_utf16le to ensure the returned length is non-zero before
allocating the result buffer and calling convert_utf16le_to_utf8. Store the
result of utf8_length_from_utf16le in a variable, check if it equals zero and
return an appropriate empty buffer if so, then proceed with the Vector
allocation and conversion only if the length is valid. Apply the same fix to the
transcodeGeneric path around lines 346-349 where the identical pattern exists.
Additionally, add test coverage to verify the handling of malformed UTF-16
surrogates in input that is otherwise well-formed UCS2 pairs, beyond the
existing test that only covers odd-length input.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a810c28d-8a9a-4b8f-a01c-8c73814be3d0

📥 Commits

Reviewing files that changed from the base of the PR and between 66878e1 and cce0575.

📒 Files selected for processing (2)

• src/jsc/modules/NodeBufferModule.h
• test/js/node/buffer.test.js

[robobun]

Thanks for the PR. buffer.transcode landed on main in #34660 (native implementation in src/jsc/modules/NodeBufferModule.cpp) and #24235 is closed, so this is no longer needed. Closing.