fix: Early termination of strings with NULL values in-between for JavascriptCore

[swittk]

Summary

createStringFromUtf8 currently calls JavascriptCore's JSStringCreateWithUTF8CString(const char *) method when retrieving strings from the native side, causing strings with NULL values in-between to be early-truncated (since it stops reading with the first NULL value).

Fixes https://github.com/facebook/react-native/issues/24129

Changelog

Changed jsi::String JSCRuntime::createStringFromUtf8(const uint8_t *str, size_t length) to use a parser to UTF-16 characters as appropriate for the JSChar * constructor, JSStringCreateWithCharacters(const JSChar* chars, size_t numChars), which allows specifying of length.

This also should provide a minor speed-up for longer strings since JSStringCreateWithUTF8CString internally calls strlen(), then does UTF-8 to UTF-16 conversions.

[General] [Fixed] - Early termination of strings with NULL values in JavascriptCore (https://github.com/facebook/react-native/issues/24129)

Test Plan

I executed calls to my level adapter, saving and loading strings.

await db.put('hi', 'hi\u0000world');
const gotRes = await db.get('hi'); 
console.log(gotRes)

Without this fix it would result in only 'hi'. With this fix, you get the whole string, 'hi\u0000world'.

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,829,829	+0
android	hermes	armeabi-v7a	7,220,851	+0
android	hermes	x86	8,142,240	+0
android	hermes	x86_64	8,120,936	+0
android	jsc	arm64-v8a	9,711,193	+5,010
android	jsc	armeabi-v7a	8,465,008	+4,158
android	jsc	x86	9,662,359	+5,576
android	jsc	x86_64	10,262,046	+6,960

Base commit: 1af2beaaa34ac01f04b14035acdfe0a5ecb329d8 Branch: main

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 1af2beaaa34ac01f04b14035acdfe0a5ecb329d8 Branch: main

[github-actions[bot]]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[swittk]

not stale?

[javache]

Thanks for the fix. The approach makes sense, but I'd cautious this may cause performance regressions, given the existing fast path that exists in JSC for all-ASCII stings. I'd also prefer if we could defer a system implementation here, or have an implementation of these methods outside of this file that reference their original source (e.g. https://github.com/WebKit/WebKit/blob/main/Source/WTF/wtf/unicode/UTF8Conversion.cpp)

[swittk]

Hi. Thanks for reviewing! (and sorry for the late reply)

Just wondering; do you have any suggestions on how I might import that function (the one that converts UTF-8 to UTF-16 strings over from the WebKit/JSC side?) over? It's implemented in CPP and I'm not sure how I might define extern function declarations or whatnot to make it work.

As for potential performance regressions, maybe we just have to benchmark in the end? Because of I understand correctly, eventually what the existing code path does is it needs to convert the strings over to UTF-16 internally anyway when on JSC.

[github-actions[bot]]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[swittk]

... any potentials for this?

[react-native-bot]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[swittk]

Is this basically stale because JavascriptCore is not a priority and Hermes is the future? Or is this still a valid issue?