From d6657d1958ae9fdd0c795fac4eb3ddcd4befda7d Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Wed, 12 Aug 2026 11:42:14 -0500 Subject: [PATCH 01/10] spec(rust-i18n): implement Rust port of Node.js i18n transcode Ports workerd::api::node::i18n::transcode to a new Rust crate, //src/rust/i18n, selected at runtime by a new NODEJS_I18N_RUST autogate. The C++ implementation is left in place, byte for byte, as the gate-off/rollback path. The dispatch table, output-size computation, ICU substitute-character setup, empty-input handling, ISOLATE_LIMIT checks, and truncation all move to Rust (src/rust/i18n/dispatch.rs). The underlying codecs stay the same: a new C++ shim (src/rust/i18n/shim.{h,c++}) exposes the ICU ucnv_* primitives and the four simdutf functions the C++ path already uses, so both paths call identical codecs and cannot silently diverge. i18n::transcode gets a gate branch at the top, following the createNodeException pattern: it maps api::node::Encoding to the bridge's Encoding enum via a fromImpl overload (kj-rs/convert.h idiom) whose switch has no default arm, rejecting BASE64/BASE64URL/HEX on either side (a deliberate divergence from the unmodified C++ dispatch, which only checks the from encoding). The Rust entry point returns a jsg::v8::ffi::MaybeLocal naming a Uint8Array; ffi-inl.h gains the missing Rust-to-C++ MaybeLocal conversion (maybe_local_from_ffi), mirroring the existing local_from_ffi. 10 Rust unit tests in dispatch.rs cover every (from, to) pair, empty input, unmappable-character substitution, the ASCII->UTF16LE latin1-widening quirk, the UTF8->UTF16LE zero-estimate quirk, invalid UTF-8, odd-length UTF-16LE input, and unpaired surrogates. --- src/rust/i18n/BUILD.bazel | 37 ++++ src/rust/i18n/dispatch.rs | 347 +++++++++++++++++++++++++++++++ src/rust/i18n/error.rs | 42 ++++ src/rust/i18n/lib.rs | 114 ++++++++++ src/rust/i18n/shim.c++ | 101 +++++++++ src/rust/i18n/shim.h | 83 ++++++++ src/rust/i18n/shim.rs | 113 ++++++++++ src/rust/jsg/ffi-inl.h | 6 + src/workerd/api/node/BUILD.bazel | 1 + src/workerd/api/node/i18n.c++ | 48 +++++ src/workerd/util/autogate.h | 7 +- 11 files changed, 898 insertions(+), 1 deletion(-) create mode 100644 src/rust/i18n/BUILD.bazel create mode 100644 src/rust/i18n/dispatch.rs create mode 100644 src/rust/i18n/error.rs create mode 100644 src/rust/i18n/lib.rs create mode 100644 src/rust/i18n/shim.c++ create mode 100644 src/rust/i18n/shim.h create mode 100644 src/rust/i18n/shim.rs diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel new file mode 100644 index 00000000000..8a9fd8b1b09 --- /dev/null +++ b/src/rust/i18n/BUILD.bazel @@ -0,0 +1,37 @@ +load("//:build/wd_cc_library.bzl", "wd_cc_library") +load("//:build/wd_rust_crate.bzl", "wd_rust_crate") + +# C++ shim exposing the ICU `ucnv_*` primitives and the simdutf conversion +# functions that `workerd::api::node::i18n::transcode` uses, so the Rust +# implementation of `transcode` (in `lib.rs` and friends) calls the exact same +# codecs as the C++ path in `src/workerd/api/node/i18n.c++` instead of +# reimplementing them. Declares its codec dependencies explicitly rather than +# relying on them arriving transitively through V8. +wd_cc_library( + name = "shim", + srcs = ["shim.c++"], + hdrs = ["shim.h"], + visibility = ["//visibility:public"], + deps = [ + "//src/rust/cxx:core", + "@capnp-cpp//src/kj", + "@com_googlesource_chromium_icu//:icu", + "@simdutf", + ], +) + +wd_rust_crate( + name = "i18n", + cxx_bridge_deps = [ + ":shim", + "//src/rust/jsg", + ], + cxx_bridge_src = "lib.rs", + link_deps = [":shim"], + test_deps = ["//src/rust/jsg-test"], + visibility = ["//visibility:public"], + deps = [ + "//src/rust/jsg", + "@crates_vendor//:thiserror", + ], +) diff --git a/src/rust/i18n/dispatch.rs b/src/rust/i18n/dispatch.rs new file mode 100644 index 00000000000..316157dac87 --- /dev/null +++ b/src/rust/i18n/dispatch.rs @@ -0,0 +1,347 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! The transcoding dispatch table and per-pair conversion logic, ported from +//! `i18n.c++`'s `TranscodeDefault` / `TranscodeLatin1ToUTF16` / +//! `TranscodeFromUTF16` / `TranscodeUTF16FromUTF8` / `TranscodeUTF8FromUTF16` +//! and the `switch` in `transcode()` that picks between them. All sizing, +//! substitute-character setup, empty-input handling, and truncation happens +//! here; [`crate::shim`] only forwards to the underlying ICU/simdutf calls. + +use crate::error::TranscodeError; +use crate::ffi::Encoding; +use crate::shim; +use crate::shim::Converter; + +/// An isolate has a 128MB memory limit. Mirrors `ISOLATE_LIMIT` in `i18n.c++`. +const ISOLATE_LIMIT: usize = 134_217_728; + +/// Transcodes `source` from `from` to `to`, matching the dispatch table built +/// by `i18n::transcode()` in `i18n.c++`. +pub fn transcode(source: &[u8], from: Encoding, to: Encoding) -> Result, TranscodeError> { + let result = match (from, to) { + (Encoding::Ascii | Encoding::Latin1, Encoding::Utf16Le) => { + transcode_latin1_to_utf16(source)? + } + (Encoding::Utf8, Encoding::Utf16Le) => transcode_utf16_from_utf8(source)?, + (Encoding::Utf16Le, Encoding::Utf8) => transcode_utf8_from_utf16(source)?, + (Encoding::Utf16Le, Encoding::Ascii | Encoding::Latin1) => { + transcode_from_utf16(source, to)? + } + // TranscodeDefault: identity conversions, UTF16LE -> UTF16LE, and every + // other (from, to) pair not overridden above -- mirrors the default + // `TranscodeImpl transcode_function = &TranscodeDefault;` in + // `i18n::transcode()`. + _ => transcode_default(source, from, to)?, + }; + result.ok_or(TranscodeError::UnableToTranscode) +} + +/// Mirrors `TranscodeDefault`: a plain ICU `ucnv_convertEx` between two +/// converters. Used for every (from, to) pair not handled by simdutf, plus +/// the UTF16LE -> UTF16LE identity conversion. +fn transcode_default( + source: &[u8], + from: Encoding, + to: Encoding, +) -> Result>, TranscodeError> { + let to_conv = Converter::open(to); + let substitute = "?".repeat(to_conv.min_char_size()); + to_conv.set_subst_chars(&substitute); + let from_conv = Converter::open(from); + + let limit = source + .len() + .checked_mul(to_conv.max_char_size()) + .ok_or(TranscodeError::SourceBufferTooLarge)?; + if limit == 0 { + return Ok(Some(Vec::new())); + } + // Workers are limited to 128MB so this isn't actually a realistic concern, + // but sanity check. + if limit > ISOLATE_LIMIT { + return Err(TranscodeError::SourceBufferTooLarge); + } + + let mut out = vec![0u8; limit]; + Ok( + shim::convert_ex(&to_conv, &from_conv, source, &mut out).map(|written| { + out.truncate(written); + out + }), + ) +} + +/// Mirrors `TranscodeLatin1ToUTF16`: widens ASCII/LATIN1 `source` into UTF-16 +/// via simdutf. Source bytes `0x80`-`0xFF` become U+0080-U+00FF rather than +/// being substituted -- this is the "latin1" path, taken even when `from` is +/// `ASCII` (see R8). +fn transcode_latin1_to_utf16(source: &[u8]) -> Result>, TranscodeError> { + let length_in_chars = source + .len() + .checked_mul(2) + .ok_or(TranscodeError::SourceBufferTooLarge)?; + // Workers are limited to 128MB so this isn't actually a realistic concern, + // but sanity check. + if length_in_chars > ISOLATE_LIMIT { + return Err(TranscodeError::SourceBufferTooLarge); + } + if length_in_chars == 0 { + return Ok(Some(Vec::new())); + } + + let mut dest = vec![0u8; length_in_chars]; + let actual_length = shim::convert_latin1_to_utf16(source, &mut dest); + // simdutf returns 0 for invalid input. + if actual_length == 0 { + return Ok(None); + } + dest.truncate(actual_length * 2); + Ok(Some(dest)) +} + +/// Mirrors `TranscodeFromUTF16`: `ucnv_fromUChars` from UTF-16LE `source` into +/// `to`'s encoding (ASCII or LATIN1). +fn transcode_from_utf16(source: &[u8], to: Encoding) -> Result>, TranscodeError> { + let to_conv = Converter::open(to); + let substitute = "?".repeat(to_conv.min_char_size()); + to_conv.set_subst_chars(&substitute); + + if !source.len().is_multiple_of(2) { + return Err(TranscodeError::OddUtf16leInput); + } + let utf16_len = source.len() / 2; + + let limit = utf16_len + .checked_mul(to_conv.max_char_size()) + .ok_or(TranscodeError::BufferTooLarge)?; + // Workers are limited to 128MB so this isn't actually a realistic concern, + // but sanity check. + if limit > ISOLATE_LIMIT { + return Err(TranscodeError::BufferTooLarge); + } + if limit == 0 { + return Ok(Some(Vec::new())); + } + + let mut dest = vec![0u8; limit]; + Ok( + shim::from_uchars(&to_conv, source, &mut dest).map(|written| { + dest.truncate(written); + dest + }), + ) +} + +/// Mirrors `TranscodeUTF16FromUTF8`: converts UTF-8 `source` to UTF-16LE via +/// simdutf. The output size comes from `simdutf::utf16_length_from_utf8`; when +/// that estimate is zero the result is an empty buffer, even for non-empty +/// input (see R8). +fn transcode_utf16_from_utf8(source: &[u8]) -> Result>, TranscodeError> { + let expected_utf16_length = shim::utf16_length_from_utf8(source); + // Workers are limited to 128MB so this isn't actually a realistic concern, + // but sanity check. + if expected_utf16_length > ISOLATE_LIMIT { + return Err(TranscodeError::ExpectedUtf16LengthTooLarge); + } + + let length_in_chars = expected_utf16_length + .checked_mul(2) + .ok_or(TranscodeError::ExpectedUtf16LengthTooLarge)?; + if length_in_chars == 0 { + return Ok(Some(Vec::new())); + } + + let mut dest = vec![0u8; length_in_chars]; + let actual_length = shim::convert_utf8_to_utf16le(source, &mut dest); + // simdutf returns 0 for invalid UTF-8 input. + if actual_length == 0 { + return Ok(None); + } + if actual_length != expected_utf16_length { + return Err(TranscodeError::Utf16LengthMismatch); + } + Ok(Some(dest)) +} + +/// Mirrors `TranscodeUTF8FromUTF16`: converts UTF-16LE `source` to UTF-8 via +/// simdutf, requiring the actual conversion length to equal the estimate from +/// `simdutf::utf8_length_from_utf16le`. +fn transcode_utf8_from_utf16(source: &[u8]) -> Result>, TranscodeError> { + if !source.len().is_multiple_of(2) { + return Err(TranscodeError::OddUtf16leInput); + } + + let expected_utf8_length = shim::utf8_length_from_utf16le(source); + // Workers are limited to 128MB so this isn't actually a realistic concern, + // but sanity check. + if expected_utf8_length > ISOLATE_LIMIT { + return Err(TranscodeError::ExpectedUtf8LengthTooLarge); + } + if expected_utf8_length == 0 { + return Ok(Some(Vec::new())); + } + + let mut dest = vec![0u8; expected_utf8_length]; + let actual_length = shim::convert_utf16le_to_utf8(source, &mut dest); + if actual_length != expected_utf8_length { + return Err(TranscodeError::Utf8LengthMismatch); + } + // Unreachable in practice: `actual_length` was just required to equal + // `expected_utf8_length`, which is nonzero at this point. This mirrors the + // equivalent dead branch in the C++ `TranscodeUTF8FromUTF16`, which checks + // `actual_length == 0` only *after* already requiring it to equal the + // (nonzero) expected length above -- so a simdutf failure (return value 0) + // surfaces as "Expected UTF8 length mismatch" here, not + // "Unable to transcode buffer", unlike every other conversion direction. + if actual_length == 0 { + return Ok(None); + } + Ok(Some(dest)) +} + +#[cfg(test)] +mod tests { + use jsg_test::Harness; + + use super::*; + + /// All four transcodable encodings, for exhaustively testing every pair. + const ENCODINGS: [Encoding; 4] = [ + Encoding::Ascii, + Encoding::Latin1, + Encoding::Utf8, + Encoding::Utf16Le, + ]; + + // `Harness::new()` initializes the V8 platform, which is what installs the + // embedded ICU data. ICU converter opens fail without it, even though + // these tests never create an isolate or run JavaScript. + fn init_icu() -> Harness { + Harness::new() + } + + #[test] + fn every_pair_round_trips_ascii_text() { + let _harness = init_icu(); + for &from in &ENCODINGS { + // UTF16LE source bytes must have even length; every other encoding + // is happy with plain ASCII bytes. + let source: &[u8] = if from == Encoding::Utf16Le { + &[0x48, 0x00, 0x69, 0x00] // "Hi" as UTF-16LE code units. + } else { + b"Hi" + }; + for &to in &ENCODINGS { + let result = transcode(source, from, to); + assert!(result.is_ok(), "{from:?} -> {to:?} failed: {result:?}"); + } + } + } + + #[test] + fn every_pair_empty_input_is_empty_output() { + let _harness = init_icu(); + for &from in &ENCODINGS { + for &to in &ENCODINGS { + let result = transcode(&[], from, to); + assert_eq!( + result.as_deref(), + Ok([].as_slice()), + "{from:?} -> {to:?} on empty input should be empty, got {result:?}" + ); + } + } + } + + #[test] + fn identity_pairs_round_trip() { + let _harness = init_icu(); + for &encoding in &ENCODINGS { + let source = b"identity"; + let result = transcode(source, encoding, encoding).unwrap(); + if encoding == Encoding::Utf16Le { + // `source` is treated as raw UTF-16LE code units, so identity is + // not a byte-for-byte passthrough of ASCII text; just check it + // succeeds (covered by `every_pair_round_trips_ascii_text`). + continue; + } + assert_eq!(result, source); + } + } + + #[test] + fn unmappable_characters_become_question_marks() { + let _harness = init_icu(); + // '☕' (U+2615, HOT BEVERAGE) has no representation in ASCII or + // LATIN1/windows-1252 -- the same character the existing C++ path is + // exercised with in `buffer-nodejs-test.js`'s `transcodeTest`. + let source = "☕".as_bytes(); + assert_eq!( + transcode(source, Encoding::Utf8, Encoding::Ascii).unwrap(), + b"?" + ); + assert_eq!( + transcode(source, Encoding::Utf8, Encoding::Latin1).unwrap(), + b"?" + ); + } + + #[test] + fn ascii_to_utf16le_widens_high_bytes_via_latin1_path() { + let _harness = init_icu(); + // R8: ASCII -> UTF16LE takes the latin1 path, so a high byte is + // widened to U+00FF rather than substituted with '?'. + let result = transcode(&[0xff], Encoding::Ascii, Encoding::Utf16Le).unwrap(); + assert_eq!(result, vec![0xff, 0x00]); + } + + #[test] + fn utf8_continuation_byte_only_input_yields_empty_utf16le() { + let _harness = init_icu(); + // R8: the UTF8 -> UTF16LE size estimate is zero for input consisting + // only of continuation bytes, so the result is empty rather than an + // error, even though the input is non-empty. + let result = transcode(&[0x80], Encoding::Utf8, Encoding::Utf16Le).unwrap(); + assert!(result.is_empty()); + } + + #[test] + fn invalid_utf8_to_utf16le_is_unable_to_transcode() { + let _harness = init_icu(); + let result = transcode(&[0x61, 0xc3], Encoding::Utf8, Encoding::Utf16Le); + assert_eq!(result, Err(TranscodeError::UnableToTranscode)); + } + + #[test] + fn odd_length_utf16le_to_utf8_is_rejected() { + let _harness = init_icu(); + let result = transcode(&[0x61], Encoding::Utf16Le, Encoding::Utf8); + assert_eq!(result, Err(TranscodeError::OddUtf16leInput)); + } + + #[test] + fn odd_length_utf16le_to_latin1_is_rejected() { + let _harness = init_icu(); + let result = transcode(&[0x61], Encoding::Utf16Le, Encoding::Latin1); + assert_eq!(result, Err(TranscodeError::OddUtf16leInput)); + } + + #[test] + fn unpaired_surrogate_utf16le_to_utf8() { + let _harness = init_icu(); + // U+D800, an unpaired high surrogate, encoded as UTF-16LE bytes. + let source = [0x00, 0xd8]; + // simdutf treats this as invalid input; whatever it decides (empty + // output or an error) must not panic, and must match what the C++ + // path's `simdutf::utf8_length_from_utf16le` / + // `simdutf::convert_utf16le_to_utf8` pair would produce, since both + // implementations call the exact same simdutf functions. + let result = transcode(&source, Encoding::Utf16Le, Encoding::Utf8); + match result { + Ok(bytes) => assert!(bytes.is_empty()), + Err(err) => assert_eq!(err, TranscodeError::Utf8LengthMismatch), + } + } +} diff --git a/src/rust/i18n/error.rs b/src/rust/i18n/error.rs new file mode 100644 index 00000000000..5f384361437 --- /dev/null +++ b/src/rust/i18n/error.rs @@ -0,0 +1,42 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +use thiserror::Error; + +/// Errors from the Rust `i18n::transcode` implementation ([`crate::dispatch`]). +/// +/// Every message matches the corresponding `JSG_REQUIRE` / `JSG_FAIL_REQUIRE` +/// string in `workerd::api::node::i18n::transcode` +/// (`src/workerd/api/node/i18n.c++`) verbatim, so gate-on and gate-off are +/// indistinguishable to JavaScript. `"Invalid encoding passed to transcode"` +/// is not a variant here: it is raised by the C++ `fromImpl` conversion before +/// the Rust entry point is ever called (see `i18n.c++`), since the bridge +/// `Encoding` enum can only represent the four transcodable encodings. +#[derive(Debug, Error, PartialEq, Eq)] +pub enum TranscodeError { + #[error("Source buffer is too large to transcode")] + SourceBufferTooLarge, + #[error("Buffer is too large to transcode")] + BufferTooLarge, + #[error("Expected UTF-16le length is too large to transcode")] + ExpectedUtf16LengthTooLarge, + #[error("Expected UTF-8 length is too large to transcode")] + ExpectedUtf8LengthTooLarge, + #[error("UTF-16le input size should be multiple of 2")] + OddUtf16leInput, + #[error("Expected UTF16 length mismatch")] + Utf16LengthMismatch, + #[error("Expected UTF8 length mismatch")] + Utf8LengthMismatch, + #[error("Unable to transcode buffer")] + UnableToTranscode, +} + +impl From for jsg::Error { + fn from(value: TranscodeError) -> Self { + // All of these are plain JS `Error`s, matching the `JSG_REQUIRE(..., Error, ...)` + // calls they replace. + Self::new_error(value.to_string()) + } +} diff --git a/src/rust/i18n/lib.rs b/src/rust/i18n/lib.rs new file mode 100644 index 00000000000..645d9699fd1 --- /dev/null +++ b/src/rust/i18n/lib.rs @@ -0,0 +1,114 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! Rust port of `workerd::api::node::i18n::transcode` +//! (`src/workerd/api/node/i18n.c++`), the engine behind `node:buffer`'s +//! `transcode()`. Selected at runtime by the `NODEJS_I18N_RUST` autogate; when +//! the gate is off, the C++ implementation is used instead. The two paths are +//! byte-for-byte and error-message identical by construction: [`dispatch`] +//! ports the C++ dispatch/sizing/truncation logic to Rust, while [`shim`] +//! calls the exact same ICU and simdutf primitives the C++ path uses, through +//! the C++ shim in `shim.h` / `shim.c++`. + +use jsg::Lock; +use jsg::ToJS; +use jsg::v8; + +mod dispatch; +mod error; +mod shim; + +#[cxx::bridge(namespace = "workerd::rust::i18n")] +mod ffi { + /// The four encodings `i18n::transcode` supports. Mirrors the + /// transcodable subset of `workerd::api::node::Encoding` + /// (`src/workerd/api/node/i18n.h`). `src/workerd/api/node/i18n.c++` maps + /// into this type through a `fromImpl` overload that rejects the + /// non-transcodable `BASE64`, `BASE64URL`, and `HEX` variants before ever + /// calling into Rust (see R10). + #[derive(Debug, PartialEq, Eq, Copy, Clone)] + #[repr(u8)] + enum Encoding { + Ascii, + Latin1, + Utf8, + Utf16Le, + } + + unsafe extern "C++" { + include!("workerd/rust/i18n/shim.h"); + + type Converter; + + fn open_converter(name: &str) -> UniquePtr; + fn max_char_size(self: &Converter) -> usize; + fn min_char_size(self: &Converter) -> usize; + fn set_subst_chars(self: &Converter, substitute: &str); + + fn convert_ex(to: &Converter, from: &Converter, source: &[u8], target: &mut [u8]) -> i64; + fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> i64; + + fn convert_latin1_to_utf16(source: &[u8], target: &mut [u8]) -> usize; + fn utf16_length_from_utf8(source: &[u8]) -> usize; + fn convert_utf8_to_utf16le(source: &[u8], target: &mut [u8]) -> usize; + fn utf8_length_from_utf16le(source: &[u8]) -> usize; + fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize; + } + + #[namespace = "workerd::rust::jsg"] + unsafe extern "C++" { + include!("workerd/rust/jsg/ffi.h"); + include!("workerd/rust/jsg/v8.rs.h"); + + type Isolate = jsg::v8::ffi::Isolate; + type MaybeLocal = jsg::v8::ffi::MaybeLocal; + } + + extern "Rust" { + /// Transcodes `source` from `from_encoding` to `to_encoding`, matching + /// `workerd::api::node::i18n::transcode`. Returns a `MaybeLocal` + /// naming a `Uint8Array`, or an empty `MaybeLocal` with a JS exception + /// already scheduled on `isolate` if transcoding fails. + /// + /// # Safety + /// `isolate` must be a valid pointer to a locked `v8::Isolate` with an + /// active `HandleScope`. + unsafe fn transcode( + isolate: *mut Isolate, + source: &[u8], + from_encoding: Encoding, + to_encoding: Encoding, + ) -> MaybeLocal; + } +} + +/// # Safety +/// `isolate` must be a valid pointer to a locked `v8::Isolate` with an active +/// `HandleScope`. +unsafe fn transcode( + isolate: *mut ffi::Isolate, + source: &[u8], + from_encoding: ffi::Encoding, + to_encoding: ffi::Encoding, +) -> ffi::MaybeLocal { + // SAFETY: forwarded from this function's own safety contract -- the C++ + // caller (`i18n::transcode` in `i18n.c++`) guarantees `isolate` is valid, + // locked, and has an active HandleScope. + let mut lock = unsafe { Lock::from_isolate_ptr(isolate) }; + match dispatch::transcode(source, from_encoding, to_encoding) { + Ok(bytes) => { + let local: v8::Local = bytes.to_js(&mut lock); + // SAFETY: `local` was just created in the isolate's active + // HandleScope; its FFI representation is handed to the C++ + // caller, which reconstitutes it via `maybe_local_from_ffi` and + // immediately passes it through `jsg::check()`. + let raw = unsafe { local.into_ffi() }; + ffi::MaybeLocal { ptr: raw.ptr } + } + Err(err) => { + lock.throw_exception(&err.into()); + ffi::MaybeLocal { ptr: 0 } + } + } +} diff --git a/src/rust/i18n/shim.c++ b/src/rust/i18n/shim.c++ new file mode 100644 index 00000000000..4564e034803 --- /dev/null +++ b/src/rust/i18n/shim.c++ @@ -0,0 +1,101 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#include "shim.h" + +#include "simdutf.h" + +#include + +#include + +#include + +namespace workerd::rust::i18n { + +Converter::~Converter() noexcept { + if (conv_ != nullptr) { + ucnv_close(conv_); + } +} + +size_t Converter::max_char_size() const { + return static_cast(ucnv_getMaxCharSize(conv_)); +} + +size_t Converter::min_char_size() const { + return static_cast(ucnv_getMinCharSize(conv_)); +} + +void Converter::set_subst_chars(::rust::Str substitute) const { + if (substitute.size() == 0) return; + UErrorCode status = U_ZERO_ERROR; + ucnv_setSubstChars(conv_, substitute.data(), static_cast(substitute.size()), &status); + // Unreachable in practice: the substitute strings this module sets are always + // short, valid ASCII ('?' repeated `minCharSize()` times), so ICU never + // rejects them for any of the four transcodable encodings. + KJ_REQUIRE(U_SUCCESS(status), "Setting ICU substitute characters failed"); +} + +std::unique_ptr open_converter(::rust::Str name) { + UErrorCode status = U_ZERO_ERROR; + std::string nameStr(name.data(), name.size()); + auto* conv = ucnv_open(nameStr.c_str(), &status); + // Unreachable in practice: this module only ever opens converters for the + // four fixed, always-valid ICU encoding names ("us-ascii", "iso8859-1", + // "utf-8", "utf16le"). + KJ_REQUIRE(U_SUCCESS(status), "Failed to initialize converter"); + return std::make_unique(conv); +} + +int64_t convert_ex(const Converter& to, + const Converter& from, + ::rust::Slice source, + ::rust::Slice target) { + char* const targetStart = reinterpret_cast(target.data()); + char* targetPtr = targetStart; + const char* sourcePtr = reinterpret_cast(source.data()); + UErrorCode status = U_ZERO_ERROR; + ucnv_convertEx(to.conv_, from.conv_, &targetPtr, targetStart + target.size(), &sourcePtr, + sourcePtr + source.size(), nullptr, nullptr, nullptr, nullptr, true, true, &status); + if (U_FAILURE(status)) return -1; + return static_cast(targetPtr - targetStart); +} + +int64_t from_uchars( + const Converter& to, ::rust::Slice source, ::rust::Slice target) { + UErrorCode status = U_ZERO_ERROR; + auto len = ucnv_fromUChars(to.conv_, reinterpret_cast(target.data()), + static_cast(target.size()), reinterpret_cast(source.data()), + static_cast(source.size() / sizeof(UChar)), &status); + if (U_FAILURE(status)) return -1; + return static_cast(len); +} + +size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target) { + return simdutf::convert_latin1_to_utf16(reinterpret_cast(source.data()), + source.size(), reinterpret_cast(target.data())); +} + +size_t utf16_length_from_utf8(::rust::Slice source) { + return simdutf::utf16_length_from_utf8( + reinterpret_cast(source.data()), source.size()); +} + +size_t convert_utf8_to_utf16le(::rust::Slice source, ::rust::Slice target) { + return simdutf::convert_utf8_to_utf16le(reinterpret_cast(source.data()), + source.size(), reinterpret_cast(target.data())); +} + +size_t utf8_length_from_utf16le(::rust::Slice source) { + return simdutf::utf8_length_from_utf16le( + reinterpret_cast(source.data()), source.size() / sizeof(char16_t)); +} + +size_t convert_utf16le_to_utf8(::rust::Slice source, ::rust::Slice target) { + return simdutf::convert_utf16le_to_utf8(reinterpret_cast(source.data()), + source.size() / sizeof(char16_t), reinterpret_cast(target.data())); +} + +} // namespace workerd::rust::i18n diff --git a/src/rust/i18n/shim.h b/src/rust/i18n/shim.h new file mode 100644 index 00000000000..d9433f3185a --- /dev/null +++ b/src/rust/i18n/shim.h @@ -0,0 +1,83 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 +#pragma once + +// C++ shim exposing the ICU `ucnv_*` primitives and the simdutf conversion +// functions used by `workerd::api::node::i18n::transcode` +// (`src/workerd/api/node/i18n.c++`), so the Rust implementation in `lib.rs` +// calls the exact same codecs as the C++ path rather than reimplementing +// them. + +#include + +#include +#include +#include + +// ICU's converter handle (``). Kept opaque here; the full ICU +// header is only needed in shim.c++. +struct UConverter; + +namespace workerd::rust::i18n { + +// RAII wrapper around an ICU `UConverter*`, opened by `open_converter()` for +// one of the four transcodable encodings. Exposed to Rust as an opaque C++ +// type behind `UniquePtr`, so `ucnv_close()` runs in the +// destructor and the converter's lifetime is owned entirely by C++: even if +// Rust code holding the `UniquePtr` panics, unwinding still drops it and runs +// the destructor, unlike a raw `UConverter*` smuggled across the FFI boundary, +// which a panic could leak. +class Converter { + public: + explicit Converter(UConverter* conv) noexcept: conv_(conv) {} + ~Converter() noexcept; + Converter(const Converter&) = delete; + Converter& operator=(const Converter&) = delete; + + size_t max_char_size() const; + size_t min_char_size() const; + void set_subst_chars(::rust::Str substitute) const; + + private: + UConverter* conv_; + + friend int64_t convert_ex(const Converter& to, + const Converter& from, + ::rust::Slice source, + ::rust::Slice target); + friend int64_t from_uchars( + const Converter& to, ::rust::Slice source, ::rust::Slice target); +}; + +// Opens an ICU converter for `name` (an ICU encoding name, e.g. "us-ascii"). +std::unique_ptr open_converter(::rust::Str name); + +// `ucnv_convertEx()`-based conversion between two ICU converters, mirroring +// `TranscodeDefault` in `i18n.c++`. `source` and `target` are raw bytes. +// Returns the number of bytes written to `target`, or -1 if ICU reports +// failure. +int64_t convert_ex(const Converter& to, + const Converter& from, + ::rust::Slice source, + ::rust::Slice target); + +// `ucnv_fromUChars()`-based conversion from UTF-16LE, mirroring +// `TranscodeFromUTF16` in `i18n.c++`. `source` holds UTF-16LE code units as +// raw bytes (its length must be even); `target` is raw output bytes. Returns +// the number of bytes written to `target`, or -1 if ICU reports failure. +int64_t from_uchars( + const Converter& to, ::rust::Slice source, ::rust::Slice target); + +// simdutf wrappers, mirroring the four `simdutf::*` calls in `i18n.c++`. +// Buffers holding UTF-16LE code units are passed as raw bytes and cast to +// `char16_t*` internally, exactly as the C++ path does via +// `JsUint8Array::asArrayPtr()`. + +size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target); +size_t utf16_length_from_utf8(::rust::Slice source); +size_t convert_utf8_to_utf16le(::rust::Slice source, ::rust::Slice target); +size_t utf8_length_from_utf16le(::rust::Slice source); +size_t convert_utf16le_to_utf8(::rust::Slice source, ::rust::Slice target); + +} // namespace workerd::rust::i18n diff --git a/src/rust/i18n/shim.rs b/src/rust/i18n/shim.rs new file mode 100644 index 00000000000..61fc10f8a2b --- /dev/null +++ b/src/rust/i18n/shim.rs @@ -0,0 +1,113 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! Thin Rust wrappers around the ICU/simdutf primitives exposed by the C++ +//! shim (`shim.h` / `shim.c++`). All the transcoding *logic* -- dispatch, +//! sizing, substitute-character setup, truncation -- lives in [`crate::dispatch`]; +//! this module only adapts the shim's C-ish sentinel-value return conventions +//! (`-1` for ICU failure, `0` for simdutf failure) into idiomatic `Option`s. + +use crate::ffi; + +/// An open ICU converter for one of the four transcodable encodings. +/// +/// Wraps a `cxx::UniquePtr`: the underlying `UConverter*` and +/// its `ucnv_close()` teardown are owned entirely by the C++ shim, so the +/// converter is torn down correctly even if Rust code holding it panics -- +/// unlike a raw `UConverter*` smuggled across the FFI boundary, which a panic +/// could leak. +pub struct Converter(cxx::UniquePtr); + +/// Returns the ICU converter name for a transcodable encoding, matching +/// `getEncodingName()` in `i18n.c++`. +fn icu_name(encoding: ffi::Encoding) -> &'static str { + match encoding { + ffi::Encoding::Ascii => "us-ascii", + ffi::Encoding::Latin1 => "iso8859-1", + ffi::Encoding::Utf16Le => "utf16le", + ffi::Encoding::Utf8 => "utf-8", + // The bridge `Encoding` enum has exactly these four variants (R4); any + // other discriminant would mean the C++/Rust enum definitions have + // drifted out of sync. + _ => unreachable!("Encoding has exactly four variants"), + } +} + +impl Converter { + /// Opens an ICU converter for `encoding`. + /// + /// Never fails in practice -- the four encoding names above are always + /// valid ICU converter names -- so a shim-side open failure (see + /// `shim.c++`) is treated as an unrecoverable invariant violation rather + /// than a catchable error, matching how unreachable `KJ_ASSERT`-style + /// conditions are handled elsewhere in this codebase. + pub fn open(encoding: ffi::Encoding) -> Self { + Self(ffi::open_converter(icu_name(encoding))) + } + + pub fn max_char_size(&self) -> usize { + self.0.max_char_size() + } + + pub fn min_char_size(&self) -> usize { + self.0.min_char_size() + } + + /// Sets the converter's substitute character sequence, used in place of + /// unmappable characters during conversion. + pub fn set_subst_chars(&self, substitute: &str) { + self.0.set_subst_chars(substitute); + } +} + +/// Converts `source` from `from`'s encoding to `to`'s encoding via ICU's +/// `ucnv_convertEx`, mirroring `TranscodeDefault` in `i18n.c++`. Returns the +/// number of bytes written to `target`, or `None` if ICU reports failure. +pub fn convert_ex( + to: &Converter, + from: &Converter, + source: &[u8], + target: &mut [u8], +) -> Option { + usize::try_from(ffi::convert_ex(&to.0, &from.0, source, target)).ok() +} + +/// Converts UTF-16LE `source` (as raw bytes) to `to`'s encoding via ICU's +/// `ucnv_fromUChars`, mirroring `TranscodeFromUTF16` in `i18n.c++`. Returns +/// the number of bytes written to `target`, or `None` if ICU reports failure. +pub fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> Option { + usize::try_from(ffi::from_uchars(&to.0, source, target)).ok() +} + +/// Widens Latin-1 `source` into UTF-16 (written to `target` as raw bytes), +/// mirroring `simdutf::convert_latin1_to_utf16`. Returns the number of +/// `char16_t` units written. +pub fn convert_latin1_to_utf16(source: &[u8], target: &mut [u8]) -> usize { + ffi::convert_latin1_to_utf16(source, target) +} + +/// Estimates the UTF-16 length (in `char16_t` units) of UTF-8 `source`, +/// mirroring `simdutf::utf16_length_from_utf8`. +pub fn utf16_length_from_utf8(source: &[u8]) -> usize { + ffi::utf16_length_from_utf8(source) +} + +/// Converts UTF-8 `source` to UTF-16LE (written to `target` as raw bytes), +/// mirroring `simdutf::convert_utf8_to_utf16le`. Returns the number of +/// `char16_t` units written, or `0` on invalid UTF-8. +pub fn convert_utf8_to_utf16le(source: &[u8], target: &mut [u8]) -> usize { + ffi::convert_utf8_to_utf16le(source, target) +} + +/// Estimates the UTF-8 length (in bytes) of UTF-16LE `source` (as raw bytes), +/// mirroring `simdutf::utf8_length_from_utf16le`. +pub fn utf8_length_from_utf16le(source: &[u8]) -> usize { + ffi::utf8_length_from_utf16le(source) +} + +/// Converts UTF-16LE `source` (as raw bytes) to UTF-8, mirroring +/// `simdutf::convert_utf16le_to_utf8`. Returns the number of bytes written. +pub fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize { + ffi::convert_utf16le_to_utf8(source, target) +} diff --git a/src/rust/jsg/ffi-inl.h b/src/rust/jsg/ffi-inl.h index 70eb9af97bd..9cc65aee465 100644 --- a/src/rust/jsg/ffi-inl.h +++ b/src/rust/jsg/ffi-inl.h @@ -50,6 +50,12 @@ inline MaybeLocal maybe_local_to_ffi(v8::MaybeLocal value) { return MaybeLocal{result}; } +template +inline v8::MaybeLocal maybe_local_from_ffi(MaybeLocal&& value) { + auto ptr_void = reinterpret_cast(&value.ptr); + return *reinterpret_cast*>(ptr_void); +} + // Global // // ffi::Global stores only the strong v8::Global in `ptr`. diff --git a/src/workerd/api/node/BUILD.bazel b/src/workerd/api/node/BUILD.bazel index cb5b8db9050..c86e3599acf 100644 --- a/src/workerd/api/node/BUILD.bazel +++ b/src/workerd/api/node/BUILD.bazel @@ -95,6 +95,7 @@ wd_cc_library( ], implementation_deps = [ "//src/rust/cxx-integration", + "//src/rust/i18n", "//src/rust/net", "@ada-url", "@nbytes", diff --git a/src/workerd/api/node/i18n.c++ b/src/workerd/api/node/i18n.c++ index a895a7c72c0..1b270023286 100644 --- a/src/workerd/api/node/i18n.c++ +++ b/src/workerd/api/node/i18n.c++ @@ -8,7 +8,11 @@ #include "simdutf.h" #include +#include +#include +#include +#include #include #include #include @@ -18,6 +22,40 @@ namespace workerd::api::node { +namespace rust_i18n = ::workerd::rust::i18n; + +// Maps the C++ Encoding to the Rust bridge enum, following the kj-rs +// convert.h idiom (see `NODEJS_EXCEPTIONS_RUST`'s equivalent in +// exceptions.c++), so callers use `kj::from(value)`. It +// is `static` (rather than in an anonymous namespace) because Clang's ADL +// does not consider unnamed-namespace functions, and ADL is how +// `kj::from` locates this overload. The switch has no `default:` arm +// so that enum drift between the two `Encoding` types is a compile error. +// +// `BASE64`, `BASE64URL`, and `HEX` are not transcodable (see +// `i18n::canBeTranscoded`); rejecting them here, for both `fromEncoding` and +// `toEncoding`, is a deliberate divergence from the C++ dispatch below, which +// only checks `fromEncoding` and would reach `KJ_UNREACHABLE` for a +// non-transcodable `toEncoding`. Neither is reachable from JavaScript because +// `BufferUtil::transcode` validates both encodings first. +static rust_i18n::Encoding fromImpl(rust_i18n::Encoding*, Encoding encoding) { + switch (encoding) { + case Encoding::ASCII: + return rust_i18n::Encoding::Ascii; + case Encoding::LATIN1: + return rust_i18n::Encoding::Latin1; + case Encoding::UTF8: + return rust_i18n::Encoding::Utf8; + case Encoding::UTF16LE: + return rust_i18n::Encoding::Utf16Le; + case Encoding::BASE64: + case Encoding::BASE64URL: + case Encoding::HEX: + JSG_FAIL_REQUIRE(Error, "Invalid encoding passed to transcode"); + } + KJ_UNREACHABLE; +} + namespace i18n { namespace { @@ -240,6 +278,16 @@ void Converter::setSubstituteChars(kj::StringPtr sub) { jsg::JsUint8Array transcode( jsg::Lock& js, kj::ArrayPtr source, Encoding fromEncoding, Encoding toEncoding) { + if (util::Autogate::isEnabled(util::AutogateKey::NODEJS_I18N_RUST)) { + auto rustFrom = kj::from(fromEncoding); + auto rustTo = kj::from(toEncoding); + auto maybeLocal = + rust_i18n::transcode(js.v8Isolate, source.as(), rustFrom, rustTo); + auto local = + jsg::check(::workerd::rust::jsg::maybe_local_from_ffi(kj::mv(maybeLocal))); + return jsg::JsUint8Array(local); + } + TranscodeImpl transcode_function = &TranscodeDefault; switch (fromEncoding) { case Encoding::ASCII: diff --git a/src/workerd/util/autogate.h b/src/workerd/util/autogate.h index 9f8b3eb5693..2c37c3c5c0e 100644 --- a/src/workerd/util/autogate.h +++ b/src/workerd/util/autogate.h @@ -106,7 +106,12 @@ namespace workerd::util { /* Allow a Socket to be transferred over JS RPC. When disabled, serializing a Socket fails as \ though the type were not serializable at all, and an incoming transferred Socket is \ rejected. */ \ - V(SOCKET_RPC_TRANSFER) + V(SOCKET_RPC_TRANSFER) \ + /* When enabled, the Node.js `i18n` transcode implementation (api::node \ + i18n::transcode) is provided by the Rust implementation (src/rust/i18n) \ + instead of the C++ implementation. The C++ implementation is retained \ + for rollback.*/ \ + V(NODEJS_I18N_RUST) // clang-format on // -------------------------------------------------------------------------------------- From 58f41ea1408336aeecce4b0574bf3c63257d3873 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Wed, 12 Aug 2026 12:10:32 -0500 Subject: [PATCH 02/10] spec(rust-i18n): round 1 refactor Point the src/rust/i18n shim's ICU dependency at :icuuc instead of the umbrella :icu target, per R7 (the shim only uses ucnv_* primitives from ICU's common library, not icui18n). The bazel/BUILD.icu file vendored via V8's patch set only marks :icu and :icudata public; :icuuc is package-private. Add a small new V8 patch (0039) granting :icuuc public visibility, following the existing precedent of patch 0027 doing the same for :icudata. Verified: just clippy i18n is clean; bazel test //src/rust/i18n:i18n_test, //src/workerd/api/node/tests:buffer-nodejs-test@, and @all-autogates all pass; full bazel test //... passes 1623/1623, observed running fresh (no cached results). --- build/deps/v8.MODULE.bazel | 1 + ...-icuuc-bazel-target-publicly-visible.patch | 22 +++++++++++++++++++ src/rust/i18n/BUILD.bazel | 2 +- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch diff --git a/build/deps/v8.MODULE.bazel b/build/deps/v8.MODULE.bazel index de1dc53d42f..7c557806f6a 100644 --- a/build/deps/v8.MODULE.bazel +++ b/build/deps/v8.MODULE.bazel @@ -62,6 +62,7 @@ PATCHES = [ "0037-Fix-CFunction-MemorySpan-declarations-on-Windows.patch", "0038-Properly-depend-on-llvm-libc.patch", "0039-wasm-memory.discard-prototype-for-the-memory-control.patch", + "0040-Make-icuuc-bazel-target-publicly-visible.patch", ] http_archive( diff --git a/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch b/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch new file mode 100644 index 00000000000..d63cfcb3644 --- /dev/null +++ b/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch @@ -0,0 +1,22 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Logan Gatlin +Date: Wed, 12 Aug 2026 00:00:00 +0000 +Subject: Make icuuc bazel target publicly visible + +Only the `icu` umbrella target (which also pulls in `icui18n`) is public. +Consumers that need just the ICU common library -- e.g. a converter-only +shim -- should be able to depend on `icuuc` directly instead of picking up +`icui18n` transitively. + +diff --git a/bazel/BUILD.icu b/bazel/BUILD.icu +index de8e20ac3ad50fe2199d3b56753e655d1355b19f..1111111111111111111111111111111111111111 100644 +--- a/bazel/BUILD.icu ++++ b/bazel/BUILD.icu +@@ -15,6 +15,7 @@ cc_library( + "source/common/**/*.h", + "source/common/**/*.cpp", + ]), ++ visibility = ["//visibility:public"], + copts = select({ + "@platforms//os:windows": [ + "/wd4005", # Macro redefinition. diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index 8a9fd8b1b09..405c7ef195b 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -15,7 +15,7 @@ wd_cc_library( deps = [ "//src/rust/cxx:core", "@capnp-cpp//src/kj", - "@com_googlesource_chromium_icu//:icu", + "@com_googlesource_chromium_icu//:icuuc", "@simdutf", ], ) From c49bc7a6311a3d35e8e579f2e9e6121e0bce8a4e Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Tue, 18 Aug 2026 16:15:34 -0500 Subject: [PATCH 03/10] restructure --- src/rust/i18n/BUILD.bazel | 12 +- src/rust/i18n/dispatch.rs | 420 +++++++++++++++++++--------------- src/rust/i18n/error.rs | 42 ++-- src/rust/i18n/lib.rs | 127 +++++----- src/rust/i18n/shim.c++ | 27 +-- src/rust/i18n/shim.h | 20 +- src/rust/i18n/shim.rs | 55 +++-- src/rust/jsg/ffi-inl.h | 6 - src/workerd/api/node/i18n.c++ | 40 ++-- 9 files changed, 409 insertions(+), 340 deletions(-) diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index 405c7ef195b..6e1f8ef25ab 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -22,16 +22,12 @@ wd_cc_library( wd_rust_crate( name = "i18n", - cxx_bridge_deps = [ - ":shim", - "//src/rust/jsg", - ], + cxx_bridge_deps = [":shim"], cxx_bridge_src = "lib.rs", link_deps = [":shim"], + # `jsg-test`'s harness initializes the V8 platform, which is what installs + # the embedded ICU data the unit tests need in order to open converters. test_deps = ["//src/rust/jsg-test"], visibility = ["//visibility:public"], - deps = [ - "//src/rust/jsg", - "@crates_vendor//:thiserror", - ], + deps = ["@crates_vendor//:thiserror"], ) diff --git a/src/rust/i18n/dispatch.rs b/src/rust/i18n/dispatch.rs index 316157dac87..75e5ffddd69 100644 --- a/src/rust/i18n/dispatch.rs +++ b/src/rust/i18n/dispatch.rs @@ -2,203 +2,244 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! The transcoding dispatch table and per-pair conversion logic, ported from -//! `i18n.c++`'s `TranscodeDefault` / `TranscodeLatin1ToUTF16` / -//! `TranscodeFromUTF16` / `TranscodeUTF16FromUTF8` / `TranscodeUTF8FromUTF16` -//! and the `switch` in `transcode()` that picks between them. All sizing, -//! substitute-character setup, empty-input handling, and truncation happens -//! here; [`crate::shim`] only forwards to the underlying ICU/simdutf calls. +//! Encoding-pair dispatch and the conversion logic behind each pair. +//! +//! A transcode runs in two steps. [`Transcoder::new`] validates `source`, +//! picks the conversion to perform, and computes +//! [`Transcoder::dest_len`] -- the exact size of the destination buffer that +//! conversion needs. The caller allocates a buffer of that size and passes it +//! to [`Transcoder::transcode_into`], which fills it and returns the number of +//! bytes actually written. +//! +//! The destination is sized for the worst case, so the write is often shorter +//! than the buffer; the caller is expected to narrow its view of the buffer to +//! the returned length rather than shrink the buffer itself. +//! +//! All sizing, validation, substitute-character setup, and length checking +//! happens here; [`crate::shim`] only forwards to the underlying ICU and +//! simdutf calls. use crate::error::TranscodeError; use crate::ffi::Encoding; use crate::shim; use crate::shim::Converter; -/// An isolate has a 128MB memory limit. Mirrors `ISOLATE_LIMIT` in `i18n.c++`. -const ISOLATE_LIMIT: usize = 134_217_728; +/// The memory limit of an isolate, and thus the ceiling on any single +/// destination buffer. Conversions are rejected rather than attempted above +/// this size. +const ISOLATE_LIMIT: usize = 128 * 1024 * 1024; -/// Transcodes `source` from `from` to `to`, matching the dispatch table built -/// by `i18n::transcode()` in `i18n.c++`. -pub fn transcode(source: &[u8], from: Encoding, to: Encoding) -> Result, TranscodeError> { - let result = match (from, to) { - (Encoding::Ascii | Encoding::Latin1, Encoding::Utf16Le) => { - transcode_latin1_to_utf16(source)? - } - (Encoding::Utf8, Encoding::Utf16Le) => transcode_utf16_from_utf8(source)?, - (Encoding::Utf16Le, Encoding::Utf8) => transcode_utf8_from_utf16(source)?, - (Encoding::Utf16Le, Encoding::Ascii | Encoding::Latin1) => { - transcode_from_utf16(source, to)? - } - // TranscodeDefault: identity conversions, UTF16LE -> UTF16LE, and every - // other (from, to) pair not overridden above -- mirrors the default - // `TranscodeImpl transcode_function = &TranscodeDefault;` in - // `i18n::transcode()`. - _ => transcode_default(source, from, to)?, - }; - result.ok_or(TranscodeError::UnableToTranscode) +/// A validated, sized transcode, ready to run. +pub struct Transcoder { + conversion: Conversion, + dest_len: usize, } -/// Mirrors `TranscodeDefault`: a plain ICU `ucnv_convertEx` between two -/// converters. Used for every (from, to) pair not handled by simdutf, plus -/// the UTF16LE -> UTF16LE identity conversion. -fn transcode_default( - source: &[u8], - from: Encoding, - to: Encoding, -) -> Result>, TranscodeError> { - let to_conv = Converter::open(to); - let substitute = "?".repeat(to_conv.min_char_size()); - to_conv.set_subst_chars(&substitute); - let from_conv = Converter::open(from); - - let limit = source - .len() - .checked_mul(to_conv.max_char_size()) - .ok_or(TranscodeError::SourceBufferTooLarge)?; - if limit == 0 { - return Ok(Some(Vec::new())); - } - // Workers are limited to 128MB so this isn't actually a realistic concern, - // but sanity check. - if limit > ISOLATE_LIMIT { - return Err(TranscodeError::SourceBufferTooLarge); - } - - let mut out = vec![0u8; limit]; - Ok( - shim::convert_ex(&to_conv, &from_conv, source, &mut out).map(|written| { - out.truncate(written); - out - }), - ) +/// The conversion [`Transcoder::transcode_into`] will perform, along with any +/// ICU converters [`Transcoder::new`] had to open to size the destination. +enum Conversion { + /// ICU `ucnv_convertEx` between two converters. Handles every pair the + /// simdutf conversions below do not, including all four identity pairs. + ConvertEx { to: Converter, from: Converter }, + /// simdutf Latin-1 to UTF-16. + Latin1ToUtf16, + /// ICU `ucnv_fromUChars` from UTF-16LE. + FromUtf16 { to: Converter }, + /// simdutf UTF-8 to UTF-16LE. + Utf16FromUtf8, + /// simdutf UTF-16LE to UTF-8. + Utf8FromUtf16, } -/// Mirrors `TranscodeLatin1ToUTF16`: widens ASCII/LATIN1 `source` into UTF-16 -/// via simdutf. Source bytes `0x80`-`0xFF` become U+0080-U+00FF rather than -/// being substituted -- this is the "latin1" path, taken even when `from` is -/// `ASCII` (see R8). -fn transcode_latin1_to_utf16(source: &[u8]) -> Result>, TranscodeError> { - let length_in_chars = source - .len() - .checked_mul(2) - .ok_or(TranscodeError::SourceBufferTooLarge)?; - // Workers are limited to 128MB so this isn't actually a realistic concern, - // but sanity check. - if length_in_chars > ISOLATE_LIMIT { - return Err(TranscodeError::SourceBufferTooLarge); - } - if length_in_chars == 0 { - return Ok(Some(Vec::new())); +impl Transcoder { + /// Prepares a transcode of `source` from `from` to `to`. + /// + /// Returns an error if `source` is malformed for `from`, or if the + /// destination the conversion would need exceeds [`ISOLATE_LIMIT`]. + pub fn new(source: &[u8], from: Encoding, to: Encoding) -> Result { + match (from, to) { + (Encoding::Ascii | Encoding::Latin1, Encoding::Utf16Le) => { + Self::latin1_to_utf16(source) + } + (Encoding::Utf8, Encoding::Utf16Le) => Self::utf16_from_utf8(source), + (Encoding::Utf16Le, Encoding::Utf8) => Self::utf8_from_utf16(source), + (Encoding::Utf16Le, Encoding::Ascii | Encoding::Latin1) => Self::from_utf16(source, to), + // Identity pairs, UTF16LE -> UTF16LE, and anything else the + // simdutf conversions above do not cover. + _ => Self::convert_ex(source, from, to), + } } - let mut dest = vec![0u8; length_in_chars]; - let actual_length = shim::convert_latin1_to_utf16(source, &mut dest); - // simdutf returns 0 for invalid input. - if actual_length == 0 { - return Ok(None); + /// The exact size, in bytes, of the destination buffer + /// [`Transcoder::transcode_into`] requires. + pub fn dest_len(&self) -> usize { + self.dest_len } - dest.truncate(actual_length * 2); - Ok(Some(dest)) -} -/// Mirrors `TranscodeFromUTF16`: `ucnv_fromUChars` from UTF-16LE `source` into -/// `to`'s encoding (ASCII or LATIN1). -fn transcode_from_utf16(source: &[u8], to: Encoding) -> Result>, TranscodeError> { - let to_conv = Converter::open(to); - let substitute = "?".repeat(to_conv.min_char_size()); - to_conv.set_subst_chars(&substitute); + /// Transcodes `source` into `dest`, returning the number of bytes written. + /// + /// `source` must be the buffer this transcoder was built from, and `dest` + /// must be exactly [`Transcoder::dest_len`] bytes long. The written length + /// is always less than or equal to `dest.len()`. + pub fn transcode_into(&self, source: &[u8], dest: &mut [u8]) -> Result { + if dest.len() != self.dest_len { + return Err(TranscodeError::DestinationSizeMismatch); + } + // A zero-length destination means the conversion has nothing to write: + // either the source was empty, or the estimated output length was zero. + if dest.is_empty() { + return Ok(0); + } - if !source.len().is_multiple_of(2) { - return Err(TranscodeError::OddUtf16leInput); - } - let utf16_len = source.len() / 2; - - let limit = utf16_len - .checked_mul(to_conv.max_char_size()) - .ok_or(TranscodeError::BufferTooLarge)?; - // Workers are limited to 128MB so this isn't actually a realistic concern, - // but sanity check. - if limit > ISOLATE_LIMIT { - return Err(TranscodeError::BufferTooLarge); - } - if limit == 0 { - return Ok(Some(Vec::new())); + match &self.conversion { + Conversion::ConvertEx { to, from } => { + shim::convert_ex(to, from, source, dest).ok_or(TranscodeError::UnableToTranscode) + } + Conversion::Latin1ToUtf16 => { + let units = shim::convert_latin1_to_utf16(source, dest); + // simdutf returns 0 for invalid input. + if units == 0 { + return Err(TranscodeError::UnableToTranscode); + } + // Each Latin-1 byte widens to exactly one UTF-16 code unit, and + // `dest` was sized as two bytes per source byte. + Ok(units * 2) + } + Conversion::FromUtf16 { to } => { + shim::from_uchars(to, source, dest).ok_or(TranscodeError::UnableToTranscode) + } + Conversion::Utf16FromUtf8 => { + // `dest` was sized as two bytes per estimated code unit. + let expected_units = dest.len() / 2; + let units = shim::convert_utf8_to_utf16le(source, dest); + // simdutf returns 0 for invalid UTF-8 input. + if units == 0 { + return Err(TranscodeError::UnableToTranscode); + } + if units != expected_units { + return Err(TranscodeError::Utf16LengthMismatch); + } + Ok(dest.len()) + } + Conversion::Utf8FromUtf16 => { + let expected_bytes = dest.len(); + let written = shim::convert_utf16le_to_utf8(source, dest); + // simdutf returns 0 for invalid input, which fails this check + // because `expected_bytes` is nonzero here. + if written != expected_bytes { + return Err(TranscodeError::Utf8LengthMismatch); + } + Ok(written) + } + } } - let mut dest = vec![0u8; limit]; - Ok( - shim::from_uchars(&to_conv, source, &mut dest).map(|written| { - dest.truncate(written); - dest - }), - ) -} + /// ICU `ucnv_convertEx` between two converters, sized at `to`'s maximum + /// bytes per character. + fn convert_ex(source: &[u8], from: Encoding, to: Encoding) -> Result { + let to_conv = Converter::open(to)?; + to_conv.set_subst_chars(&"?".repeat(to_conv.min_char_size()))?; + let from_conv = Converter::open(from)?; + + let dest_len = source + .len() + .checked_mul(to_conv.max_char_size()) + .ok_or(TranscodeError::SourceBufferTooLarge)?; + if dest_len > ISOLATE_LIMIT { + return Err(TranscodeError::SourceBufferTooLarge); + } -/// Mirrors `TranscodeUTF16FromUTF8`: converts UTF-8 `source` to UTF-16LE via -/// simdutf. The output size comes from `simdutf::utf16_length_from_utf8`; when -/// that estimate is zero the result is an empty buffer, even for non-empty -/// input (see R8). -fn transcode_utf16_from_utf8(source: &[u8]) -> Result>, TranscodeError> { - let expected_utf16_length = shim::utf16_length_from_utf8(source); - // Workers are limited to 128MB so this isn't actually a realistic concern, - // but sanity check. - if expected_utf16_length > ISOLATE_LIMIT { - return Err(TranscodeError::ExpectedUtf16LengthTooLarge); + Ok(Self { + conversion: Conversion::ConvertEx { + to: to_conv, + from: from_conv, + }, + dest_len, + }) } - let length_in_chars = expected_utf16_length - .checked_mul(2) - .ok_or(TranscodeError::ExpectedUtf16LengthTooLarge)?; - if length_in_chars == 0 { - return Ok(Some(Vec::new())); - } + /// simdutf widening of ASCII/Latin-1 into UTF-16. + /// + /// Taken for an `Ascii` source as well as a `Latin1` one, so source bytes + /// `0x80`-`0xFF` widen to U+0080-U+00FF instead of being substituted. + /// + /// No ICU converter is involved: the widening is purely arithmetic. + fn latin1_to_utf16(source: &[u8]) -> Result { + let dest_len = source + .len() + .checked_mul(2) + .ok_or(TranscodeError::SourceBufferTooLarge)?; + if dest_len > ISOLATE_LIMIT { + return Err(TranscodeError::SourceBufferTooLarge); + } - let mut dest = vec![0u8; length_in_chars]; - let actual_length = shim::convert_utf8_to_utf16le(source, &mut dest); - // simdutf returns 0 for invalid UTF-8 input. - if actual_length == 0 { - return Ok(None); - } - if actual_length != expected_utf16_length { - return Err(TranscodeError::Utf16LengthMismatch); + Ok(Self { + conversion: Conversion::Latin1ToUtf16, + dest_len, + }) } - Ok(Some(dest)) -} -/// Mirrors `TranscodeUTF8FromUTF16`: converts UTF-16LE `source` to UTF-8 via -/// simdutf, requiring the actual conversion length to equal the estimate from -/// `simdutf::utf8_length_from_utf16le`. -fn transcode_utf8_from_utf16(source: &[u8]) -> Result>, TranscodeError> { - if !source.len().is_multiple_of(2) { - return Err(TranscodeError::OddUtf16leInput); - } + /// ICU `ucnv_fromUChars` from UTF-16LE into `to`'s encoding, sized at + /// `to`'s maximum bytes per character. + fn from_utf16(source: &[u8], to: Encoding) -> Result { + let to_conv = Converter::open(to)?; + to_conv.set_subst_chars(&"?".repeat(to_conv.min_char_size()))?; - let expected_utf8_length = shim::utf8_length_from_utf16le(source); - // Workers are limited to 128MB so this isn't actually a realistic concern, - // but sanity check. - if expected_utf8_length > ISOLATE_LIMIT { - return Err(TranscodeError::ExpectedUtf8LengthTooLarge); - } - if expected_utf8_length == 0 { - return Ok(Some(Vec::new())); + if !source.len().is_multiple_of(2) { + return Err(TranscodeError::OddUtf16leInput); + } + + let dest_len = (source.len() / 2) + .checked_mul(to_conv.max_char_size()) + .ok_or(TranscodeError::BufferTooLarge)?; + if dest_len > ISOLATE_LIMIT { + return Err(TranscodeError::BufferTooLarge); + } + + Ok(Self { + conversion: Conversion::FromUtf16 { to: to_conv }, + dest_len, + }) } - let mut dest = vec![0u8; expected_utf8_length]; - let actual_length = shim::convert_utf16le_to_utf8(source, &mut dest); - if actual_length != expected_utf8_length { - return Err(TranscodeError::Utf8LengthMismatch); + /// simdutf UTF-8 to UTF-16LE, sized from + /// `simdutf::utf16_length_from_utf8`. + /// + /// That estimate is zero for some non-empty inputs -- a source of nothing + /// but UTF-8 continuation bytes, for instance -- which yields an empty + /// result rather than an error. + fn utf16_from_utf8(source: &[u8]) -> Result { + let expected_units = shim::utf16_length_from_utf8(source); + if expected_units > ISOLATE_LIMIT { + return Err(TranscodeError::ExpectedUtf16LengthTooLarge); + } + let dest_len = expected_units + .checked_mul(2) + .ok_or(TranscodeError::ExpectedUtf16LengthTooLarge)?; + + Ok(Self { + conversion: Conversion::Utf16FromUtf8, + dest_len, + }) } - // Unreachable in practice: `actual_length` was just required to equal - // `expected_utf8_length`, which is nonzero at this point. This mirrors the - // equivalent dead branch in the C++ `TranscodeUTF8FromUTF16`, which checks - // `actual_length == 0` only *after* already requiring it to equal the - // (nonzero) expected length above -- so a simdutf failure (return value 0) - // surfaces as "Expected UTF8 length mismatch" here, not - // "Unable to transcode buffer", unlike every other conversion direction. - if actual_length == 0 { - return Ok(None); + + /// simdutf UTF-16LE to UTF-8, sized from + /// `simdutf::utf8_length_from_utf16le`. + fn utf8_from_utf16(source: &[u8]) -> Result { + if !source.len().is_multiple_of(2) { + return Err(TranscodeError::OddUtf16leInput); + } + + let dest_len = shim::utf8_length_from_utf16le(source); + if dest_len > ISOLATE_LIMIT { + return Err(TranscodeError::ExpectedUtf8LengthTooLarge); + } + + Ok(Self { + conversion: Conversion::Utf8FromUtf16, + dest_len, + }) } - Ok(Some(dest)) } #[cfg(test)] @@ -222,6 +263,21 @@ mod tests { Harness::new() } + /// Runs both transcode steps the way the C++ caller does -- size, allocate, + /// convert, narrow to the written length -- and returns the written bytes. + fn transcode(source: &[u8], from: Encoding, to: Encoding) -> Result, TranscodeError> { + let transcoder = Transcoder::new(source, from, to)?; + let mut dest = vec![0u8; transcoder.dest_len()]; + let written = transcoder.transcode_into(source, &mut dest)?; + assert!( + written <= dest.len(), + "{from:?} -> {to:?} wrote {written} bytes into a {} byte buffer", + dest.len() + ); + dest.truncate(written); + Ok(dest) + } + #[test] fn every_pair_round_trips_ascii_text() { let _harness = init_icu(); @@ -274,9 +330,7 @@ mod tests { #[test] fn unmappable_characters_become_question_marks() { let _harness = init_icu(); - // '☕' (U+2615, HOT BEVERAGE) has no representation in ASCII or - // LATIN1/windows-1252 -- the same character the existing C++ path is - // exercised with in `buffer-nodejs-test.js`'s `transcodeTest`. + // '☕' (U+2615, HOT BEVERAGE) has no representation in ASCII or Latin-1. let source = "☕".as_bytes(); assert_eq!( transcode(source, Encoding::Utf8, Encoding::Ascii).unwrap(), @@ -291,8 +345,8 @@ mod tests { #[test] fn ascii_to_utf16le_widens_high_bytes_via_latin1_path() { let _harness = init_icu(); - // R8: ASCII -> UTF16LE takes the latin1 path, so a high byte is - // widened to U+00FF rather than substituted with '?'. + // ASCII -> UTF16LE takes the Latin-1 path, so a high byte is widened to + // U+00FF rather than substituted with '?'. let result = transcode(&[0xff], Encoding::Ascii, Encoding::Utf16Le).unwrap(); assert_eq!(result, vec![0xff, 0x00]); } @@ -300,9 +354,9 @@ mod tests { #[test] fn utf8_continuation_byte_only_input_yields_empty_utf16le() { let _harness = init_icu(); - // R8: the UTF8 -> UTF16LE size estimate is zero for input consisting - // only of continuation bytes, so the result is empty rather than an - // error, even though the input is non-empty. + // The UTF8 -> UTF16LE size estimate is zero for input consisting only of + // continuation bytes, so the result is empty rather than an error, even + // though the input is non-empty. let result = transcode(&[0x80], Encoding::Utf8, Encoding::Utf16Le).unwrap(); assert!(result.is_empty()); } @@ -333,15 +387,11 @@ mod tests { let _harness = init_icu(); // U+D800, an unpaired high surrogate, encoded as UTF-16LE bytes. let source = [0x00, 0xd8]; - // simdutf treats this as invalid input; whatever it decides (empty - // output or an error) must not panic, and must match what the C++ - // path's `simdutf::utf8_length_from_utf16le` / - // `simdutf::convert_utf16le_to_utf8` pair would produce, since both - // implementations call the exact same simdutf functions. + // `simdutf::utf8_length_from_utf16le` does not validate, and reports the + // three bytes a replacement character would occupy, but + // `simdutf::convert_utf16le_to_utf8` rejects the input and writes + // nothing -- so the mismatch check is what surfaces the failure. let result = transcode(&source, Encoding::Utf16Le, Encoding::Utf8); - match result { - Ok(bytes) => assert!(bytes.is_empty()), - Err(err) => assert_eq!(err, TranscodeError::Utf8LengthMismatch), - } + assert_eq!(result, Err(TranscodeError::Utf8LengthMismatch)); } } diff --git a/src/rust/i18n/error.rs b/src/rust/i18n/error.rs index 5f384361437..0cbbd06cd70 100644 --- a/src/rust/i18n/error.rs +++ b/src/rust/i18n/error.rs @@ -2,17 +2,15 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 +//! The error type reported by [`crate::dispatch`], and its adapter for the +//! trip across the CXX bridge. + use thiserror::Error; -/// Errors from the Rust `i18n::transcode` implementation ([`crate::dispatch`]). +/// A failed transcode. /// -/// Every message matches the corresponding `JSG_REQUIRE` / `JSG_FAIL_REQUIRE` -/// string in `workerd::api::node::i18n::transcode` -/// (`src/workerd/api/node/i18n.c++`) verbatim, so gate-on and gate-off are -/// indistinguishable to JavaScript. `"Invalid encoding passed to transcode"` -/// is not a variant here: it is raised by the C++ `fromImpl` conversion before -/// the Rust entry point is ever called (see `i18n.c++`), since the bridge -/// `Encoding` enum can only represent the four transcodable encodings. +/// Every message is the text of the JavaScript `Error` that reaches the +/// caller of `node:buffer`'s `transcode()`. #[derive(Debug, Error, PartialEq, Eq)] pub enum TranscodeError { #[error("Source buffer is too large to transcode")] @@ -31,12 +29,26 @@ pub enum TranscodeError { Utf8LengthMismatch, #[error("Unable to transcode buffer")] UnableToTranscode, + #[error("Invalid encoding passed to transcode")] + InvalidEncoding, + #[error("Failed to initialize converter")] + ConverterOpenFailed, + #[error("Setting ICU substitute characters failed")] + SetSubstituteCharsFailed, + #[error("Destination buffer size does not match the prepared transcode")] + DestinationSizeMismatch, } -impl From for jsg::Error { - fn from(value: TranscodeError) -> Self { - // All of these are plain JS `Error`s, matching the `JSG_REQUIRE(..., Error, ...)` - // calls they replace. - Self::new_error(value.to_string()) - } -} +/// A [`TranscodeError`] on its way out through the CXX bridge. +/// +/// `cxx` converts a returned `Err` into a `kj::Exception` whose description is +/// the error's `Display` output. A description that begins with +/// `jsg.: ` tells JSG to throw that JavaScript error type using the +/// remaining text as the message (see `tunneledErrorType` in +/// `src/workerd/jsg/exception.c++`); this is the same encoding +/// `JSG_REQUIRE(..., Error, ...)` produces. Emitting the prefix here is +/// therefore what turns a [`TranscodeError`] into a JavaScript `Error` whose +/// `message` is the variant's text. +#[derive(Debug, Error)] +#[error("jsg.Error: {0}")] +pub struct JsError(#[from] TranscodeError); diff --git a/src/rust/i18n/lib.rs b/src/rust/i18n/lib.rs index 645d9699fd1..8d2b216bf81 100644 --- a/src/rust/i18n/lib.rs +++ b/src/rust/i18n/lib.rs @@ -2,31 +2,40 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! Rust port of `workerd::api::node::i18n::transcode` -//! (`src/workerd/api/node/i18n.c++`), the engine behind `node:buffer`'s -//! `transcode()`. Selected at runtime by the `NODEJS_I18N_RUST` autogate; when -//! the gate is off, the C++ implementation is used instead. The two paths are -//! byte-for-byte and error-message identical by construction: [`dispatch`] -//! ports the C++ dispatch/sizing/truncation logic to Rust, while [`shim`] -//! calls the exact same ICU and simdutf primitives the C++ path uses, through -//! the C++ shim in `shim.h` / `shim.c++`. - -use jsg::Lock; -use jsg::ToJS; -use jsg::v8; +//! The engine behind `node:buffer`'s `transcode()`: converts a byte buffer +//! between the four transcodable encodings using ICU and simdutf. +//! +//! The C++ caller owns the destination buffer. It first builds a +//! [`Transcoder`], which validates the input and reports the exact destination +//! size the conversion needs, then allocates a buffer of that size and asks +//! the transcoder to fill it. Separating sizing from writing lets the +//! destination be a JavaScript `Uint8Array`'s backing store, so a conversion +//! writes straight into the buffer that is handed back to JavaScript instead of +//! into an intermediate the caller would have to copy. +//! +//! Nothing here touches V8: the bridge deals only in byte slices, and the +//! caller is responsible for allocating the destination and for narrowing its +//! view of that buffer to the written length. +//! +//! Reached only when the `NODEJS_I18N_RUST` autogate is enabled; otherwise +//! `workerd::api::node::i18n::transcode` (`src/workerd/api/node/i18n.c++`) +//! performs the conversion itself. mod dispatch; mod error; mod shim; +use crate::dispatch::Transcoder; +use crate::error::JsError; + #[cxx::bridge(namespace = "workerd::rust::i18n")] mod ffi { - /// The four encodings `i18n::transcode` supports. Mirrors the - /// transcodable subset of `workerd::api::node::Encoding` - /// (`src/workerd/api/node/i18n.h`). `src/workerd/api/node/i18n.c++` maps - /// into this type through a `fromImpl` overload that rejects the - /// non-transcodable `BASE64`, `BASE64URL`, and `HEX` variants before ever - /// calling into Rust (see R10). + /// The encodings `transcode` supports. + /// + /// `src/workerd/api/node/i18n.c++` maps `workerd::api::node::Encoding` + /// into this type through a `fromImpl` overload, which rejects the + /// non-transcodable `BASE64`, `BASE64URL`, and `HEX` encodings before any + /// of this crate runs. #[derive(Debug, PartialEq, Eq, Copy, Clone)] #[repr(u8)] enum Encoding { @@ -44,7 +53,7 @@ mod ffi { fn open_converter(name: &str) -> UniquePtr; fn max_char_size(self: &Converter) -> usize; fn min_char_size(self: &Converter) -> usize; - fn set_subst_chars(self: &Converter, substitute: &str); + fn set_subst_chars(self: &Converter, substitute: &str) -> bool; fn convert_ex(to: &Converter, from: &Converter, source: &[u8], target: &mut [u8]) -> i64; fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> i64; @@ -56,59 +65,53 @@ mod ffi { fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize; } - #[namespace = "workerd::rust::jsg"] - unsafe extern "C++" { - include!("workerd/rust/jsg/ffi.h"); - include!("workerd/rust/jsg/v8.rs.h"); - - type Isolate = jsg::v8::ffi::Isolate; - type MaybeLocal = jsg::v8::ffi::MaybeLocal; - } - extern "Rust" { - /// Transcodes `source` from `from_encoding` to `to_encoding`, matching - /// `workerd::api::node::i18n::transcode`. Returns a `MaybeLocal` - /// naming a `Uint8Array`, or an empty `MaybeLocal` with a JS exception - /// already scheduled on `isolate` if transcoding fails. + /// A validated transcode, ready to run. See [`new_transcoder`]. + type Transcoder; + + /// Prepares a transcode of `source` from `from_encoding` to + /// `to_encoding`. /// - /// # Safety - /// `isolate` must be a valid pointer to a locked `v8::Isolate` with an - /// active `HandleScope`. - unsafe fn transcode( - isolate: *mut Isolate, + /// Throws if `source` is malformed for `from_encoding`, or if the + /// destination the conversion would need is too large for an isolate. + // Boxed because the CXX bridge requires it of opaque Rust types. + fn new_transcoder( source: &[u8], from_encoding: Encoding, to_encoding: Encoding, - ) -> MaybeLocal; + ) -> Result>; + + /// The exact size, in bytes, of the destination buffer [`run`] + /// requires. + fn dest_len(self: &Transcoder) -> usize; + + /// Transcodes `source` into `dest`, returning the number of bytes + /// written, which may be less than `dest.len()`. + /// + /// `source` must be the same buffer that was passed to + /// [`new_transcoder`], and `dest` must be exactly [`dest_len`] bytes + /// long. + fn run(self: &Transcoder, source: &[u8], dest: &mut [u8]) -> Result; } } -/// # Safety -/// `isolate` must be a valid pointer to a locked `v8::Isolate` with an active -/// `HandleScope`. -unsafe fn transcode( - isolate: *mut ffi::Isolate, +fn new_transcoder( source: &[u8], from_encoding: ffi::Encoding, to_encoding: ffi::Encoding, -) -> ffi::MaybeLocal { - // SAFETY: forwarded from this function's own safety contract -- the C++ - // caller (`i18n::transcode` in `i18n.c++`) guarantees `isolate` is valid, - // locked, and has an active HandleScope. - let mut lock = unsafe { Lock::from_isolate_ptr(isolate) }; - match dispatch::transcode(source, from_encoding, to_encoding) { - Ok(bytes) => { - let local: v8::Local = bytes.to_js(&mut lock); - // SAFETY: `local` was just created in the isolate's active - // HandleScope; its FFI representation is handed to the C++ - // caller, which reconstitutes it via `maybe_local_from_ffi` and - // immediately passes it through `jsg::check()`. - let raw = unsafe { local.into_ffi() }; - ffi::MaybeLocal { ptr: raw.ptr } - } - Err(err) => { - lock.throw_exception(&err.into()); - ffi::MaybeLocal { ptr: 0 } - } +) -> Result, JsError> { + Ok(Box::new(Transcoder::new( + source, + from_encoding, + to_encoding, + )?)) +} + +impl Transcoder { + /// The bridge's spelling of [`Transcoder::transcode_into`], reporting + /// failures as a [`JsError`] so they reach JavaScript as the expected + /// `Error`. + fn run(&self, source: &[u8], dest: &mut [u8]) -> Result { + Ok(self.transcode_into(source, dest)?) } } diff --git a/src/rust/i18n/shim.c++ b/src/rust/i18n/shim.c++ index 4564e034803..6d15b371970 100644 --- a/src/rust/i18n/shim.c++ +++ b/src/rust/i18n/shim.c++ @@ -8,9 +8,7 @@ #include -#include - -#include +#include namespace workerd::rust::i18n { @@ -28,24 +26,23 @@ size_t Converter::min_char_size() const { return static_cast(ucnv_getMinCharSize(conv_)); } -void Converter::set_subst_chars(::rust::Str substitute) const { - if (substitute.size() == 0) return; +bool Converter::set_subst_chars(::rust::Str substitute) const { + if (substitute.empty()) return true; + // `ucnv_setSubstChars` takes the length as an `int8_t`, and reads a negative + // length as "NUL-terminated", which `rust::Str` is not. Reject anything that + // would not survive the narrowing; ICU's own limit is far lower still. + if (substitute.size() > INT8_MAX) return false; UErrorCode status = U_ZERO_ERROR; ucnv_setSubstChars(conv_, substitute.data(), static_cast(substitute.size()), &status); - // Unreachable in practice: the substitute strings this module sets are always - // short, valid ASCII ('?' repeated `minCharSize()` times), so ICU never - // rejects them for any of the four transcodable encodings. - KJ_REQUIRE(U_SUCCESS(status), "Setting ICU substitute characters failed"); + return U_SUCCESS(status); } std::unique_ptr open_converter(::rust::Str name) { UErrorCode status = U_ZERO_ERROR; - std::string nameStr(name.data(), name.size()); - auto* conv = ucnv_open(nameStr.c_str(), &status); - // Unreachable in practice: this module only ever opens converters for the - // four fixed, always-valid ICU encoding names ("us-ascii", "iso8859-1", - // "utf-8", "utf16le"). - KJ_REQUIRE(U_SUCCESS(status), "Failed to initialize converter"); + // `ucnv_open` needs a NUL-terminated name, which `rust::Str` is not. + auto nameStr = kj::str(kj::ArrayPtr(name.data(), name.size())); + auto* conv = ucnv_open(nameStr.cStr(), &status); + if (U_FAILURE(status)) return nullptr; return std::make_unique(conv); } diff --git a/src/rust/i18n/shim.h b/src/rust/i18n/shim.h index d9433f3185a..82015bfcdcc 100644 --- a/src/rust/i18n/shim.h +++ b/src/rust/i18n/shim.h @@ -37,7 +37,11 @@ class Converter { size_t max_char_size() const; size_t min_char_size() const; - void set_subst_chars(::rust::Str substitute) const; + + // Sets the byte sequence ICU substitutes for characters that cannot be + // represented in this converter's encoding. Returns false if `substitute` is + // too long, or if ICU rejects it for this encoding. + bool set_subst_chars(::rust::Str substitute) const; private: UConverter* conv_; @@ -50,7 +54,8 @@ class Converter { const Converter& to, ::rust::Slice source, ::rust::Slice target); }; -// Opens an ICU converter for `name` (an ICU encoding name, e.g. "us-ascii"). +// Opens an ICU converter for `name` (an ICU encoding name, e.g. "us-ascii"), +// or returns null if ICU does not recognize `name`. std::unique_ptr open_converter(::rust::Str name); // `ucnv_convertEx()`-based conversion between two ICU converters, mirroring @@ -69,10 +74,13 @@ int64_t convert_ex(const Converter& to, int64_t from_uchars( const Converter& to, ::rust::Slice source, ::rust::Slice target); -// simdutf wrappers, mirroring the four `simdutf::*` calls in `i18n.c++`. -// Buffers holding UTF-16LE code units are passed as raw bytes and cast to -// `char16_t*` internally, exactly as the C++ path does via -// `JsUint8Array::asArrayPtr()`. +// simdutf wrappers. Buffers holding UTF-16LE code units are passed as raw +// bytes and cast to `char16_t*` internally. +// +// `target` must be aligned for `char16_t`. Callers satisfy this by passing a +// JavaScript `ArrayBuffer`'s backing store, whose base address V8 aligns well +// past two bytes. `source` carries no such guarantee: it is caller-supplied +// buffer contents, which a `Uint8Array` can expose at an odd `byteOffset`. size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target); size_t utf16_length_from_utf8(::rust::Slice source); diff --git a/src/rust/i18n/shim.rs b/src/rust/i18n/shim.rs index 61fc10f8a2b..e4128113d7f 100644 --- a/src/rust/i18n/shim.rs +++ b/src/rust/i18n/shim.rs @@ -8,56 +8,63 @@ //! this module only adapts the shim's C-ish sentinel-value return conventions //! (`-1` for ICU failure, `0` for simdutf failure) into idiomatic `Option`s. +use crate::error::TranscodeError; use crate::ffi; /// An open ICU converter for one of the four transcodable encodings. /// /// Wraps a `cxx::UniquePtr`: the underlying `UConverter*` and /// its `ucnv_close()` teardown are owned entirely by the C++ shim, so the -/// converter is torn down correctly even if Rust code holding it panics -- -/// unlike a raw `UConverter*` smuggled across the FFI boundary, which a panic -/// could leak. +/// converter is torn down correctly even if Rust code holding it panics. pub struct Converter(cxx::UniquePtr); -/// Returns the ICU converter name for a transcodable encoding, matching -/// `getEncodingName()` in `i18n.c++`. -fn icu_name(encoding: ffi::Encoding) -> &'static str { +/// Returns the ICU converter name for a transcodable encoding. +/// +/// The bridge `Encoding` enum is a `cxx` shared enum, which is a `u8` newtype +/// rather than a real Rust enum, so a value outside the four declared variants +/// is representable. It can only arise if the C++ and Rust halves of the +/// bridge disagree, and is reported as an error rather than a panic because a +/// panic crossing the bridge aborts the process. +fn icu_name(encoding: ffi::Encoding) -> Result<&'static str, TranscodeError> { match encoding { - ffi::Encoding::Ascii => "us-ascii", - ffi::Encoding::Latin1 => "iso8859-1", - ffi::Encoding::Utf16Le => "utf16le", - ffi::Encoding::Utf8 => "utf-8", - // The bridge `Encoding` enum has exactly these four variants (R4); any - // other discriminant would mean the C++/Rust enum definitions have - // drifted out of sync. - _ => unreachable!("Encoding has exactly four variants"), + ffi::Encoding::Ascii => Ok("us-ascii"), + ffi::Encoding::Latin1 => Ok("iso8859-1"), + ffi::Encoding::Utf16Le => Ok("utf16le"), + ffi::Encoding::Utf8 => Ok("utf-8"), + _ => Err(TranscodeError::InvalidEncoding), } } impl Converter { /// Opens an ICU converter for `encoding`. - /// - /// Never fails in practice -- the four encoding names above are always - /// valid ICU converter names -- so a shim-side open failure (see - /// `shim.c++`) is treated as an unrecoverable invariant violation rather - /// than a catchable error, matching how unreachable `KJ_ASSERT`-style - /// conditions are handled elsewhere in this codebase. - pub fn open(encoding: ffi::Encoding) -> Self { - Self(ffi::open_converter(icu_name(encoding))) + pub fn open(encoding: ffi::Encoding) -> Result { + let conv = ffi::open_converter(icu_name(encoding)?); + if conv.is_null() { + return Err(TranscodeError::ConverterOpenFailed); + } + Ok(Self(conv)) } + /// Returns the largest number of bytes a single character occupies in this + /// converter's encoding. pub fn max_char_size(&self) -> usize { self.0.max_char_size() } + /// Returns the smallest number of bytes a single character occupies in this + /// converter's encoding. pub fn min_char_size(&self) -> usize { self.0.min_char_size() } /// Sets the converter's substitute character sequence, used in place of /// unmappable characters during conversion. - pub fn set_subst_chars(&self, substitute: &str) { - self.0.set_subst_chars(substitute); + pub fn set_subst_chars(&self, substitute: &str) -> Result<(), TranscodeError> { + if self.0.set_subst_chars(substitute) { + Ok(()) + } else { + Err(TranscodeError::SetSubstituteCharsFailed) + } } } diff --git a/src/rust/jsg/ffi-inl.h b/src/rust/jsg/ffi-inl.h index 9cc65aee465..70eb9af97bd 100644 --- a/src/rust/jsg/ffi-inl.h +++ b/src/rust/jsg/ffi-inl.h @@ -50,12 +50,6 @@ inline MaybeLocal maybe_local_to_ffi(v8::MaybeLocal value) { return MaybeLocal{result}; } -template -inline v8::MaybeLocal maybe_local_from_ffi(MaybeLocal&& value) { - auto ptr_void = reinterpret_cast(&value.ptr); - return *reinterpret_cast*>(ptr_void); -} - // Global // // ffi::Global stores only the strong v8::Global in `ptr`. diff --git a/src/workerd/api/node/i18n.c++ b/src/workerd/api/node/i18n.c++ index 1b270023286..90a93324e55 100644 --- a/src/workerd/api/node/i18n.c++ +++ b/src/workerd/api/node/i18n.c++ @@ -9,7 +9,6 @@ #include #include -#include #include #include @@ -24,19 +23,17 @@ namespace workerd::api::node { namespace rust_i18n = ::workerd::rust::i18n; -// Maps the C++ Encoding to the Rust bridge enum, following the kj-rs -// convert.h idiom (see `NODEJS_EXCEPTIONS_RUST`'s equivalent in -// exceptions.c++), so callers use `kj::from(value)`. It -// is `static` (rather than in an anonymous namespace) because Clang's ADL -// does not consider unnamed-namespace functions, and ADL is how -// `kj::from` locates this overload. The switch has no `default:` arm -// so that enum drift between the two `Encoding` types is a compile error. +// Maps the C++ Encoding to the Rust bridge enum, so callers can write +// `kj::from(value)`; see `kj-rs/convert.h` for the +// `fromImpl` convention, and `exceptions.c++` for another instance of it. The +// switch has no `default:` arm so that enum drift between the two `Encoding` +// types is a compile error. // // `BASE64`, `BASE64URL`, and `HEX` are not transcodable (see -// `i18n::canBeTranscoded`); rejecting them here, for both `fromEncoding` and -// `toEncoding`, is a deliberate divergence from the C++ dispatch below, which -// only checks `fromEncoding` and would reach `KJ_UNREACHABLE` for a -// non-transcodable `toEncoding`. Neither is reachable from JavaScript because +// `i18n::canBeTranscoded`). Rejecting them here covers `toEncoding` as well as +// `fromEncoding`, unlike the C++ dispatch below, which checks only +// `fromEncoding` and reaches `KJ_UNREACHABLE` for a non-transcodable +// `toEncoding`. Neither is reachable from JavaScript because // `BufferUtil::transcode` validates both encodings first. static rust_i18n::Encoding fromImpl(rust_i18n::Encoding*, Encoding encoding) { switch (encoding) { @@ -279,13 +276,18 @@ void Converter::setSubstituteChars(kj::StringPtr sub) { jsg::JsUint8Array transcode( jsg::Lock& js, kj::ArrayPtr source, Encoding fromEncoding, Encoding toEncoding) { if (util::Autogate::isEnabled(util::AutogateKey::NODEJS_I18N_RUST)) { - auto rustFrom = kj::from(fromEncoding); - auto rustTo = kj::from(toEncoding); - auto maybeLocal = - rust_i18n::transcode(js.v8Isolate, source.as(), rustFrom, rustTo); - auto local = - jsg::check(::workerd::rust::jsg::maybe_local_from_ffi(kj::mv(maybeLocal))); - return jsg::JsUint8Array(local); + auto rustSource = source.as(); + auto transcoder = rust_i18n::new_transcoder(rustSource, + kj::from(fromEncoding), kj::from(toEncoding)); + + // The destination is sized for the worst case, so the conversion often + // writes less than it was given; narrow the result to what was written. + auto out = jsg::JsUint8Array::create(js, transcoder->dest_len()); + auto written = transcoder->run(rustSource, out.asArrayPtr().as()); + if (written < out.size()) { + return out.slice(js, written); + } + return out; } TranscodeImpl transcode_function = &TranscodeDefault; From 036608154aef6fd3621d08ad069cea78c1dd5634 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Thu, 20 Aug 2026 11:03:37 -0500 Subject: [PATCH 04/10] accept unsafe cost of isolate ptr --- src/rust/i18n/BUILD.bazel | 12 +- src/rust/i18n/dispatch.rs | 118 ++++++++++++------- src/rust/i18n/error.rs | 44 ++++--- src/rust/i18n/lib.rs | 213 ++++++++++++++++++++++++---------- src/rust/i18n/shim.h | 16 ++- src/rust/i18n/shim.rs | 7 +- src/rust/jsg/ffi-inl.h | 6 + src/rust/jsg/ffi.c++ | 9 ++ src/rust/jsg/ffi.h | 7 ++ src/rust/jsg/v8.rs | 47 ++++++++ src/workerd/api/node/i18n.c++ | 40 +++---- 11 files changed, 367 insertions(+), 152 deletions(-) diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index 6e1f8ef25ab..405c7ef195b 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -22,12 +22,16 @@ wd_cc_library( wd_rust_crate( name = "i18n", - cxx_bridge_deps = [":shim"], + cxx_bridge_deps = [ + ":shim", + "//src/rust/jsg", + ], cxx_bridge_src = "lib.rs", link_deps = [":shim"], - # `jsg-test`'s harness initializes the V8 platform, which is what installs - # the embedded ICU data the unit tests need in order to open converters. test_deps = ["//src/rust/jsg-test"], visibility = ["//visibility:public"], - deps = ["@crates_vendor//:thiserror"], + deps = [ + "//src/rust/jsg", + "@crates_vendor//:thiserror", + ], ) diff --git a/src/rust/i18n/dispatch.rs b/src/rust/i18n/dispatch.rs index 75e5ffddd69..132fdb87fc0 100644 --- a/src/rust/i18n/dispatch.rs +++ b/src/rust/i18n/dispatch.rs @@ -2,18 +2,22 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! Encoding-pair dispatch and the conversion logic behind each pair. +//! Encoding-pair dispatch and the conversion logic behind each pair, ported +//! from `i18n.c++`'s `TranscodeDefault` / `TranscodeLatin1ToUTF16` / +//! `TranscodeFromUTF16` / `TranscodeUTF16FromUTF8` / `TranscodeUTF8FromUTF16` +//! and the `switch` in `transcode()` that picks between them. //! -//! A transcode runs in two steps. [`Transcoder::new`] validates `source`, -//! picks the conversion to perform, and computes -//! [`Transcoder::dest_len`] -- the exact size of the destination buffer that -//! conversion needs. The caller allocates a buffer of that size and passes it -//! to [`Transcoder::transcode_into`], which fills it and returns the number of -//! bytes actually written. +//! A transcode runs in two steps. [`Transcoder::new`] validates the source, +//! picks the conversion, and computes [`Transcoder::dest_len`] -- the exact +//! size of the destination buffer that conversion needs. +//! [`Transcoder::transcode_into`] then fills a buffer of that size and reports +//! how many bytes it actually wrote, which is often fewer because the +//! destination is sized for the worst case. //! -//! The destination is sized for the worst case, so the write is often shorter -//! than the buffer; the caller is expected to narrow its view of the buffer to -//! the returned length rather than shrink the buffer itself. +//! Splitting sizing from writing is what lets [`crate::transcode`] allocate +//! the destination as a V8 `ArrayBuffer` and convert straight into its backing +//! store, with no intermediate `Vec` and no copy. Nothing in this module +//! touches V8, so it stays unit-testable against a plain `Vec` destination. //! //! All sizing, validation, substitute-character setup, and length checking //! happens here; [`crate::shim`] only forwards to the underlying ICU and @@ -24,13 +28,19 @@ use crate::ffi::Encoding; use crate::shim; use crate::shim::Converter; -/// The memory limit of an isolate, and thus the ceiling on any single -/// destination buffer. Conversions are rejected rather than attempted above -/// this size. -const ISOLATE_LIMIT: usize = 128 * 1024 * 1024; +/// An isolate has a 128MB memory limit, and thus so does any single +/// destination buffer. Mirrors `ISOLATE_LIMIT` in `i18n.c++`. +const ISOLATE_LIMIT: usize = 134_217_728; /// A validated, sized transcode, ready to run. -pub struct Transcoder { +/// +/// Borrows its source for the whole of its life, so [`Transcoder::dest_len`] +/// cannot go stale: the bytes it was computed from are the same bytes +/// [`Transcoder::transcode_into`] reads. This matters because three of the +/// five conversions bottom out in simdutf functions that take no output +/// length and size their writes purely from the source. +pub struct Transcoder<'a> { + source: &'a [u8], conversion: Conversion, dest_len: usize, } @@ -38,25 +48,27 @@ pub struct Transcoder { /// The conversion [`Transcoder::transcode_into`] will perform, along with any /// ICU converters [`Transcoder::new`] had to open to size the destination. enum Conversion { - /// ICU `ucnv_convertEx` between two converters. Handles every pair the - /// simdutf conversions below do not, including all four identity pairs. + /// ICU `ucnv_convertEx` between two converters, mirroring + /// `TranscodeDefault`. Handles every pair the simdutf conversions below do + /// not, including all four identity pairs. ConvertEx { to: Converter, from: Converter }, - /// simdutf Latin-1 to UTF-16. + /// simdutf Latin-1 to UTF-16, mirroring `TranscodeLatin1ToUTF16`. Latin1ToUtf16, - /// ICU `ucnv_fromUChars` from UTF-16LE. + /// ICU `ucnv_fromUChars` from UTF-16LE, mirroring `TranscodeFromUTF16`. FromUtf16 { to: Converter }, - /// simdutf UTF-8 to UTF-16LE. + /// simdutf UTF-8 to UTF-16LE, mirroring `TranscodeUTF16FromUTF8`. Utf16FromUtf8, - /// simdutf UTF-16LE to UTF-8. + /// simdutf UTF-16LE to UTF-8, mirroring `TranscodeUTF8FromUTF16`. Utf8FromUtf16, } -impl Transcoder { - /// Prepares a transcode of `source` from `from` to `to`. +impl<'a> Transcoder<'a> { + /// Prepares a transcode of `source` from `from` to `to`, matching the + /// dispatch table built by `i18n::transcode()` in `i18n.c++`. /// /// Returns an error if `source` is malformed for `from`, or if the /// destination the conversion would need exceeds [`ISOLATE_LIMIT`]. - pub fn new(source: &[u8], from: Encoding, to: Encoding) -> Result { + pub fn new(source: &'a [u8], from: Encoding, to: Encoding) -> Result { match (from, to) { (Encoding::Ascii | Encoding::Latin1, Encoding::Utf16Le) => { Self::latin1_to_utf16(source) @@ -76,12 +88,14 @@ impl Transcoder { self.dest_len } - /// Transcodes `source` into `dest`, returning the number of bytes written. + /// Transcodes into `dest`, returning the number of bytes written, which + /// may be fewer than `dest.len()`. /// - /// `source` must be the buffer this transcoder was built from, and `dest` - /// must be exactly [`Transcoder::dest_len`] bytes long. The written length - /// is always less than or equal to `dest.len()`. - pub fn transcode_into(&self, source: &[u8], dest: &mut [u8]) -> Result { + /// `dest` must be exactly [`Transcoder::dest_len`] bytes long. + pub fn transcode_into(&self, dest: &mut [u8]) -> Result { + // Not merely a sanity check: the simdutf conversions below take no + // output length, so a destination shorter than the size computed for + // this source would overflow it. if dest.len() != self.dest_len { return Err(TranscodeError::DestinationSizeMismatch); } @@ -91,6 +105,7 @@ impl Transcoder { return Ok(0); } + let source = self.source; match &self.conversion { Conversion::ConvertEx { to, from } => { shim::convert_ex(to, from, source, dest).ok_or(TranscodeError::UnableToTranscode) @@ -125,7 +140,11 @@ impl Transcoder { let expected_bytes = dest.len(); let written = shim::convert_utf16le_to_utf8(source, dest); // simdutf returns 0 for invalid input, which fails this check - // because `expected_bytes` is nonzero here. + // because `expected_bytes` is nonzero here. The C++ + // `TranscodeUTF8FromUTF16` checks for 0 only *after* requiring + // equality with the (nonzero) estimate, so that branch is dead + // there too: a simdutf failure surfaces as a length mismatch, + // not "Unable to transcode buffer", unlike every other pair. if written != expected_bytes { return Err(TranscodeError::Utf8LengthMismatch); } @@ -136,7 +155,7 @@ impl Transcoder { /// ICU `ucnv_convertEx` between two converters, sized at `to`'s maximum /// bytes per character. - fn convert_ex(source: &[u8], from: Encoding, to: Encoding) -> Result { + fn convert_ex(source: &'a [u8], from: Encoding, to: Encoding) -> Result { let to_conv = Converter::open(to)?; to_conv.set_subst_chars(&"?".repeat(to_conv.min_char_size()))?; let from_conv = Converter::open(from)?; @@ -150,6 +169,7 @@ impl Transcoder { } Ok(Self { + source, conversion: Conversion::ConvertEx { to: to_conv, from: from_conv, @@ -164,7 +184,7 @@ impl Transcoder { /// `0x80`-`0xFF` widen to U+0080-U+00FF instead of being substituted. /// /// No ICU converter is involved: the widening is purely arithmetic. - fn latin1_to_utf16(source: &[u8]) -> Result { + fn latin1_to_utf16(source: &'a [u8]) -> Result { let dest_len = source .len() .checked_mul(2) @@ -174,6 +194,7 @@ impl Transcoder { } Ok(Self { + source, conversion: Conversion::Latin1ToUtf16, dest_len, }) @@ -181,14 +202,14 @@ impl Transcoder { /// ICU `ucnv_fromUChars` from UTF-16LE into `to`'s encoding, sized at /// `to`'s maximum bytes per character. - fn from_utf16(source: &[u8], to: Encoding) -> Result { - let to_conv = Converter::open(to)?; - to_conv.set_subst_chars(&"?".repeat(to_conv.min_char_size()))?; - + fn from_utf16(source: &'a [u8], to: Encoding) -> Result { if !source.len().is_multiple_of(2) { return Err(TranscodeError::OddUtf16leInput); } + let to_conv = Converter::open(to)?; + to_conv.set_subst_chars(&"?".repeat(to_conv.min_char_size()))?; + let dest_len = (source.len() / 2) .checked_mul(to_conv.max_char_size()) .ok_or(TranscodeError::BufferTooLarge)?; @@ -197,6 +218,7 @@ impl Transcoder { } Ok(Self { + source, conversion: Conversion::FromUtf16 { to: to_conv }, dest_len, }) @@ -208,7 +230,7 @@ impl Transcoder { /// That estimate is zero for some non-empty inputs -- a source of nothing /// but UTF-8 continuation bytes, for instance -- which yields an empty /// result rather than an error. - fn utf16_from_utf8(source: &[u8]) -> Result { + fn utf16_from_utf8(source: &'a [u8]) -> Result { let expected_units = shim::utf16_length_from_utf8(source); if expected_units > ISOLATE_LIMIT { return Err(TranscodeError::ExpectedUtf16LengthTooLarge); @@ -218,6 +240,7 @@ impl Transcoder { .ok_or(TranscodeError::ExpectedUtf16LengthTooLarge)?; Ok(Self { + source, conversion: Conversion::Utf16FromUtf8, dest_len, }) @@ -225,7 +248,7 @@ impl Transcoder { /// simdutf UTF-16LE to UTF-8, sized from /// `simdutf::utf8_length_from_utf16le`. - fn utf8_from_utf16(source: &[u8]) -> Result { + fn utf8_from_utf16(source: &'a [u8]) -> Result { if !source.len().is_multiple_of(2) { return Err(TranscodeError::OddUtf16leInput); } @@ -236,6 +259,7 @@ impl Transcoder { } Ok(Self { + source, conversion: Conversion::Utf8FromUtf16, dest_len, }) @@ -263,12 +287,13 @@ mod tests { Harness::new() } - /// Runs both transcode steps the way the C++ caller does -- size, allocate, - /// convert, narrow to the written length -- and returns the written bytes. + /// Runs both transcode steps the way [`crate::transcode`] does -- size, + /// allocate, convert, narrow to the written length -- against a plain + /// `Vec` rather than a V8 backing store, and returns the written bytes. fn transcode(source: &[u8], from: Encoding, to: Encoding) -> Result, TranscodeError> { let transcoder = Transcoder::new(source, from, to)?; let mut dest = vec![0u8; transcoder.dest_len()]; - let written = transcoder.transcode_into(source, &mut dest)?; + let written = transcoder.transcode_into(&mut dest)?; assert!( written <= dest.len(), "{from:?} -> {to:?} wrote {written} bytes into a {} byte buffer", @@ -394,4 +419,15 @@ mod tests { let result = transcode(&source, Encoding::Utf16Le, Encoding::Utf8); assert_eq!(result, Err(TranscodeError::Utf8LengthMismatch)); } + + #[test] + fn destination_size_mismatch_is_rejected() { + let _harness = init_icu(); + let transcoder = Transcoder::new(b"Hi", Encoding::Latin1, Encoding::Utf16Le).unwrap(); + let mut too_small = vec![0u8; transcoder.dest_len() - 1]; + assert_eq!( + transcoder.transcode_into(&mut too_small), + Err(TranscodeError::DestinationSizeMismatch) + ); + } } diff --git a/src/rust/i18n/error.rs b/src/rust/i18n/error.rs index 0cbbd06cd70..ad51062882e 100644 --- a/src/rust/i18n/error.rs +++ b/src/rust/i18n/error.rs @@ -2,15 +2,23 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! The error type reported by [`crate::dispatch`], and its adapter for the -//! trip across the CXX bridge. +//! The error type reported by [`crate::dispatch`], and its conversion into the +//! JavaScript `Error` the caller of `node:buffer`'s `transcode()` sees. use thiserror::Error; /// A failed transcode. /// -/// Every message is the text of the JavaScript `Error` that reaches the -/// caller of `node:buffer`'s `transcode()`. +/// The messages of the variants that have a C++ counterpart match the +/// corresponding `JSG_REQUIRE` / `JSG_FAIL_REQUIRE` string in +/// `workerd::api::node::i18n::transcode` (`src/workerd/api/node/i18n.c++`) +/// verbatim, so gate-on and gate-off are indistinguishable to JavaScript. +/// Do not reword them. +/// +/// `"Invalid encoding passed to transcode"` has no variant here: the C++ +/// `fromImpl` conversion raises it before the Rust entry point is reached, +/// since the bridge `Encoding` enum can only name the four transcodable +/// encodings. #[derive(Debug, Error, PartialEq, Eq)] pub enum TranscodeError { #[error("Source buffer is too large to transcode")] @@ -29,26 +37,24 @@ pub enum TranscodeError { Utf8LengthMismatch, #[error("Unable to transcode buffer")] UnableToTranscode, - #[error("Invalid encoding passed to transcode")] - InvalidEncoding, #[error("Failed to initialize converter")] ConverterOpenFailed, #[error("Setting ICU substitute characters failed")] SetSubstituteCharsFailed, + // The remaining variants report broken internal invariants rather than bad + // input, and so have no C++ counterpart to match. + #[error("Invalid encoding passed to transcode")] + InvalidEncoding, #[error("Destination buffer size does not match the prepared transcode")] DestinationSizeMismatch, + #[error("Failed to allocate transcode destination buffer")] + AllocationFailed, } -/// A [`TranscodeError`] on its way out through the CXX bridge. -/// -/// `cxx` converts a returned `Err` into a `kj::Exception` whose description is -/// the error's `Display` output. A description that begins with -/// `jsg.: ` tells JSG to throw that JavaScript error type using the -/// remaining text as the message (see `tunneledErrorType` in -/// `src/workerd/jsg/exception.c++`); this is the same encoding -/// `JSG_REQUIRE(..., Error, ...)` produces. Emitting the prefix here is -/// therefore what turns a [`TranscodeError`] into a JavaScript `Error` whose -/// `message` is the variant's text. -#[derive(Debug, Error)] -#[error("jsg.Error: {0}")] -pub struct JsError(#[from] TranscodeError); +impl From for jsg::Error { + fn from(value: TranscodeError) -> Self { + // All of these are plain JS `Error`s, matching the + // `JSG_REQUIRE(..., Error, ...)` calls they replace. + Self::new_error(value.to_string()) + } +} diff --git a/src/rust/i18n/lib.rs b/src/rust/i18n/lib.rs index 8d2b216bf81..6d8043c6d2e 100644 --- a/src/rust/i18n/lib.rs +++ b/src/rust/i18n/lib.rs @@ -2,40 +2,33 @@ // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -//! The engine behind `node:buffer`'s `transcode()`: converts a byte buffer -//! between the four transcodable encodings using ICU and simdutf. -//! -//! The C++ caller owns the destination buffer. It first builds a -//! [`Transcoder`], which validates the input and reports the exact destination -//! size the conversion needs, then allocates a buffer of that size and asks -//! the transcoder to fill it. Separating sizing from writing lets the -//! destination be a JavaScript `Uint8Array`'s backing store, so a conversion -//! writes straight into the buffer that is handed back to JavaScript instead of -//! into an intermediate the caller would have to copy. -//! -//! Nothing here touches V8: the bridge deals only in byte slices, and the -//! caller is responsible for allocating the destination and for narrowing its -//! view of that buffer to the written length. -//! -//! Reached only when the `NODEJS_I18N_RUST` autogate is enabled; otherwise -//! `workerd::api::node::i18n::transcode` (`src/workerd/api/node/i18n.c++`) -//! performs the conversion itself. +//! Rust port of `workerd::api::node::i18n::transcode` +//! (`src/workerd/api/node/i18n.c++`), the engine behind `node:buffer`'s +//! `transcode()`. Selected at runtime by the `NODEJS_I18N_RUST` autogate; when +//! the gate is off, the C++ implementation is used instead. The two paths are +//! byte-for-byte and error-message identical by construction: [`dispatch`] +//! ports the C++ dispatch/sizing/truncation logic to Rust, while [`shim`] +//! calls the exact same ICU and simdutf primitives the C++ path uses, through +//! the C++ shim in `shim.h` / `shim.c++`. + +use jsg::Lock; +use jsg::v8; mod dispatch; mod error; mod shim; use crate::dispatch::Transcoder; -use crate::error::JsError; +use crate::error::TranscodeError; #[cxx::bridge(namespace = "workerd::rust::i18n")] mod ffi { - /// The encodings `transcode` supports. - /// - /// `src/workerd/api/node/i18n.c++` maps `workerd::api::node::Encoding` - /// into this type through a `fromImpl` overload, which rejects the - /// non-transcodable `BASE64`, `BASE64URL`, and `HEX` encodings before any - /// of this crate runs. + /// The four encodings `i18n::transcode` supports. Mirrors the + /// transcodable subset of `workerd::api::node::Encoding` + /// (`src/workerd/api/node/i18n.h`). `src/workerd/api/node/i18n.c++` maps + /// into this type through a `fromImpl` overload that rejects the + /// non-transcodable `BASE64`, `BASE64URL`, and `HEX` variants before ever + /// calling into Rust. #[derive(Debug, PartialEq, Eq, Copy, Clone)] #[repr(u8)] enum Encoding { @@ -65,53 +58,155 @@ mod ffi { fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize; } - extern "Rust" { - /// A validated transcode, ready to run. See [`new_transcoder`]. - type Transcoder; + #[namespace = "workerd::rust::jsg"] + unsafe extern "C++" { + include!("workerd/rust/jsg/ffi.h"); + include!("workerd/rust/jsg/v8.rs.h"); - /// Prepares a transcode of `source` from `from_encoding` to - /// `to_encoding`. + type Isolate = jsg::v8::ffi::Isolate; + type MaybeLocal = jsg::v8::ffi::MaybeLocal; + } + + extern "Rust" { + /// Transcodes `source` from `from_encoding` to `to_encoding`, matching + /// `workerd::api::node::i18n::transcode`. Returns a `MaybeLocal` + /// naming a `Uint8Array`, or an empty `MaybeLocal` with a JS exception + /// already scheduled on `isolate` if transcoding fails. /// - /// Throws if `source` is malformed for `from_encoding`, or if the - /// destination the conversion would need is too large for an isolate. - // Boxed because the CXX bridge requires it of opaque Rust types. - fn new_transcoder( + /// # Safety + /// `isolate` must be a valid pointer to a locked `v8::Isolate` with an + /// active `HandleScope`. + unsafe fn transcode( + isolate: *mut Isolate, source: &[u8], from_encoding: Encoding, to_encoding: Encoding, - ) -> Result>; - - /// The exact size, in bytes, of the destination buffer [`run`] - /// requires. - fn dest_len(self: &Transcoder) -> usize; + ) -> MaybeLocal; + } +} - /// Transcodes `source` into `dest`, returning the number of bytes - /// written, which may be less than `dest.len()`. - /// - /// `source` must be the same buffer that was passed to - /// [`new_transcoder`], and `dest` must be exactly [`dest_len`] bytes - /// long. - fn run(self: &Transcoder, source: &[u8], dest: &mut [u8]) -> Result; +/// # Safety +/// `isolate` must be a valid pointer to a locked `v8::Isolate` with an active +/// `HandleScope`. +unsafe fn transcode( + isolate: *mut ffi::Isolate, + source: &[u8], + from_encoding: ffi::Encoding, + to_encoding: ffi::Encoding, +) -> ffi::MaybeLocal { + // SAFETY: forwarded from this function's own safety contract -- the C++ + // caller (`i18n::transcode` in `i18n.c++`) guarantees `isolate` is valid, + // locked, and has an active HandleScope. + let mut lock = unsafe { Lock::from_isolate_ptr(isolate) }; + match transcode_impl(&mut lock, source, from_encoding, to_encoding) { + Ok(local) => { + // SAFETY: `local` was just created in the isolate's active + // HandleScope; its FFI representation is handed to the C++ caller, + // which reconstitutes it via `maybe_local_from_ffi` and + // immediately passes it through `jsg::check()`. + let raw = unsafe { local.into_ffi() }; + ffi::MaybeLocal { ptr: raw.ptr } + } + Err(err) => { + lock.throw_exception(&err.into()); + ffi::MaybeLocal { ptr: 0 } + } } } -fn new_transcoder( +/// Transcodes `source` into a freshly allocated `Uint8Array`. +/// +/// The conversion writes straight into the V8 `ArrayBuffer`'s backing store. +/// There is no intermediate `Vec` and no copy: [`Transcoder`] reports the +/// destination size up front, the buffer is allocated at exactly that size, +/// and the returned view is narrowed to the bytes actually written. +fn transcode_impl<'a>( + lock: &mut Lock, source: &[u8], from_encoding: ffi::Encoding, to_encoding: ffi::Encoding, -) -> Result, JsError> { - Ok(Box::new(Transcoder::new( - source, - from_encoding, - to_encoding, - )?)) +) -> Result, TranscodeError> { + let transcoder = Transcoder::new(source, from_encoding, to_encoding)?; + + // Every byte of the buffer is either written by the conversion or excluded + // from the returned view, so zeroing it first would be wasted work. + let mut buffer = v8::ArrayBuffer::new_with_mode( + lock, + transcoder.dest_len(), + v8::ffi::BackingStoreInitializationMode::Uninitialized, + ) + .ok_or(TranscodeError::AllocationFailed)?; + + let written = { + // SAFETY: `buffer` was created immediately above and has not been + // handed to JavaScript or aliased by another handle, so this is the + // only live reference into its backing store. `&mut Lock` is borrowed + // for the whole of `dest`, so no JavaScript can run and detach the + // buffer meanwhile. + let dest = unsafe { buffer.as_mut_slice(lock) }; + transcoder.transcode_into(dest)? + }; + + Ok(v8::Uint8Array::from_buffer(lock, &buffer, 0, written)) } -impl Transcoder { - /// The bridge's spelling of [`Transcoder::transcode_into`], reporting - /// failures as a [`JsError`] so they reach JavaScript as the expected - /// `Error`. - fn run(&self, source: &[u8], dest: &mut [u8]) -> Result { - Ok(self.transcode_into(source, dest)?) +#[cfg(test)] +mod tests { + use jsg_test::Harness; + + use super::*; + + /// Exercises the full V8 path: allocate, convert into the backing store, + /// narrow the view. `dispatch.rs` covers conversion behaviour itself; this + /// checks the parts that only exist once V8 is involved. + #[test] + fn transcodes_into_a_narrowed_uint8_array() { + let harness = Harness::new(); + harness.run_in_context(|lock, _ctx| { + // '☕' is three UTF-8 bytes and transcodes to the single byte "?" + // in ASCII, so the destination is allocated at 3 bytes and the + // returned view must be narrowed to 1. + let source = "☕".as_bytes(); + let array = + transcode_impl(lock, source, ffi::Encoding::Utf8, ffi::Encoding::Ascii).unwrap(); + assert_eq!(array.len(), 1); + assert_eq!(array.as_slice(), b"?"); + Ok(()) + }); + } + + #[test] + fn empty_input_yields_an_empty_uint8_array() { + let harness = Harness::new(); + harness.run_in_context(|lock, _ctx| { + let array = + transcode_impl(lock, &[], ffi::Encoding::Utf8, ffi::Encoding::Utf16Le).unwrap(); + assert!(array.is_empty()); + Ok(()) + }); + } + + #[test] + fn full_length_result_is_not_narrowed() { + let harness = Harness::new(); + harness.run_in_context(|lock, _ctx| { + // Latin-1 -> UTF-16LE widens every byte to exactly two, so the + // conversion fills the destination exactly. + let array = + transcode_impl(lock, b"Hi", ffi::Encoding::Latin1, ffi::Encoding::Utf16Le).unwrap(); + assert_eq!(array.as_slice(), &[0x48, 0x00, 0x69, 0x00]); + Ok(()) + }); + } + + #[test] + fn failure_surfaces_as_an_error() { + let harness = Harness::new(); + harness.run_in_context(|lock, _ctx| { + // Odd-length UTF-16LE input is rejected before any allocation. + let result = transcode_impl(lock, &[0x61], ffi::Encoding::Utf16Le, ffi::Encoding::Utf8); + assert!(result.is_err()); + Ok(()) + }); } } diff --git a/src/rust/i18n/shim.h b/src/rust/i18n/shim.h index 82015bfcdcc..eb8f29cfae9 100644 --- a/src/rust/i18n/shim.h +++ b/src/rust/i18n/shim.h @@ -74,13 +74,17 @@ int64_t convert_ex(const Converter& to, int64_t from_uchars( const Converter& to, ::rust::Slice source, ::rust::Slice target); -// simdutf wrappers. Buffers holding UTF-16LE code units are passed as raw -// bytes and cast to `char16_t*` internally. +// simdutf wrappers, mirroring the four `simdutf::*` calls in `i18n.c++`. +// Buffers holding UTF-16LE code units are passed as raw bytes and cast to +// `char16_t*` internally, exactly as the C++ path does via +// `JsUint8Array::asArrayPtr()`. // -// `target` must be aligned for `char16_t`. Callers satisfy this by passing a -// JavaScript `ArrayBuffer`'s backing store, whose base address V8 aligns well -// past two bytes. `source` carries no such guarantee: it is caller-supplied -// buffer contents, which a `Uint8Array` can expose at an odd `byteOffset`. +// Neither buffer is required to be `char16_t`-aligned. `target` always is in +// practice, being a V8 backing store, but `source` is caller-supplied buffer +// contents that a `Uint8Array` can expose at an odd `byteOffset`. simdutf +// reads and writes through unaligned SIMD loads and stores, so this is +// well-defined for the simdutf entry points below; `from_uchars` casts to +// `UChar*` for ICU on the same basis as the C++ path. size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target); size_t utf16_length_from_utf8(::rust::Slice source); diff --git a/src/rust/i18n/shim.rs b/src/rust/i18n/shim.rs index e4128113d7f..8c6291082fe 100644 --- a/src/rust/i18n/shim.rs +++ b/src/rust/i18n/shim.rs @@ -15,10 +15,13 @@ use crate::ffi; /// /// Wraps a `cxx::UniquePtr`: the underlying `UConverter*` and /// its `ucnv_close()` teardown are owned entirely by the C++ shim, so the -/// converter is torn down correctly even if Rust code holding it panics. +/// converter is torn down correctly even if Rust code holding it panics -- +/// unlike a raw `UConverter*` smuggled across the FFI boundary, which a panic +/// could leak. pub struct Converter(cxx::UniquePtr); -/// Returns the ICU converter name for a transcodable encoding. +/// Returns the ICU converter name for a transcodable encoding, matching +/// `getEncodingName()` in `i18n.c++`. /// /// The bridge `Encoding` enum is a `cxx` shared enum, which is a `u8` newtype /// rather than a real Rust enum, so a value outside the four declared variants diff --git a/src/rust/jsg/ffi-inl.h b/src/rust/jsg/ffi-inl.h index 70eb9af97bd..9cc65aee465 100644 --- a/src/rust/jsg/ffi-inl.h +++ b/src/rust/jsg/ffi-inl.h @@ -50,6 +50,12 @@ inline MaybeLocal maybe_local_to_ffi(v8::MaybeLocal value) { return MaybeLocal{result}; } +template +inline v8::MaybeLocal maybe_local_from_ffi(MaybeLocal&& value) { + auto ptr_void = reinterpret_cast(&value.ptr); + return *reinterpret_cast*>(ptr_void); +} + // Global // // ffi::Global stores only the strong v8::Global in `ptr`. diff --git a/src/rust/jsg/ffi.c++ b/src/rust/jsg/ffi.c++ index a2f904a59aa..0f7bd6359ac 100644 --- a/src/rust/jsg/ffi.c++ +++ b/src/rust/jsg/ffi.c++ @@ -775,6 +775,15 @@ DEFINE_TYPED_ARRAY_UNWRAP(biguint64_array, BigUint64Array, uint64_t) } // Local +Local uint8_array_from_buffer( + Isolate* isolate, const Local& buffer, size_t byte_offset, size_t length) { + auto arrayBuffer = local_as_ref_from_ffi(buffer); + KJ_REQUIRE( + byte_offset <= arrayBuffer->ByteLength() && length <= arrayBuffer->ByteLength() - byte_offset, + "Uint8Array view is out of bounds of its ArrayBuffer"); + return to_ffi(v8::Uint8Array::New(arrayBuffer, byte_offset, length)); +} + size_t local_typed_array_length(Isolate* isolate, const Local& array) { return local_as_ref_from_ffi(array)->Length(); } diff --git a/src/rust/jsg/ffi.h b/src/rust/jsg/ffi.h index 16f56170280..99ff3d1157d 100644 --- a/src/rust/jsg/ffi.h +++ b/src/rust/jsg/ffi.h @@ -206,6 +206,13 @@ Local local_new_float32_array(Isolate* isolate, const float* data, size_t length Local local_new_float64_array(Isolate* isolate, const double* data, size_t length); Local local_new_bigint64_array(Isolate* isolate, const int64_t* data, size_t length); Local local_new_biguint64_array(Isolate* isolate, const uint64_t* data, size_t length); +// Creates a Uint8Array view over an existing ArrayBuffer, without copying. Unlike +// local_new_uint8_array, which allocates a backing store and memcpys into it, this +// lets a caller allocate the buffer up front, write into it directly, and only then +// wrap the written prefix in a view. `byte_offset + length` must be within +// `buffer`'s byte length. +Local uint8_array_from_buffer( + Isolate* isolate, const Local& buffer, size_t byte_offset, size_t length); size_t local_typed_array_length(Isolate* isolate, const Local& array); // Returns a raw pointer to the underlying ArrayBuffer's data (without byte offset). // Use local_typed_array_byte_offset to compute the start of this view's data. diff --git a/src/rust/jsg/v8.rs b/src/rust/jsg/v8.rs index f6c55ae49bd..bc96710fd00 100644 --- a/src/rust/jsg/v8.rs +++ b/src/rust/jsg/v8.rs @@ -403,6 +403,12 @@ pub mod ffi { ) -> Result>; // Local + pub unsafe fn uint8_array_from_buffer( + isolate: *mut Isolate, + buffer: &Local, + byte_offset: usize, + length: usize, + ) -> Local; pub unsafe fn local_typed_array_length(isolate: *mut Isolate, array: &Local) -> usize; pub unsafe fn local_typed_array_buffer_data(isolate: *mut Isolate, array: &Local) -> usize; pub unsafe fn local_typed_array_byte_offset(isolate: *mut Isolate, array: &Local) -> usize; @@ -2315,6 +2321,47 @@ impl_typed_array!(BigUint64Array, u64, local_biguint64_array_get); // Uint8ClampedArray has the same element type as Uint8Array; clamping is a write-side JS concern. impl_typed_array!(Uint8ClampedArray, u8, local_uint8clamped_array_get); +impl Uint8Array { + /// Creates a `Uint8Array` viewing `length` bytes of `buffer`, starting at + /// `byte_offset`. + /// + /// Zero-copy, unlike [`Vec::to_js`](crate::ToJS), which allocates a fresh + /// backing store and copies into it. Producing a `Uint8Array` from bytes computed + /// in Rust therefore does not require an intermediate `Vec`: allocate the buffer + /// with [`ArrayBuffer::new_with_mode`], fill it through + /// [`Local::::as_mut_slice`], and wrap the written prefix here. + /// + /// # Panics + /// + /// Panics if `byte_offset + length` exceeds `buffer`'s byte length. The + /// check is repeated in C++, but failing it here keeps the failure a Rust + /// panic rather than a `kj::Exception` thrown across the bridge, which + /// this function's signature cannot carry. + pub fn from_buffer<'a>( + lock: &mut crate::Lock, + buffer: &Local<'_, ArrayBuffer>, + byte_offset: usize, + length: usize, + ) -> Local<'a, Self> { + let byte_length = buffer.byte_length(); + assert!( + byte_offset <= byte_length && length <= byte_length - byte_offset, + "Uint8Array view [{byte_offset}, {byte_offset}+{length}) is out of bounds \ + of its {byte_length}-byte ArrayBuffer" + ); + let isolate = lock.isolate(); + // SAFETY: Lock guarantees the isolate is locked and a HandleScope is active; + // `buffer` is a live handle to an ArrayBuffer. The bounds precondition is + // checked on the C++ side. + unsafe { + Local::from_ffi( + isolate, + ffi::uint8_array_from_buffer(isolate.as_ffi(), &buffer.handle, byte_offset, length), + ) + } + } +} + // ============================================================================= // `String`-specific implementations // ============================================================================= diff --git a/src/workerd/api/node/i18n.c++ b/src/workerd/api/node/i18n.c++ index 90a93324e55..1b270023286 100644 --- a/src/workerd/api/node/i18n.c++ +++ b/src/workerd/api/node/i18n.c++ @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -23,17 +24,19 @@ namespace workerd::api::node { namespace rust_i18n = ::workerd::rust::i18n; -// Maps the C++ Encoding to the Rust bridge enum, so callers can write -// `kj::from(value)`; see `kj-rs/convert.h` for the -// `fromImpl` convention, and `exceptions.c++` for another instance of it. The -// switch has no `default:` arm so that enum drift between the two `Encoding` -// types is a compile error. +// Maps the C++ Encoding to the Rust bridge enum, following the kj-rs +// convert.h idiom (see `NODEJS_EXCEPTIONS_RUST`'s equivalent in +// exceptions.c++), so callers use `kj::from(value)`. It +// is `static` (rather than in an anonymous namespace) because Clang's ADL +// does not consider unnamed-namespace functions, and ADL is how +// `kj::from` locates this overload. The switch has no `default:` arm +// so that enum drift between the two `Encoding` types is a compile error. // // `BASE64`, `BASE64URL`, and `HEX` are not transcodable (see -// `i18n::canBeTranscoded`). Rejecting them here covers `toEncoding` as well as -// `fromEncoding`, unlike the C++ dispatch below, which checks only -// `fromEncoding` and reaches `KJ_UNREACHABLE` for a non-transcodable -// `toEncoding`. Neither is reachable from JavaScript because +// `i18n::canBeTranscoded`); rejecting them here, for both `fromEncoding` and +// `toEncoding`, is a deliberate divergence from the C++ dispatch below, which +// only checks `fromEncoding` and would reach `KJ_UNREACHABLE` for a +// non-transcodable `toEncoding`. Neither is reachable from JavaScript because // `BufferUtil::transcode` validates both encodings first. static rust_i18n::Encoding fromImpl(rust_i18n::Encoding*, Encoding encoding) { switch (encoding) { @@ -276,18 +279,13 @@ void Converter::setSubstituteChars(kj::StringPtr sub) { jsg::JsUint8Array transcode( jsg::Lock& js, kj::ArrayPtr source, Encoding fromEncoding, Encoding toEncoding) { if (util::Autogate::isEnabled(util::AutogateKey::NODEJS_I18N_RUST)) { - auto rustSource = source.as(); - auto transcoder = rust_i18n::new_transcoder(rustSource, - kj::from(fromEncoding), kj::from(toEncoding)); - - // The destination is sized for the worst case, so the conversion often - // writes less than it was given; narrow the result to what was written. - auto out = jsg::JsUint8Array::create(js, transcoder->dest_len()); - auto written = transcoder->run(rustSource, out.asArrayPtr().as()); - if (written < out.size()) { - return out.slice(js, written); - } - return out; + auto rustFrom = kj::from(fromEncoding); + auto rustTo = kj::from(toEncoding); + auto maybeLocal = + rust_i18n::transcode(js.v8Isolate, source.as(), rustFrom, rustTo); + auto local = + jsg::check(::workerd::rust::jsg::maybe_local_from_ffi(kj::mv(maybeLocal))); + return jsg::JsUint8Array(local); } TranscodeImpl transcode_function = &TranscodeDefault; From a8cd5ceb26cedc92e906c027bed0a341308b4339 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Thu, 20 Aug 2026 11:16:25 -0500 Subject: [PATCH 05/10] deslop --- src/rust/i18n/dispatch.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/rust/i18n/dispatch.rs b/src/rust/i18n/dispatch.rs index 132fdb87fc0..59bd9ef99d3 100644 --- a/src/rust/i18n/dispatch.rs +++ b/src/rust/i18n/dispatch.rs @@ -14,11 +14,6 @@ //! how many bytes it actually wrote, which is often fewer because the //! destination is sized for the worst case. //! -//! Splitting sizing from writing is what lets [`crate::transcode`] allocate -//! the destination as a V8 `ArrayBuffer` and convert straight into its backing -//! store, with no intermediate `Vec` and no copy. Nothing in this module -//! touches V8, so it stays unit-testable against a plain `Vec` destination. -//! //! All sizing, validation, substitute-character setup, and length checking //! happens here; [`crate::shim`] only forwards to the underlying ICU and //! simdutf calls. From 0fb6394cc87b71049a84fbff43e38eb2f3fedd9a Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Thu, 20 Aug 2026 15:06:49 -0500 Subject: [PATCH 06/10] Remove patch --- build/deps/v8.MODULE.bazel | 1 - ...-icuuc-bazel-target-publicly-visible.patch | 22 ------------------- src/rust/i18n/BUILD.bazel | 11 ++++++---- 3 files changed, 7 insertions(+), 27 deletions(-) delete mode 100644 patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch diff --git a/build/deps/v8.MODULE.bazel b/build/deps/v8.MODULE.bazel index 7c557806f6a..de1dc53d42f 100644 --- a/build/deps/v8.MODULE.bazel +++ b/build/deps/v8.MODULE.bazel @@ -62,7 +62,6 @@ PATCHES = [ "0037-Fix-CFunction-MemorySpan-declarations-on-Windows.patch", "0038-Properly-depend-on-llvm-libc.patch", "0039-wasm-memory.discard-prototype-for-the-memory-control.patch", - "0040-Make-icuuc-bazel-target-publicly-visible.patch", ] http_archive( diff --git a/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch b/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch deleted file mode 100644 index d63cfcb3644..00000000000 --- a/patches/v8/0040-Make-icuuc-bazel-target-publicly-visible.patch +++ /dev/null @@ -1,22 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Logan Gatlin -Date: Wed, 12 Aug 2026 00:00:00 +0000 -Subject: Make icuuc bazel target publicly visible - -Only the `icu` umbrella target (which also pulls in `icui18n`) is public. -Consumers that need just the ICU common library -- e.g. a converter-only -shim -- should be able to depend on `icuuc` directly instead of picking up -`icui18n` transitively. - -diff --git a/bazel/BUILD.icu b/bazel/BUILD.icu -index de8e20ac3ad50fe2199d3b56753e655d1355b19f..1111111111111111111111111111111111111111 100644 ---- a/bazel/BUILD.icu -+++ b/bazel/BUILD.icu -@@ -15,6 +15,7 @@ cc_library( - "source/common/**/*.h", - "source/common/**/*.cpp", - ]), -+ visibility = ["//visibility:public"], - copts = select({ - "@platforms//os:windows": [ - "/wd4005", # Macro redefinition. diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index 405c7ef195b..e21763ce94f 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -5,18 +5,21 @@ load("//:build/wd_rust_crate.bzl", "wd_rust_crate") # functions that `workerd::api::node::i18n::transcode` uses, so the Rust # implementation of `transcode` (in `lib.rs` and friends) calls the exact same # codecs as the C++ path in `src/workerd/api/node/i18n.c++` instead of -# reimplementing them. Declares its codec dependencies explicitly rather than -# relying on them arriving transitively through V8. +# reimplementing them. ICU comes from `@workerd-v8//:v8`, the seam every other +# workerd target uses for it; depending on the ICU repository directly breaks +# builds that substitute their own V8/ICU for that module. wd_cc_library( name = "shim", srcs = ["shim.c++"], hdrs = ["shim.h"], + implementation_deps = [ + "@simdutf", + "@workerd-v8//:v8", + ], visibility = ["//visibility:public"], deps = [ "//src/rust/cxx:core", "@capnp-cpp//src/kj", - "@com_googlesource_chromium_icu//:icuuc", - "@simdutf", ], ) From f2f30d3422f5adfbb7a3416b114c35005635c648 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Fri, 21 Aug 2026 12:02:33 -0500 Subject: [PATCH 07/10] remove shim --- src/rust/AGENTS.md | 2 + src/rust/cxx/PATCHES.md | 9 + src/rust/cxx/gen/src/write.rs | 5 +- src/rust/cxx/src/cxx.cc | 12 ++ src/rust/cxx/src/lib.rs | 20 ++ src/rust/cxx/src/symbols/rust_vec.rs | 5 + src/rust/cxx/syntax/atom.rs | 6 + src/rust/cxx/syntax/check.rs | 12 +- src/rust/cxx/syntax/names.rs | 38 ++++ src/rust/cxx/syntax/namespace.rs | 5 + src/rust/cxx/syntax/pod.rs | 4 +- src/rust/cxx/syntax/tokens.rs | 5 + src/rust/cxx/tests/cxx_gen.rs | 54 ++++++ src/rust/cxx/tests/ffi/lib.rs | 17 ++ src/rust/cxx/tests/ffi/tests.cc | 30 +++ src/rust/cxx/tests/ffi/tests.h | 4 + src/rust/cxx/tests/test.rs | 13 ++ src/rust/i18n/BUILD.bazel | 36 ++-- src/rust/i18n/codecs.rs | 270 +++++++++++++++++++++++++++ src/rust/i18n/dispatch.rs | 20 +- src/rust/i18n/lib.rs | 106 +++++++++-- src/rust/i18n/shim.c++ | 98 ---------- src/rust/i18n/shim.h | 95 ---------- src/rust/i18n/shim.rs | 123 ------------ 24 files changed, 613 insertions(+), 376 deletions(-) create mode 100644 src/rust/i18n/codecs.rs delete mode 100644 src/rust/i18n/shim.c++ delete mode 100644 src/rust/i18n/shim.h delete mode 100644 src/rust/i18n/shim.rs diff --git a/src/rust/AGENTS.md b/src/rust/AGENTS.md index f80bd19bb63..225ef4732a8 100644 --- a/src/rust/AGENTS.md +++ b/src/rust/AGENTS.md @@ -117,6 +117,8 @@ If a C++ library already depends on your crate (C++ → Rust) and you also add a cxx also supports **reusing a binding type across bridges** ([docs](https://cxx.rs/extern-c++.html#reusing-existing-binding-types)): the `worker` crate's `error.rs` / `ok.rs` / `kill_switch.rs` bridges reuse `ffi.rs`'s types by depending on `:ffi.rs@cxx`. Still, keep a struct that only crosses FFI within one crate in that crate's bridge. +**UTF-16 crosses the FFI as `c_char16`, not `u16`.** `u16` in a bridge means C++ `uint16_t`; `cxx::c_char16` means `char16_t`. The two are layout-identical but distinct C++ types, so only `c_char16` resolves to a `char16_t` overload or matches a `char16_t*` parameter. On the Rust side `c_char16` is an alias for `u16`, so `&[c_char16]` accepts a `&[u16]` from `str::encode_utf16` or `encoding_rs` with no cast. See the doc comment on `cxx::c_char16` for the container limits the alias implies. + **V8 handles must always cross the FFI as the shared `jsg::v8::ffi` types, never as a bare `usize`.** When another crate's bridge passes a V8 `Local`/`Global`, reuse the jsg shared struct via a type alias (`type Local = jsg::v8::ffi::Local;`) plus `include!("workerd/rust/jsg/v8.rs.h")`, and depend on `//src/rust/jsg` (which supplies the generated header transitively). Do not smuggle the handle word through a `usize` — the shared type keeps both sides in one canonical, cxx-verified definition. See `node-exceptions/lib.rs`. ### Testing crates that cross the FFI diff --git a/src/rust/cxx/PATCHES.md b/src/rust/cxx/PATCHES.md index 0385246b7d7..05c54583318 100644 --- a/src/rust/cxx/PATCHES.md +++ b/src/rust/cxx/PATCHES.md @@ -13,5 +13,14 @@ The in-tree fork contains these major behavioral changes: `kj::Maybe`, `kj::Date`, exceptions, promises, and Rust futures. - **`__WORKERD_CXX__`.** This preprocessor definition identifies the fork even though its crate and generated symbol names remain compatible with cxx. +- **`c_char16`.** An additional builtin type, spelled `char16_t` in C++, for UTF-16 data. `u16` + means `uint16_t`, which C++ mangles and overloads separately from `char16_t`, so a bridge that + must name `char16_t` cannot use `u16`. The Rust side is an alias for `u16`, following `c_char`, + so no cast is needed at call sites; see the doc comment on `cxx::c_char16` for the limits that + follow from being an alias. +- **Unqualified emission of C++ fundamental types.** A `Pair` whose name is a fundamental type and + whose namespace is empty is written bare rather than with a leading `::`, because those names are + keywords: `::char16_t` is ill-formed where `::uint16_t` is fine. This lets an `ExternType` alias + bind a fundamental type that has no builtin spelling in a bridge. - **Workerd build integration.** Bazel targets use workerd's in-tree Rust toolchain, crate vendor repository, Cap'n Proto/KJ dependency, formatting, and lint configuration. diff --git a/src/rust/cxx/gen/src/write.rs b/src/rust/cxx/gen/src/write.rs index 19cda863dc9..c5ae0dcaaa2 100644 --- a/src/rust/cxx/gen/src/write.rs +++ b/src/rust/cxx/gen/src/write.rs @@ -242,7 +242,9 @@ fn pick_includes_and_builtins(out: &mut OutFile, apis: &[Api]) { Some(Isize) => out.builtin.rust_isize = true, Some(CxxString) => out.include.string = true, Some(RustString) => out.builtin.rust_string = true, - Some(Bool | Char | F32 | F64) | None => {} + // bool, char, char16_t, float and double are C++ keywords; no + // header supplies them. + Some(Bool | Char | Char16 | F32 | F64) | None => {} }, Type::RustBox(_) => out.builtin.rust_box = true, Type::RustVec(_) => out.builtin.rust_vec = true, @@ -1374,6 +1376,7 @@ fn write_atom(out: &mut OutFile, atom: Atom) { match atom { Bool => write!(out, "bool"), Char => write!(out, "char"), + Char16 => write!(out, "char16_t"), U8 => write!(out, "::std::uint8_t"), U16 => write!(out, "::std::uint16_t"), U32 => write!(out, "::std::uint32_t"), diff --git a/src/rust/cxx/src/cxx.cc b/src/rust/cxx/src/cxx.cc index 7384d28992d..afc447581f2 100644 --- a/src/rust/cxx/src/cxx.cc +++ b/src/rust/cxx/src/cxx.cc @@ -437,6 +437,14 @@ static_assert(sizeof(rust::isize) == sizeof(std::intptr_t), static_assert(alignof(rust::isize) == alignof(std::intptr_t), "unsupported ssize_t alignment"); +// The cxx crate spells char16_t as `c_char16`, an alias for u16. char16_t's +// underlying type is uint_least16_t, which the standard permits to be wider +// than 16 bits; on such a target the alias would misdescribe the C++ type. +static_assert(sizeof(char16_t) == sizeof(std::uint16_t), + "unsupported char16_t size"); +static_assert(alignof(char16_t) == alignof(std::uint16_t), + "unsupported char16_t alignment"); + static_assert(std::is_trivially_copy_constructible::value, "trivial Str(const Str &)"); static_assert(std::is_trivially_copy_assignable::value, @@ -827,10 +835,14 @@ static_assert(sizeof(std::string) <= kMaxExpectedWordsInString * sizeof(void *), FOR_EACH_TRIVIAL_STD_VECTOR(MACRO) \ MACRO(string, std::string) +// char16_t needs no _if_unique guard: [basic.fundamental] makes it a distinct +// type from every other fundamental type, so rust::Vec can never +// collide with another specialization. #define FOR_EACH_RUST_VEC(MACRO) \ FOR_EACH_NUMERIC(MACRO) \ MACRO(bool, bool) \ MACRO(char, rust::detail::char_if_unique) \ + MACRO(char16_t, char16_t) \ MACRO(usize, rust::detail::usize_if_unique) \ MACRO(isize, rust::detail::isize_if_unique) \ MACRO(string, rust::String) \ diff --git a/src/rust/cxx/src/lib.rs b/src/rust/cxx/src/lib.rs index 993f5f90a49..fb5c62d9ac6 100644 --- a/src/rust/cxx/src/lib.rs +++ b/src/rust/cxx/src/lib.rs @@ -467,6 +467,26 @@ pub type String = CxxString; /// import and use `CxxVector`. pub type Vector = CxxVector; +/// Rust spelling of C++'s `char16_t`, for UTF-16 data crossing the bridge. +/// +/// `char16_t` and `uint16_t` are layout-identical but distinct C++ types, so +/// C++ mangles them differently and overloads on them separately. `u16` in a +/// bridge means `uint16_t`; use `c_char16` where the signature must name +/// `char16_t`, which is the usual case for UTF-16 buffers. +/// +/// This is a plain alias rather than a newtype, following `c_char`, so a +/// `&[c_char16]` argument accepts a `&[u16]` from `str::encode_utf16` or +/// `encoding_rs` with no cast. The alias means `c_char16` and `u16` are one +/// Rust type; only their C++ spelling differs. The two therefore cannot both +/// be used as the element type of a `CxxVector` in the same program, and +/// `CxxVector` is rejected for the same reason `CxxVector` +/// is. +/// +/// `char16_t` is guaranteed by the C++ standard to be a distinct type whose +/// underlying type is `uint_least16_t`. `cxx.cc` asserts that it really is 16 +/// bits wide on the target, which is what makes this alias sound. +pub type c_char16 = u16; + // Not public API. #[doc(hidden)] pub mod private { diff --git a/src/rust/cxx/src/symbols/rust_vec.rs b/src/rust/cxx/src/symbols/rust_vec.rs index 23540f0796c..52b3981ea34 100644 --- a/src/rust/cxx/src/symbols/rust_vec.rs +++ b/src/rust/cxx/src/symbols/rust_vec.rs @@ -3,6 +3,7 @@ use core::ffi::c_char; use core::mem; use core::ptr; +use crate::c_char16; use crate::rust_string::RustString; use crate::rust_vec::RustVec; @@ -78,5 +79,9 @@ rust_vec_shims_for_primitive!(f32); rust_vec_shims_for_primitive!(f64); rust_vec_shims!("char", c_char); +// c_char16 is an alias for u16, so these duplicate the u16 shims above under a +// second set of export names. The C++ side reaches them through +// rust::Vec, which mangles to the char16_t segment. +rust_vec_shims!("char16_t", c_char16); rust_vec_shims!("string", RustString); rust_vec_shims!("str", &str); diff --git a/src/rust/cxx/syntax/atom.rs b/src/rust/cxx/syntax/atom.rs index 135f7ba4d56..2982904deda 100644 --- a/src/rust/cxx/syntax/atom.rs +++ b/src/rust/cxx/syntax/atom.rs @@ -9,6 +9,10 @@ use crate::Type; pub enum Atom { Bool, Char, // C char, not Rust char + // C++ char16_t. Layout-identical to u16 but a distinct C++ type, so it is a + // distinct atom: it exists to make overload resolution and name mangling on + // the C++ side pick char16_t rather than uint16_t. + Char16, U8, U16, U32, @@ -35,6 +39,7 @@ impl Atom { match s { "bool" => Some(Bool), "c_char" => Some(Char), + "c_char16" => Some(Char16), "u8" => Some(U8), "u16" => Some(U16), "u32" => Some(U32), @@ -66,6 +71,7 @@ impl AsRef for Atom { match self { Bool => "bool", Char => "c_char", + Char16 => "c_char16", U8 => "u8", U16 => "u16", U32 => "u32", diff --git a/src/rust/cxx/syntax/check.rs b/src/rust/cxx/syntax/check.rs index d15a7a09a24..aa4a5854d9b 100644 --- a/src/rust/cxx/syntax/check.rs +++ b/src/rust/cxx/syntax/check.rs @@ -163,8 +163,8 @@ fn check_type_rust_vec(cx: &mut Check, ty: &Ty1) { match Atom::from(&ident.rust) { None | Some( - Bool | Char | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 | Isize | F32 - | F64 | RustString, + Bool | Char | Char16 | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 + | Isize | F32 | F64 | RustString, ) => return, Some(CxxString) => {} } @@ -266,7 +266,7 @@ fn check_type_shared_ptr(cx: &mut Check, ptr: &Ty1) { Bool | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 | Isize | F32 | F64 | CxxString, ) => return, - Some(Char | RustString) => {} + Some(Char | Char16 | RustString) => {} } } else if let Type::CxxVector(_) = &ptr.inner { cx.error(ptr, "std::shared_ptr is not supported yet"); @@ -289,7 +289,7 @@ fn check_type_weak_ptr(cx: &mut Check, ptr: &Ty1) { Bool | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 | Isize | F32 | F64 | CxxString, ) => return, - Some(Char | RustString) => {} + Some(Char | Char16 | RustString) => {} } } else if let Type::CxxVector(_) = &ptr.inner { cx.error(ptr, "std::weak_ptr is not supported yet"); @@ -363,7 +363,7 @@ fn check_type_cxx_vector(cx: &mut Check, ptr: &Ty1) { | Some( U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 | Isize | F32 | F64 | CxxString, ) => return, - Some(Char) => { /* todo */ } + Some(Char | Char16) => { /* todo */ } Some(Bool | RustString) => {} } } @@ -900,6 +900,8 @@ fn describe(cx: &mut Check, ty: &Type) -> String { "C++ string".to_owned() } else if Atom::from(&ident.rust) == Some(Char) { "C char".to_owned() + } else if Atom::from(&ident.rust) == Some(Char16) { + "C++ char16_t".to_owned() } else { ident.rust.to_string() } diff --git a/src/rust/cxx/syntax/names.rs b/src/rust/cxx/syntax/names.rs index c62d384021f..acf8679feb0 100644 --- a/src/rust/cxx/syntax/names.rs +++ b/src/rust/cxx/syntax/names.rs @@ -32,6 +32,15 @@ impl Pair { } pub fn to_fully_qualified(&self) -> String { + // A fundamental type's name is a keyword, not something declared in a + // scope, so it is spelled bare: `::char16_t` is ill-formed where + // `::uint16_t` is fine. Reaching this with a non-empty namespace means + // the name was not really a fundamental type, so leave it qualified + // and let C++ report it. + if self.namespace.is_empty() && self.cxx.is_fundamental() { + return self.cxx.to_string(); + } + let mut fully_qualified = String::new(); for segment in &self.namespace { fully_qualified += "::"; @@ -72,6 +81,35 @@ impl ForeignName { Err(err) => Err(Error::new(span, err)), } } + + /// True if this names a C++ fundamental type. + /// + /// These are the only C++ type names that are keywords, and so the only + /// ones that cannot be written with a `::` qualification. Fixed-width + /// names like `uint16_t` are ordinary typedefs, not keywords, and are + /// deliberately absent. + /// + /// Multi-token spellings such as `unsigned int` are absent because + /// [`ForeignName::parse`] cannot represent a name containing whitespace. + pub fn is_fundamental(&self) -> bool { + matches!( + self.text.as_str(), + "bool" + | "char" + | "char8_t" + | "char16_t" + | "char32_t" + | "double" + | "float" + | "int" + | "long" + | "short" + | "signed" + | "unsigned" + | "void" + | "wchar_t" + ) + } } impl Display for ForeignName { diff --git a/src/rust/cxx/syntax/namespace.rs b/src/rust/cxx/syntax/namespace.rs index da8a335da68..98c1d36be69 100644 --- a/src/rust/cxx/syntax/namespace.rs +++ b/src/rust/cxx/syntax/namespace.rs @@ -33,6 +33,11 @@ impl Namespace { self.segments.iter() } + /// True for the global namespace. + pub fn is_empty(&self) -> bool { + self.segments.is_empty() + } + pub fn parse_bridge_attr_namespace(input: ParseStream) -> Result { if input.is_empty() { return Ok(Self::ROOT); diff --git a/src/rust/cxx/syntax/pod.rs b/src/rust/cxx/syntax/pod.rs index e8a50730fce..1e5598c1905 100644 --- a/src/rust/cxx/syntax/pod.rs +++ b/src/rust/cxx/syntax/pod.rs @@ -12,8 +12,8 @@ impl Types<'_> { let ident = &ident.rust; if let Some(atom) = Atom::from(ident) { match atom { - Bool | Char | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 | I64 - | Isize | F32 | F64 => true, + Bool | Char | Char16 | U8 | U16 | U32 | U64 | Usize | I8 | I16 | I32 + | I64 | Isize | F32 | F64 => true, CxxString | RustString => false, } } else if let Some(strct) = self.structs.get(ident) { diff --git a/src/rust/cxx/syntax/tokens.rs b/src/rust/cxx/syntax/tokens.rs index eabb34f5576..a8d5644b683 100644 --- a/src/rust/cxx/syntax/tokens.rs +++ b/src/rust/cxx/syntax/tokens.rs @@ -36,6 +36,11 @@ impl ToTokens for Type { if ident.rust == Char { let span = ident.rust.span(); tokens.extend(quote_spanned!(span=> ::cxx::core::ffi::)); + } else if ident.rust == Char16 { + // Unlike c_char, there is no core::ffi equivalent of + // char16_t; the cxx crate defines c_char16 itself. + let span = ident.rust.span(); + tokens.extend(quote_spanned!(span=> ::cxx::)); } else if ident.rust == CxxString { let span = ident.rust.span(); tokens.extend(quote_spanned!(span=> ::cxx::)); diff --git a/src/rust/cxx/tests/cxx_gen.rs b/src/rust/cxx/tests/cxx_gen.rs index e9f68414c12..3f6122cbe11 100644 --- a/src/rust/cxx/tests/cxx_gen.rs +++ b/src/rust/cxx/tests/cxx_gen.rs @@ -88,6 +88,33 @@ const BRIDGE4: &str = r#" } "#; +const BRIDGE5: &str = r#" + #[cxx::bridge] + mod ffi { + unsafe extern "C++" { + fn utf16_length(s: &[c_char16]) -> usize; + fn widen(input: &[u8], output: &mut [c_char16]) -> usize; + unsafe fn first(s: *const c_char16) -> c_char16; + } + + extern "Rust" { + fn encode(s: &str) -> Vec; + } + } +"#; + +const BRIDGE6: &str = r#" + #[cxx::bridge] + mod ffi { + unsafe extern "C++" { + #[cxx_name = "char32_t"] + type Char32 = crate::Char32; + + fn count(s: &[Char32]) -> usize; + } + } +"#; + #[test] fn test_extern_c_function() { let opt = cxx_gen::Opt::default(); @@ -177,3 +204,30 @@ fn test_kj_arc_in_shared_struct() { let expected = "::rust::ManuallyDrop<::Holder> holder$(::std::move(holder));"; assert!(implementation.contains(expected)); } + +#[test] +fn test_c_char16_maps_to_cxx_char16_t() { + let opt = cxx_gen::Opt::default(); + let source = BRIDGE5.parse().unwrap(); + let generated = generate_header_and_cc(source, &opt).unwrap(); + let implementation = str::from_utf8(&generated.implementation).unwrap(); + assert!(implementation.contains("::rust::Slice")); + assert!(implementation.contains("::rust::Slice")); + assert!(implementation.contains("char16_t const *")); + assert!(implementation.contains("::rust::Vec")); + // char16_t must not decay to uint16_t: keeping the two distinct is the + // whole reason c_char16 is a separate atom from u16. + assert!(!implementation.contains("uint16_t")); +} + +#[test] +fn test_fundamental_type_name_is_unqualified() { + let opt = cxx_gen::Opt::default(); + let source = BRIDGE6.parse().unwrap(); + let generated = generate_header_and_cc(source, &opt).unwrap(); + let implementation = str::from_utf8(&generated.implementation).unwrap(); + assert!(implementation.contains("::rust::Slice")); + // `::char32_t` is ill-formed; a fundamental type's name is a keyword and + // cannot be qualified. + assert!(!implementation.contains("::char32_t")); +} diff --git a/src/rust/cxx/tests/ffi/lib.rs b/src/rust/cxx/tests/ffi/lib.rs index 03af290cc69..f10798c8ef7 100644 --- a/src/rust/cxx/tests/ffi/lib.rs +++ b/src/rust/cxx/tests/ffi/lib.rs @@ -29,6 +29,7 @@ use cxx::KjError; use cxx::KjExceptionType; use cxx::SharedPtr; use cxx::UniquePtr; +use cxx::c_char16; use cxx::type_id; // The bridge parser accepts the unqualified smart-pointer name, while expansion // fully qualifies the emitted Rust field type. @@ -153,6 +154,8 @@ pub mod ffi { unsafe fn c_return_mut<'a>(shared: &'a mut Shared) -> &'a mut usize; unsafe fn c_return_str<'a>(shared: &'a Shared) -> &'a str; unsafe fn c_return_slice_char<'a>(shared: &'a Shared) -> &'a [c_char]; + unsafe fn c_return_slice_char16<'a>(shared: &'a Shared) -> &'a [c_char16]; + fn c_return_rust_vec_char16() -> Vec; unsafe fn c_return_mutsliceu8<'a>(slice: &'a mut [u8]) -> &'a mut [u8]; unsafe fn c_return_ref<'a>(shared: &'a Shared) -> &'a usize; fn c_return_rust_string() -> String; @@ -245,6 +248,8 @@ pub mod ffi { fn c_take_ref_c(c: &C); fn c_take_str(s: &str); fn c_take_slice_char(s: &[c_char]); + fn c_take_slice_char16(s: &[c_char16]); + fn c_take_rust_vec_char16(v: Vec); fn c_take_slice_shared(s: &[Shared]); fn c_take_slice_shared_sort(s: &mut [Shared]); fn c_take_slice_r(s: &[R]); @@ -430,6 +435,8 @@ pub mod ffi { fn r_take_ref_c(c: &C); fn r_take_str(s: &str); fn r_take_slice_char(s: &[c_char]); + fn r_take_slice_char16(s: &[c_char16]); + fn r_return_rust_vec_char16() -> Vec; fn r_take_rust_string(s: String); fn r_take_unique_ptr_string(s: UniquePtr); fn r_take_ref_vector(v: &CxxVector); @@ -758,6 +765,16 @@ fn r_take_slice_char(s: &[c_char]) { assert_eq!(std::str::from_utf8(s), Ok("2020\0")); } +// c_char16 is an alias for u16, so a &[c_char16] is a &[u16] and UTF-16 from +// the standard library needs no conversion to cross the bridge. +fn r_take_slice_char16(s: &[c_char16]) { + assert_eq!(String::from_utf16_lossy(s), "2020"); +} + +fn r_return_rust_vec_char16() -> Vec { + "2020".encode_utf16().collect() +} + fn r_take_unique_ptr_string(s: UniquePtr) { assert_eq!( s.as_ref().and_then(|value| value.to_str().ok()), diff --git a/src/rust/cxx/tests/ffi/tests.cc b/src/rust/cxx/tests/ffi/tests.cc index ea6de70cca6..950afb5e2b2 100644 --- a/src/rust/cxx/tests/ffi/tests.cc +++ b/src/rust/cxx/tests/ffi/tests.cc @@ -24,6 +24,7 @@ extern "C" bool cxx_test_suite_r_is_correct(const tests::R *) noexcept; namespace tests { static constexpr char SLICE_DATA[] = "2020"; +static constexpr char16_t SLICE_DATA16[] = u"2020"; static_assert(sizeof(SharedWithKjOwn) == sizeof(kj::Own)); static_assert(alignof(SharedWithKjOwn) == alignof(kj::Own)); @@ -153,6 +154,21 @@ rust::Slice c_return_slice_char(const Shared &shared) { return rust::Slice(SLICE_DATA, sizeof(SLICE_DATA)); } +rust::Slice c_return_slice_char16(const Shared &shared) { + (void)shared; + // Excludes the trailing NUL, unlike c_return_slice_char, so the Rust side can + // compare against "2020" directly. + return rust::Slice(SLICE_DATA16, 4); +} + +rust::Vec c_return_rust_vec_char16() { + rust::Vec vec; + for (char16_t c : rust::Slice(SLICE_DATA16, 4)) { + vec.push_back(c); + } + return vec; +} + rust::Slice c_return_mutsliceu8(rust::Slice slice) { return slice; } @@ -517,6 +533,18 @@ void c_take_slice_char(rust::Slice s) { } } +void c_take_slice_char16(rust::Slice s) { + if (std::u16string(s.data(), s.size()) == u"2020") { + cxx_test_suite_set_correct(); + } +} + +void c_take_rust_vec_char16(rust::Vec v) { + if (std::u16string(v.data(), v.size()) == u"2020") { + cxx_test_suite_set_correct(); + } +} + void c_take_slice_shared(rust::Slice s) { if (s.size() == 2 && s.data()->z == 2020 && s[1].z == 2021 && s.at(1).z == 2021 && s.front().z == 2020 && s.back().z == 2021) { @@ -1090,6 +1118,8 @@ extern "C" const char *cxx_run_test() noexcept { r_take_ref_c(C{2020}); r_take_str(rust::Str("2020")); r_take_slice_char(rust::Slice(SLICE_DATA, sizeof(SLICE_DATA))); + r_take_slice_char16(rust::Slice(SLICE_DATA16, 4)); + ASSERT(std::u16string(r_return_rust_vec_char16().data(), 4) == u"2020"); r_take_rust_string(rust::String("2020")); r_take_unique_ptr_string( std::unique_ptr(new std::string("2020"))); diff --git a/src/rust/cxx/tests/ffi/tests.h b/src/rust/cxx/tests/ffi/tests.h index 556407d44ba..b04da061332 100644 --- a/src/rust/cxx/tests/ffi/tests.h +++ b/src/rust/cxx/tests/ffi/tests.h @@ -151,6 +151,8 @@ const size_t &c_return_nested_ns_ref(const ::A::B::ABShared &shared); size_t &c_return_mut(Shared &shared); rust::Str c_return_str(const Shared &shared); rust::Slice c_return_slice_char(const Shared &shared); +rust::Slice c_return_slice_char16(const Shared &shared); +rust::Vec c_return_rust_vec_char16(); rust::Slice c_return_mutsliceu8(rust::Slice slice); rust::String c_return_rust_string(); rust::String c_return_rust_string_lossy(); @@ -230,6 +232,8 @@ void c_take_ref_c(const C &c); void c_take_ref_ns_c(const ::H::H &h); void c_take_str(rust::Str s); void c_take_slice_char(rust::Slice s); +void c_take_slice_char16(rust::Slice s); +void c_take_rust_vec_char16(rust::Vec v); void c_take_slice_shared(rust::Slice s); void c_take_slice_shared_sort(rust::Slice s); void c_take_slice_r(rust::Slice s); diff --git a/src/rust/cxx/tests/test.rs b/src/rust/cxx/tests/test.rs index cb5b6089226..db7660856e1 100644 --- a/src/rust/cxx/tests/test.rs +++ b/src/rust/cxx/tests/test.rs @@ -61,7 +61,17 @@ fn test_c_return() { b"2020\0", cast::c_char_to_unsigned(ffi::c_return_slice_char(&shared)), ); + // c_char16 is an alias for u16, so a rust::Slice from + // C++ arrives as a plain &[u16] with no cast. + assert_eq!( + String::from_utf16(ffi::c_return_slice_char16(&shared)).unwrap(), + "2020", + ); } + assert_eq!( + String::from_utf16(&ffi::c_return_rust_vec_char16()).unwrap(), + "2020", + ); assert_eq!("2020", ffi::c_return_rust_string()); assert_eq!("Hello \u{fffd}World", ffi::c_return_rust_string_lossy()); assert_eq!("2020", ffi::c_return_unique_ptr_string().to_str().unwrap()); @@ -350,6 +360,9 @@ fn test_c_take() { check!(cxx_test_suite::module::ffi::c_take_unique_ptr(unique_ptr)); check!(ffi::c_take_str("2020")); check!(ffi::c_take_slice_char(cast::unsigned_to_c_char(b"2020"))); + let utf16: Vec = "2020".encode_utf16().collect(); + check!(ffi::c_take_slice_char16(&utf16)); + check!(ffi::c_take_rust_vec_char16(utf16)); check!(ffi::c_take_slice_shared(&[ ffi::Shared { z: 2020 }, ffi::Shared { z: 2021 }, diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index e21763ce94f..4e91e81e581 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -1,36 +1,22 @@ -load("//:build/wd_cc_library.bzl", "wd_cc_library") load("//:build/wd_rust_crate.bzl", "wd_rust_crate") -# C++ shim exposing the ICU `ucnv_*` primitives and the simdutf conversion -# functions that `workerd::api::node::i18n::transcode` uses, so the Rust -# implementation of `transcode` (in `lib.rs` and friends) calls the exact same -# codecs as the C++ path in `src/workerd/api/node/i18n.c++` instead of -# reimplementing them. ICU comes from `@workerd-v8//:v8`, the seam every other -# workerd target uses for it; depending on the ICU repository directly breaks -# builds that substitute their own V8/ICU for that module. -wd_cc_library( - name = "shim", - srcs = ["shim.c++"], - hdrs = ["shim.h"], - implementation_deps = [ - "@simdutf", - "@workerd-v8//:v8", - ], - visibility = ["//visibility:public"], - deps = [ - "//src/rust/cxx:core", - "@capnp-cpp//src/kj", - ], -) - +# The bridge in `lib.rs` binds ICU's `ucnv_*` and simdutf's conversion +# functions directly, so the crate needs their headers to generate the bridge +# and their symbols at link time. ICU comes from `@workerd-v8//:v8`, the seam +# every other workerd target uses for it; depending on the ICU repository +# directly breaks builds that substitute their own V8/ICU for that module. wd_rust_crate( name = "i18n", cxx_bridge_deps = [ - ":shim", "//src/rust/jsg", + "@simdutf", + "@workerd-v8//:v8", ], cxx_bridge_src = "lib.rs", - link_deps = [":shim"], + link_deps = [ + "@simdutf", + "@workerd-v8//:v8", + ], test_deps = ["//src/rust/jsg-test"], visibility = ["//visibility:public"], deps = [ diff --git a/src/rust/i18n/codecs.rs b/src/rust/i18n/codecs.rs new file mode 100644 index 00000000000..bb3bb78c61c --- /dev/null +++ b/src/rust/i18n/codecs.rs @@ -0,0 +1,270 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +//! Safe wrappers around the ICU and simdutf primitives declared in +//! [`crate::ffi`], which are raw C and C++ entry points taking bare pointers. +//! +//! Everything unsafe about calling them lives here: deriving pointers and +//! lengths from slices, owning the `UConverter`, and turning their C-ish +//! reporting conventions (an out-parameter `UErrorCode`, `-1`, `0`) into +//! `Option` and `Result`. The transcoding *logic* -- dispatch, sizing, +//! substitute-character setup, truncation -- lives in [`crate::dispatch`]. + +use std::ffi::CStr; + +use crate::error::TranscodeError; +use crate::ffi; + +/// ICU's `UErrorCode`, a C enum whose underlying type is `int`. +/// +/// ICU reports failure through an out parameter of this type rather than a +/// return value, and requires it to be zeroed before each call that is not +/// continuing a previous one. +#[repr(transparent)] +#[derive(Debug, Default, Copy, Clone, PartialEq, Eq)] +pub struct UErrorCode(pub i32); + +// SAFETY: layout matches ICU's UErrorCode, a plain C enum over int, which is +// trivially copyable and trivially destructible. +unsafe impl cxx::ExternType for UErrorCode { + type Id = cxx::type_id!("UErrorCode"); + type Kind = cxx::kind::Trivial; +} + +impl UErrorCode { + /// ICU treats positive codes as failures and negative codes as warnings, + /// matching the `U_FAILURE` macro. + fn is_failure(self) -> bool { + self.0 > 0 + } +} + +/// Returns the ICU converter name for a transcodable encoding, matching +/// `getEncodingName()` in `i18n.c++`. +/// +/// The bridge `Encoding` enum is a `cxx` shared enum, which is a `u8` newtype +/// rather than a real Rust enum, so a value outside the four declared variants +/// is representable. It can only arise if the C++ and Rust halves of the +/// bridge disagree, and is reported as an error rather than a panic because a +/// panic crossing the bridge aborts the process. +fn icu_name(encoding: ffi::Encoding) -> Result<&'static CStr, TranscodeError> { + match encoding { + ffi::Encoding::Ascii => Ok(c"us-ascii"), + ffi::Encoding::Latin1 => Ok(c"iso8859-1"), + ffi::Encoding::Utf16Le => Ok(c"utf16le"), + ffi::Encoding::Utf8 => Ok(c"utf-8"), + _ => Err(TranscodeError::InvalidEncoding), + } +} + +/// An open ICU converter for one of the four transcodable encodings. +/// +/// Owns its `UConverter` and closes it on drop, including while unwinding. +/// Holding a raw pointer makes the type neither `Send` nor `Sync`, which is +/// what we want: ICU converters carry conversion state and are not safe to +/// share between threads. +pub struct Converter(*mut ffi::UConverter); + +impl Converter { + /// Opens an ICU converter for `encoding`. + pub fn open(encoding: ffi::Encoding) -> Result { + let mut err = UErrorCode::default(); + // SAFETY: `icu_name` returns a NUL-terminated static string, and `err` + // is a live local for the duration of the call. + let cnv = unsafe { ffi::ucnv_open(icu_name(encoding)?.as_ptr(), &raw mut err) }; + if err.is_failure() || cnv.is_null() { + return Err(TranscodeError::ConverterOpenFailed); + } + Ok(Self(cnv)) + } + + /// Returns the largest number of bytes a single character occupies in this + /// converter's encoding. + pub fn max_char_size(&self) -> usize { + // SAFETY: `self.0` is non-null for as long as `self` is alive. + let size = unsafe { ffi::ucnv_getMaxCharSize(self.0) }; + // ICU returns a positive byte count; the cast cannot lose information. + size.unsigned_abs().into() + } + + /// Returns the smallest number of bytes a single character occupies in this + /// converter's encoding. + pub fn min_char_size(&self) -> usize { + // SAFETY: `self.0` is non-null for as long as `self` is alive. + let size = unsafe { ffi::ucnv_getMinCharSize(self.0) }; + size.unsigned_abs().into() + } + + /// Sets the converter's substitute character sequence, used in place of + /// unmappable characters during conversion. + /// + /// Without this ICU substitutes its own default, which for ASCII is + /// U+001A rather than the `?` the C++ path produces. + pub fn set_subst_chars(&self, substitute: &str) -> Result<(), TranscodeError> { + if substitute.is_empty() { + return Ok(()); + } + // ICU takes the length as an `int8_t` and reads a negative length as + // "NUL-terminated", which `substitute` is not. Its own limit on + // substitute sequences is far lower still. + let length = + i8::try_from(substitute.len()).map_err(|_| TranscodeError::SetSubstituteCharsFailed)?; + + let mut err = UErrorCode::default(); + // SAFETY: `self.0` is non-null, and `substitute` outlives the call and + // is at least `length` bytes long. ICU takes the sequence as bytes and + // does not require NUL termination when given an explicit length. + unsafe { + ffi::ucnv_setSubstChars(self.0, substitute.as_ptr().cast(), length, &raw mut err); + } + if err.is_failure() { + return Err(TranscodeError::SetSubstituteCharsFailed); + } + Ok(()) + } +} + +impl Drop for Converter { + fn drop(&mut self) { + // SAFETY: `self.0` was returned non-null by `ucnv_open` and is closed + // exactly once, here. + unsafe { ffi::ucnv_close(self.0) } + } +} + +/// Converts `source` from `from`'s encoding to `to`'s encoding via ICU's +/// `ucnv_convertEx`, mirroring `TranscodeDefault` in `i18n.c++`. Returns the +/// number of bytes written to `target`, or `None` if ICU reports failure. +pub fn convert_ex( + to: &Converter, + from: &Converter, + source: &[u8], + target: &mut [u8], +) -> Option { + let target_start: *mut i8 = target.as_mut_ptr().cast(); + let mut target_cursor = target_start; + let mut source_cursor: *const i8 = source.as_ptr().cast(); + let mut err = UErrorCode::default(); + + // SAFETY: both cursors start at the base of a live slice and are bounded + // by a limit one past that slice's end, which is what ICU advances them + // against. Passing a null pivot asks ICU to use an internal one. + unsafe { + ffi::ucnv_convertEx( + to.0, + from.0, + &raw mut target_cursor, + target_start.add(target.len()), + &raw mut source_cursor, + source_cursor.add(source.len()), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null(), + 1, // reset + 1, // flush + &raw mut err, + ); + } + if err.is_failure() { + return None; + } + // SAFETY: ICU advanced `target_cursor` within `target`, so both pointers + // are into the same allocation. + let written = unsafe { target_cursor.offset_from(target_start) }; + usize::try_from(written).ok() +} + +/// Converts UTF-16LE `source` (as raw bytes) to `to`'s encoding via ICU's +/// `ucnv_fromUChars`, mirroring `TranscodeFromUTF16` in `i18n.c++`. Returns +/// the number of bytes written to `target`, or `None` if ICU reports failure. +pub fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> Option { + let src_length = i32::try_from(source.len() / size_of::()).ok()?; + let dest_capacity = i32::try_from(target.len()).ok()?; + let mut err = UErrorCode::default(); + + // SAFETY: the pointers and lengths describe the two live slices. `source` + // need not be `u16`-aligned -- it is caller-supplied buffer contents, which + // a Uint8Array can expose at an odd byteOffset -- and is reinterpreted as + // `UChar*` exactly as `i18n.c++` does with the same bytes. Casting a raw + // pointer is well-defined in Rust regardless of alignment; no reference to + // the misaligned data is ever formed on this side. + let len = unsafe { + ffi::ucnv_fromUChars( + to.0, + target.as_mut_ptr().cast(), + dest_capacity, + source.as_ptr().cast(), + src_length, + &raw mut err, + ) + }; + if err.is_failure() { + return None; + } + usize::try_from(len).ok() +} + +/// Widens Latin-1 `source` into UTF-16 (written to `target` as raw bytes), +/// mirroring `simdutf::convert_latin1_to_utf16`. Returns the number of +/// `char16_t` units written. +pub fn convert_latin1_to_utf16(source: &[u8], target: &mut [u8]) -> usize { + // SAFETY: `target` holds at least `source.len()` UTF-16 units, which + // `dispatch` guarantees by sizing it at two bytes per source byte. See + // `from_uchars` on the alignment of these casts. + unsafe { + ffi::convert_latin1_to_utf16( + source.as_ptr().cast(), + source.len(), + target.as_mut_ptr().cast(), + ) + } +} + +/// Estimates the UTF-16 length (in `char16_t` units) of UTF-8 `source`, +/// mirroring `simdutf::utf16_length_from_utf8`. +pub fn utf16_length_from_utf8(source: &[u8]) -> usize { + // SAFETY: pointer and length describe a live slice. + unsafe { ffi::utf16_length_from_utf8(source.as_ptr().cast(), source.len()) } +} + +/// Converts UTF-8 `source` to UTF-16LE (written to `target` as raw bytes), +/// mirroring `simdutf::convert_utf8_to_utf16le`. Returns the number of +/// `char16_t` units written, or `0` on invalid UTF-8. +pub fn convert_utf8_to_utf16le(source: &[u8], target: &mut [u8]) -> usize { + // SAFETY: `target` is sized by `dispatch` at two bytes per unit that + // `utf16_length_from_utf8` reported for this same `source`, which is the + // most this call can write. + unsafe { + ffi::convert_utf8_to_utf16le( + source.as_ptr().cast(), + source.len(), + target.as_mut_ptr().cast(), + ) + } +} + +/// Estimates the UTF-8 length (in bytes) of UTF-16LE `source` (as raw bytes), +/// mirroring `simdutf::utf8_length_from_utf16le`. +pub fn utf8_length_from_utf16le(source: &[u8]) -> usize { + // SAFETY: pointer and length describe a live slice. + unsafe { + ffi::utf8_length_from_utf16le(source.as_ptr().cast(), source.len() / size_of::()) + } +} + +/// Converts UTF-16LE `source` (as raw bytes) to UTF-8, mirroring +/// `simdutf::convert_utf16le_to_utf8`. Returns the number of bytes written. +pub fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize { + // SAFETY: `target` is sized by `dispatch` at the byte count + // `utf8_length_from_utf16le` reported for this same `source`, which is the + // most this call can write. + unsafe { + ffi::convert_utf16le_to_utf8( + source.as_ptr().cast(), + source.len() / size_of::(), + target.as_mut_ptr().cast(), + ) + } +} diff --git a/src/rust/i18n/dispatch.rs b/src/rust/i18n/dispatch.rs index 59bd9ef99d3..6298150ee55 100644 --- a/src/rust/i18n/dispatch.rs +++ b/src/rust/i18n/dispatch.rs @@ -15,13 +15,13 @@ //! destination is sized for the worst case. //! //! All sizing, validation, substitute-character setup, and length checking -//! happens here; [`crate::shim`] only forwards to the underlying ICU and +//! happens here; [`crate::codecs`] only forwards to the underlying ICU and //! simdutf calls. +use crate::codecs; +use crate::codecs::Converter; use crate::error::TranscodeError; use crate::ffi::Encoding; -use crate::shim; -use crate::shim::Converter; /// An isolate has a 128MB memory limit, and thus so does any single /// destination buffer. Mirrors `ISOLATE_LIMIT` in `i18n.c++`. @@ -103,10 +103,10 @@ impl<'a> Transcoder<'a> { let source = self.source; match &self.conversion { Conversion::ConvertEx { to, from } => { - shim::convert_ex(to, from, source, dest).ok_or(TranscodeError::UnableToTranscode) + codecs::convert_ex(to, from, source, dest).ok_or(TranscodeError::UnableToTranscode) } Conversion::Latin1ToUtf16 => { - let units = shim::convert_latin1_to_utf16(source, dest); + let units = codecs::convert_latin1_to_utf16(source, dest); // simdutf returns 0 for invalid input. if units == 0 { return Err(TranscodeError::UnableToTranscode); @@ -116,12 +116,12 @@ impl<'a> Transcoder<'a> { Ok(units * 2) } Conversion::FromUtf16 { to } => { - shim::from_uchars(to, source, dest).ok_or(TranscodeError::UnableToTranscode) + codecs::from_uchars(to, source, dest).ok_or(TranscodeError::UnableToTranscode) } Conversion::Utf16FromUtf8 => { // `dest` was sized as two bytes per estimated code unit. let expected_units = dest.len() / 2; - let units = shim::convert_utf8_to_utf16le(source, dest); + let units = codecs::convert_utf8_to_utf16le(source, dest); // simdutf returns 0 for invalid UTF-8 input. if units == 0 { return Err(TranscodeError::UnableToTranscode); @@ -133,7 +133,7 @@ impl<'a> Transcoder<'a> { } Conversion::Utf8FromUtf16 => { let expected_bytes = dest.len(); - let written = shim::convert_utf16le_to_utf8(source, dest); + let written = codecs::convert_utf16le_to_utf8(source, dest); // simdutf returns 0 for invalid input, which fails this check // because `expected_bytes` is nonzero here. The C++ // `TranscodeUTF8FromUTF16` checks for 0 only *after* requiring @@ -226,7 +226,7 @@ impl<'a> Transcoder<'a> { /// but UTF-8 continuation bytes, for instance -- which yields an empty /// result rather than an error. fn utf16_from_utf8(source: &'a [u8]) -> Result { - let expected_units = shim::utf16_length_from_utf8(source); + let expected_units = codecs::utf16_length_from_utf8(source); if expected_units > ISOLATE_LIMIT { return Err(TranscodeError::ExpectedUtf16LengthTooLarge); } @@ -248,7 +248,7 @@ impl<'a> Transcoder<'a> { return Err(TranscodeError::OddUtf16leInput); } - let dest_len = shim::utf8_length_from_utf16le(source); + let dest_len = codecs::utf8_length_from_utf16le(source); if dest_len > ISOLATE_LIMIT { return Err(TranscodeError::ExpectedUtf8LengthTooLarge); } diff --git a/src/rust/i18n/lib.rs b/src/rust/i18n/lib.rs index 6d8043c6d2e..e0644fda262 100644 --- a/src/rust/i18n/lib.rs +++ b/src/rust/i18n/lib.rs @@ -7,16 +7,25 @@ //! `transcode()`. Selected at runtime by the `NODEJS_I18N_RUST` autogate; when //! the gate is off, the C++ implementation is used instead. The two paths are //! byte-for-byte and error-message identical by construction: [`dispatch`] -//! ports the C++ dispatch/sizing/truncation logic to Rust, while [`shim`] -//! calls the exact same ICU and simdutf primitives the C++ path uses, through -//! the C++ shim in `shim.h` / `shim.c++`. +//! ports the C++ dispatch/sizing/truncation logic to Rust, while [`codecs`] +//! calls the exact same ICU and simdutf primitives the C++ path uses. +//! +//! Those primitives are bound directly by the [`ffi`] bridge, with no C++ of +//! their own in between. Matching the C++ path's behaviour is a matter of +//! calling the same codecs the same way, not of sharing code with it. + +// `ffi::ucnv_convertEx` takes thirteen parameters. The signature is ICU's, and +// a binding that did not mirror it exactly would not be a binding. The allow +// sits at crate level because `cxx::bridge` rejects lint attributes both on the +// bridge module and on the extern blocks inside it. +#![allow(clippy::too_many_arguments)] use jsg::Lock; use jsg::v8; +mod codecs; mod dispatch; mod error; -mod shim; use crate::dispatch::Transcoder; use crate::error::TranscodeError; @@ -38,24 +47,87 @@ mod ffi { Utf16Le, } + // ICU + // + // The same `ucnv_*` entry points `i18n.c++` calls. ICU renames every public + // symbol with its major version (`ucnv_open` -> `ucnv_open_78`) via + // `urename.h`; the generated bridge source is an ordinary translation unit + // that includes ``, so the rename applies there and no + // version appears in Rust. + // + // `UChar` is `char16_t`, so UTF-16 buffers use `c_char16` rather than + // `u16`: `uint16_t` would not match these declarations. + #[namespace = ""] unsafe extern "C++" { - include!("workerd/rust/i18n/shim.h"); + include!("unicode/ucnv.h"); + + type UConverter; + type UErrorCode = crate::codecs::UErrorCode; - type Converter; + unsafe fn ucnv_open(name: *const c_char, err: *mut UErrorCode) -> *mut UConverter; + unsafe fn ucnv_close(cnv: *mut UConverter); + unsafe fn ucnv_getMaxCharSize(cnv: *const UConverter) -> i8; + unsafe fn ucnv_getMinCharSize(cnv: *const UConverter) -> i8; + unsafe fn ucnv_setSubstChars( + cnv: *mut UConverter, + s: *const c_char, + length: i8, + err: *mut UErrorCode, + ); - fn open_converter(name: &str) -> UniquePtr; - fn max_char_size(self: &Converter) -> usize; - fn min_char_size(self: &Converter) -> usize; - fn set_subst_chars(self: &Converter, substitute: &str) -> bool; + unsafe fn ucnv_convertEx( + target_cnv: *mut UConverter, + source_cnv: *mut UConverter, + target: *mut *mut c_char, + target_limit: *const c_char, + source: *mut *const c_char, + source_limit: *const c_char, + pivot_start: *mut c_char16, + pivot_source: *mut *mut c_char16, + pivot_target: *mut *mut c_char16, + pivot_limit: *const c_char16, + reset: i8, + flush: i8, + err: *mut UErrorCode, + ); - fn convert_ex(to: &Converter, from: &Converter, source: &[u8], target: &mut [u8]) -> i64; - fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> i64; + unsafe fn ucnv_fromUChars( + cnv: *mut UConverter, + dest: *mut c_char, + dest_capacity: i32, + src: *const c_char16, + src_length: i32, + err: *mut UErrorCode, + ) -> i32; + } + + // simdutf + // + // Each of these names is an overload set: a raw-pointer overload plus + // `std::span` and constrained-template overloads. cxx binds a C++ function + // by initializing an exactly-typed function pointer with its address, which + // picks the raw-pointer overload by exact match. + #[namespace = "simdutf"] + unsafe extern "C++" { + include!("simdutf.h"); - fn convert_latin1_to_utf16(source: &[u8], target: &mut [u8]) -> usize; - fn utf16_length_from_utf8(source: &[u8]) -> usize; - fn convert_utf8_to_utf16le(source: &[u8], target: &mut [u8]) -> usize; - fn utf8_length_from_utf16le(source: &[u8]) -> usize; - fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize; + unsafe fn convert_latin1_to_utf16( + input: *const c_char, + length: usize, + utf16_output: *mut c_char16, + ) -> usize; + unsafe fn utf16_length_from_utf8(input: *const c_char, length: usize) -> usize; + unsafe fn convert_utf8_to_utf16le( + input: *const c_char, + length: usize, + utf16_output: *mut c_char16, + ) -> usize; + unsafe fn utf8_length_from_utf16le(input: *const c_char16, length: usize) -> usize; + unsafe fn convert_utf16le_to_utf8( + input: *const c_char16, + length: usize, + utf8_output: *mut c_char, + ) -> usize; } #[namespace = "workerd::rust::jsg"] diff --git a/src/rust/i18n/shim.c++ b/src/rust/i18n/shim.c++ deleted file mode 100644 index 6d15b371970..00000000000 --- a/src/rust/i18n/shim.c++ +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright (c) 2026 Cloudflare, Inc. -// Licensed under the Apache 2.0 license found in the LICENSE file or at: -// https://opensource.org/licenses/Apache-2.0 - -#include "shim.h" - -#include "simdutf.h" - -#include - -#include - -namespace workerd::rust::i18n { - -Converter::~Converter() noexcept { - if (conv_ != nullptr) { - ucnv_close(conv_); - } -} - -size_t Converter::max_char_size() const { - return static_cast(ucnv_getMaxCharSize(conv_)); -} - -size_t Converter::min_char_size() const { - return static_cast(ucnv_getMinCharSize(conv_)); -} - -bool Converter::set_subst_chars(::rust::Str substitute) const { - if (substitute.empty()) return true; - // `ucnv_setSubstChars` takes the length as an `int8_t`, and reads a negative - // length as "NUL-terminated", which `rust::Str` is not. Reject anything that - // would not survive the narrowing; ICU's own limit is far lower still. - if (substitute.size() > INT8_MAX) return false; - UErrorCode status = U_ZERO_ERROR; - ucnv_setSubstChars(conv_, substitute.data(), static_cast(substitute.size()), &status); - return U_SUCCESS(status); -} - -std::unique_ptr open_converter(::rust::Str name) { - UErrorCode status = U_ZERO_ERROR; - // `ucnv_open` needs a NUL-terminated name, which `rust::Str` is not. - auto nameStr = kj::str(kj::ArrayPtr(name.data(), name.size())); - auto* conv = ucnv_open(nameStr.cStr(), &status); - if (U_FAILURE(status)) return nullptr; - return std::make_unique(conv); -} - -int64_t convert_ex(const Converter& to, - const Converter& from, - ::rust::Slice source, - ::rust::Slice target) { - char* const targetStart = reinterpret_cast(target.data()); - char* targetPtr = targetStart; - const char* sourcePtr = reinterpret_cast(source.data()); - UErrorCode status = U_ZERO_ERROR; - ucnv_convertEx(to.conv_, from.conv_, &targetPtr, targetStart + target.size(), &sourcePtr, - sourcePtr + source.size(), nullptr, nullptr, nullptr, nullptr, true, true, &status); - if (U_FAILURE(status)) return -1; - return static_cast(targetPtr - targetStart); -} - -int64_t from_uchars( - const Converter& to, ::rust::Slice source, ::rust::Slice target) { - UErrorCode status = U_ZERO_ERROR; - auto len = ucnv_fromUChars(to.conv_, reinterpret_cast(target.data()), - static_cast(target.size()), reinterpret_cast(source.data()), - static_cast(source.size() / sizeof(UChar)), &status); - if (U_FAILURE(status)) return -1; - return static_cast(len); -} - -size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target) { - return simdutf::convert_latin1_to_utf16(reinterpret_cast(source.data()), - source.size(), reinterpret_cast(target.data())); -} - -size_t utf16_length_from_utf8(::rust::Slice source) { - return simdutf::utf16_length_from_utf8( - reinterpret_cast(source.data()), source.size()); -} - -size_t convert_utf8_to_utf16le(::rust::Slice source, ::rust::Slice target) { - return simdutf::convert_utf8_to_utf16le(reinterpret_cast(source.data()), - source.size(), reinterpret_cast(target.data())); -} - -size_t utf8_length_from_utf16le(::rust::Slice source) { - return simdutf::utf8_length_from_utf16le( - reinterpret_cast(source.data()), source.size() / sizeof(char16_t)); -} - -size_t convert_utf16le_to_utf8(::rust::Slice source, ::rust::Slice target) { - return simdutf::convert_utf16le_to_utf8(reinterpret_cast(source.data()), - source.size() / sizeof(char16_t), reinterpret_cast(target.data())); -} - -} // namespace workerd::rust::i18n diff --git a/src/rust/i18n/shim.h b/src/rust/i18n/shim.h deleted file mode 100644 index eb8f29cfae9..00000000000 --- a/src/rust/i18n/shim.h +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright (c) 2026 Cloudflare, Inc. -// Licensed under the Apache 2.0 license found in the LICENSE file or at: -// https://opensource.org/licenses/Apache-2.0 -#pragma once - -// C++ shim exposing the ICU `ucnv_*` primitives and the simdutf conversion -// functions used by `workerd::api::node::i18n::transcode` -// (`src/workerd/api/node/i18n.c++`), so the Rust implementation in `lib.rs` -// calls the exact same codecs as the C++ path rather than reimplementing -// them. - -#include - -#include -#include -#include - -// ICU's converter handle (``). Kept opaque here; the full ICU -// header is only needed in shim.c++. -struct UConverter; - -namespace workerd::rust::i18n { - -// RAII wrapper around an ICU `UConverter*`, opened by `open_converter()` for -// one of the four transcodable encodings. Exposed to Rust as an opaque C++ -// type behind `UniquePtr`, so `ucnv_close()` runs in the -// destructor and the converter's lifetime is owned entirely by C++: even if -// Rust code holding the `UniquePtr` panics, unwinding still drops it and runs -// the destructor, unlike a raw `UConverter*` smuggled across the FFI boundary, -// which a panic could leak. -class Converter { - public: - explicit Converter(UConverter* conv) noexcept: conv_(conv) {} - ~Converter() noexcept; - Converter(const Converter&) = delete; - Converter& operator=(const Converter&) = delete; - - size_t max_char_size() const; - size_t min_char_size() const; - - // Sets the byte sequence ICU substitutes for characters that cannot be - // represented in this converter's encoding. Returns false if `substitute` is - // too long, or if ICU rejects it for this encoding. - bool set_subst_chars(::rust::Str substitute) const; - - private: - UConverter* conv_; - - friend int64_t convert_ex(const Converter& to, - const Converter& from, - ::rust::Slice source, - ::rust::Slice target); - friend int64_t from_uchars( - const Converter& to, ::rust::Slice source, ::rust::Slice target); -}; - -// Opens an ICU converter for `name` (an ICU encoding name, e.g. "us-ascii"), -// or returns null if ICU does not recognize `name`. -std::unique_ptr open_converter(::rust::Str name); - -// `ucnv_convertEx()`-based conversion between two ICU converters, mirroring -// `TranscodeDefault` in `i18n.c++`. `source` and `target` are raw bytes. -// Returns the number of bytes written to `target`, or -1 if ICU reports -// failure. -int64_t convert_ex(const Converter& to, - const Converter& from, - ::rust::Slice source, - ::rust::Slice target); - -// `ucnv_fromUChars()`-based conversion from UTF-16LE, mirroring -// `TranscodeFromUTF16` in `i18n.c++`. `source` holds UTF-16LE code units as -// raw bytes (its length must be even); `target` is raw output bytes. Returns -// the number of bytes written to `target`, or -1 if ICU reports failure. -int64_t from_uchars( - const Converter& to, ::rust::Slice source, ::rust::Slice target); - -// simdutf wrappers, mirroring the four `simdutf::*` calls in `i18n.c++`. -// Buffers holding UTF-16LE code units are passed as raw bytes and cast to -// `char16_t*` internally, exactly as the C++ path does via -// `JsUint8Array::asArrayPtr()`. -// -// Neither buffer is required to be `char16_t`-aligned. `target` always is in -// practice, being a V8 backing store, but `source` is caller-supplied buffer -// contents that a `Uint8Array` can expose at an odd `byteOffset`. simdutf -// reads and writes through unaligned SIMD loads and stores, so this is -// well-defined for the simdutf entry points below; `from_uchars` casts to -// `UChar*` for ICU on the same basis as the C++ path. - -size_t convert_latin1_to_utf16(::rust::Slice source, ::rust::Slice target); -size_t utf16_length_from_utf8(::rust::Slice source); -size_t convert_utf8_to_utf16le(::rust::Slice source, ::rust::Slice target); -size_t utf8_length_from_utf16le(::rust::Slice source); -size_t convert_utf16le_to_utf8(::rust::Slice source, ::rust::Slice target); - -} // namespace workerd::rust::i18n diff --git a/src/rust/i18n/shim.rs b/src/rust/i18n/shim.rs deleted file mode 100644 index 8c6291082fe..00000000000 --- a/src/rust/i18n/shim.rs +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright (c) 2026 Cloudflare, Inc. -// Licensed under the Apache 2.0 license found in the LICENSE file or at: -// https://opensource.org/licenses/Apache-2.0 - -//! Thin Rust wrappers around the ICU/simdutf primitives exposed by the C++ -//! shim (`shim.h` / `shim.c++`). All the transcoding *logic* -- dispatch, -//! sizing, substitute-character setup, truncation -- lives in [`crate::dispatch`]; -//! this module only adapts the shim's C-ish sentinel-value return conventions -//! (`-1` for ICU failure, `0` for simdutf failure) into idiomatic `Option`s. - -use crate::error::TranscodeError; -use crate::ffi; - -/// An open ICU converter for one of the four transcodable encodings. -/// -/// Wraps a `cxx::UniquePtr`: the underlying `UConverter*` and -/// its `ucnv_close()` teardown are owned entirely by the C++ shim, so the -/// converter is torn down correctly even if Rust code holding it panics -- -/// unlike a raw `UConverter*` smuggled across the FFI boundary, which a panic -/// could leak. -pub struct Converter(cxx::UniquePtr); - -/// Returns the ICU converter name for a transcodable encoding, matching -/// `getEncodingName()` in `i18n.c++`. -/// -/// The bridge `Encoding` enum is a `cxx` shared enum, which is a `u8` newtype -/// rather than a real Rust enum, so a value outside the four declared variants -/// is representable. It can only arise if the C++ and Rust halves of the -/// bridge disagree, and is reported as an error rather than a panic because a -/// panic crossing the bridge aborts the process. -fn icu_name(encoding: ffi::Encoding) -> Result<&'static str, TranscodeError> { - match encoding { - ffi::Encoding::Ascii => Ok("us-ascii"), - ffi::Encoding::Latin1 => Ok("iso8859-1"), - ffi::Encoding::Utf16Le => Ok("utf16le"), - ffi::Encoding::Utf8 => Ok("utf-8"), - _ => Err(TranscodeError::InvalidEncoding), - } -} - -impl Converter { - /// Opens an ICU converter for `encoding`. - pub fn open(encoding: ffi::Encoding) -> Result { - let conv = ffi::open_converter(icu_name(encoding)?); - if conv.is_null() { - return Err(TranscodeError::ConverterOpenFailed); - } - Ok(Self(conv)) - } - - /// Returns the largest number of bytes a single character occupies in this - /// converter's encoding. - pub fn max_char_size(&self) -> usize { - self.0.max_char_size() - } - - /// Returns the smallest number of bytes a single character occupies in this - /// converter's encoding. - pub fn min_char_size(&self) -> usize { - self.0.min_char_size() - } - - /// Sets the converter's substitute character sequence, used in place of - /// unmappable characters during conversion. - pub fn set_subst_chars(&self, substitute: &str) -> Result<(), TranscodeError> { - if self.0.set_subst_chars(substitute) { - Ok(()) - } else { - Err(TranscodeError::SetSubstituteCharsFailed) - } - } -} - -/// Converts `source` from `from`'s encoding to `to`'s encoding via ICU's -/// `ucnv_convertEx`, mirroring `TranscodeDefault` in `i18n.c++`. Returns the -/// number of bytes written to `target`, or `None` if ICU reports failure. -pub fn convert_ex( - to: &Converter, - from: &Converter, - source: &[u8], - target: &mut [u8], -) -> Option { - usize::try_from(ffi::convert_ex(&to.0, &from.0, source, target)).ok() -} - -/// Converts UTF-16LE `source` (as raw bytes) to `to`'s encoding via ICU's -/// `ucnv_fromUChars`, mirroring `TranscodeFromUTF16` in `i18n.c++`. Returns -/// the number of bytes written to `target`, or `None` if ICU reports failure. -pub fn from_uchars(to: &Converter, source: &[u8], target: &mut [u8]) -> Option { - usize::try_from(ffi::from_uchars(&to.0, source, target)).ok() -} - -/// Widens Latin-1 `source` into UTF-16 (written to `target` as raw bytes), -/// mirroring `simdutf::convert_latin1_to_utf16`. Returns the number of -/// `char16_t` units written. -pub fn convert_latin1_to_utf16(source: &[u8], target: &mut [u8]) -> usize { - ffi::convert_latin1_to_utf16(source, target) -} - -/// Estimates the UTF-16 length (in `char16_t` units) of UTF-8 `source`, -/// mirroring `simdutf::utf16_length_from_utf8`. -pub fn utf16_length_from_utf8(source: &[u8]) -> usize { - ffi::utf16_length_from_utf8(source) -} - -/// Converts UTF-8 `source` to UTF-16LE (written to `target` as raw bytes), -/// mirroring `simdutf::convert_utf8_to_utf16le`. Returns the number of -/// `char16_t` units written, or `0` on invalid UTF-8. -pub fn convert_utf8_to_utf16le(source: &[u8], target: &mut [u8]) -> usize { - ffi::convert_utf8_to_utf16le(source, target) -} - -/// Estimates the UTF-8 length (in bytes) of UTF-16LE `source` (as raw bytes), -/// mirroring `simdutf::utf8_length_from_utf16le`. -pub fn utf8_length_from_utf16le(source: &[u8]) -> usize { - ffi::utf8_length_from_utf16le(source) -} - -/// Converts UTF-16LE `source` (as raw bytes) to UTF-8, mirroring -/// `simdutf::convert_utf16le_to_utf8`. Returns the number of bytes written. -pub fn convert_utf16le_to_utf8(source: &[u8], target: &mut [u8]) -> usize { - ffi::convert_utf16le_to_utf8(source, target) -} From f74fbfa4a3fe66509370cfdf4e3fcd003a16be89 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Fri, 21 Aug 2026 12:44:57 -0500 Subject: [PATCH 08/10] rust/i18n: use c_char instead of hardcoded i8 for ucnv_convertEx pointers c_char is unsigned on aarch64, so the explicit i8 annotations caused a type mismatch against the cxx-bridge declaration when building for ARM. --- src/rust/i18n/codecs.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rust/i18n/codecs.rs b/src/rust/i18n/codecs.rs index bb3bb78c61c..ea588e5b02a 100644 --- a/src/rust/i18n/codecs.rs +++ b/src/rust/i18n/codecs.rs @@ -12,6 +12,7 @@ //! substitute-character setup, truncation -- lives in [`crate::dispatch`]. use std::ffi::CStr; +use std::ffi::c_char; use crate::error::TranscodeError; use crate::ffi; @@ -142,9 +143,9 @@ pub fn convert_ex( source: &[u8], target: &mut [u8], ) -> Option { - let target_start: *mut i8 = target.as_mut_ptr().cast(); + let target_start: *mut c_char = target.as_mut_ptr().cast(); let mut target_cursor = target_start; - let mut source_cursor: *const i8 = source.as_ptr().cast(); + let mut source_cursor: *const c_char = source.as_ptr().cast(); let mut err = UErrorCode::default(); // SAFETY: both cursors start at the base of a live slice and are bounded From ea2c984e80ac2e73ba4c2dc6c033e296aaa02125 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Fri, 21 Aug 2026 14:00:26 -0500 Subject: [PATCH 09/10] edit agent md --- src/rust/AGENTS.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rust/AGENTS.md b/src/rust/AGENTS.md index 225ef4732a8..f80bd19bb63 100644 --- a/src/rust/AGENTS.md +++ b/src/rust/AGENTS.md @@ -117,8 +117,6 @@ If a C++ library already depends on your crate (C++ → Rust) and you also add a cxx also supports **reusing a binding type across bridges** ([docs](https://cxx.rs/extern-c++.html#reusing-existing-binding-types)): the `worker` crate's `error.rs` / `ok.rs` / `kill_switch.rs` bridges reuse `ffi.rs`'s types by depending on `:ffi.rs@cxx`. Still, keep a struct that only crosses FFI within one crate in that crate's bridge. -**UTF-16 crosses the FFI as `c_char16`, not `u16`.** `u16` in a bridge means C++ `uint16_t`; `cxx::c_char16` means `char16_t`. The two are layout-identical but distinct C++ types, so only `c_char16` resolves to a `char16_t` overload or matches a `char16_t*` parameter. On the Rust side `c_char16` is an alias for `u16`, so `&[c_char16]` accepts a `&[u16]` from `str::encode_utf16` or `encoding_rs` with no cast. See the doc comment on `cxx::c_char16` for the container limits the alias implies. - **V8 handles must always cross the FFI as the shared `jsg::v8::ffi` types, never as a bare `usize`.** When another crate's bridge passes a V8 `Local`/`Global`, reuse the jsg shared struct via a type alias (`type Local = jsg::v8::ffi::Local;`) plus `include!("workerd/rust/jsg/v8.rs.h")`, and depend on `//src/rust/jsg` (which supplies the generated header transitively). Do not smuggle the handle word through a `usize` — the shared type keeps both sides in one canonical, cxx-verified definition. See `node-exceptions/lib.rs`. ### Testing crates that cross the FFI From 41f44a51cd2b3f39ccede32bc0360e350173f702 Mon Sep 17 00:00:00 2001 From: Logan Gatlin Date: Fri, 21 Aug 2026 14:03:11 -0500 Subject: [PATCH 10/10] fix comment --- src/rust/i18n/BUILD.bazel | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/rust/i18n/BUILD.bazel b/src/rust/i18n/BUILD.bazel index 4e91e81e581..a39f0b951ac 100644 --- a/src/rust/i18n/BUILD.bazel +++ b/src/rust/i18n/BUILD.bazel @@ -1,16 +1,12 @@ load("//:build/wd_rust_crate.bzl", "wd_rust_crate") -# The bridge in `lib.rs` binds ICU's `ucnv_*` and simdutf's conversion -# functions directly, so the crate needs their headers to generate the bridge -# and their symbols at link time. ICU comes from `@workerd-v8//:v8`, the seam -# every other workerd target uses for it; depending on the ICU repository -# directly breaks builds that substitute their own V8/ICU for that module. wd_rust_crate( name = "i18n", cxx_bridge_deps = [ "//src/rust/jsg", "@simdutf", "@workerd-v8//:v8", + # Also depends on ICU for bindings, which are extracted from the v8 dep above ], cxx_bridge_src = "lib.rs", link_deps = [