diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 5d326c29..ac140efa 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -362,24 +362,18 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Localtype == BinaryTypeEncodingType::CStringEncoding) { if (arg->IsString()) { - const char* value = nullptr; - Local strArg = arg.As(); - if (strArg->IsExternalOneByte()) { - const v8::String::ExternalOneByteStringResource* resource = - strArg->GetExternalOneByteStringResource(); - value = resource->data(); - } else { - v8::String::Utf8Value utf8Value(isolate, arg); - value = strdup(*utf8Value); - // The external string only ties the strdup'd buffer's lifetime to the - // GC; it is never read as a string, and its one-byte (Latin-1) content - // matches the UTF-8 buffer only for ASCII. Length is the buffer's byte - // count, excluding the NUL. - OneByteStringResource* resource = - new OneByteStringResource(value, (size_t)utf8Value.length()); - bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg); - tns::Assert(success, isolate); - } + // Always copy: an external one-byte string's resource (e.g. one made by + // node_api_create_external_string_latin1) holds Latin-1 with no NUL + // terminator, so its data can't be handed to native code as a C string. + v8::String::Utf8Value utf8Value(isolate, arg); + const char* value = strdup(*utf8Value); + // The external string only ties the strdup'd buffer's lifetime to the + // GC; it is never read as a string. Its length counts the NUL so it is + // never zero: V8 disposes a zero-length resource immediately. + OneByteStringResource* resource = + new OneByteStringResource(value, (size_t)utf8Value.length() + 1); + bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg); + tns::Assert(success, isolate); Interop::SetValue(dest, value); } else { BaseDataWrapper* wrapper = tns::GetValue(isolate, arg); diff --git a/TestFixtures/NapiCoverageModule.mm b/TestFixtures/NapiCoverageModule.mm index 45fd11ea..0b9b0be0 100644 --- a/TestFixtures/NapiCoverageModule.mm +++ b/TestFixtures/NapiCoverageModule.mm @@ -1,7 +1,7 @@ // Addons see NAPI_VERSION 8 unless they ask for more; asking must come before // the headers. The env still reports module API version 8 either way — this // only decides which declarations exist. -#define NAPI_VERSION 9 +#define NAPI_VERSION 10 #include #include @@ -146,6 +146,25 @@ static napi_value CreateStrings(napi_env env, napi_callback_info info) { return result; } +// The shape addons hand to node_api_create_external_string_latin1: bytes that +// are Latin-1 (0xE9 is 'é') and not NUL-terminated. +static napi_value CreateExternalLatin1(napi_env env, napi_callback_info info) { + (void)info; + + static char kBytes[] = {'c', 'a', 'f', (char)0xE9, '!'}; + + napi_value text = NULL; + bool copied = true; + NAPI_CALL(env, + node_api_create_external_string_latin1(env, kBytes, 4, NULL, NULL, &text, &copied)); + + napi_value result = NULL; + NAPI_CALL(env, napi_create_object(env, &result)); + NAPI_CALL(env, napi_set_named_property(env, result, "text", text)); + NAPI_CALL(env, napi_set_named_property(env, result, "copied", NapiBool(env, copied))); + return result; +} + static napi_value StringStatus(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; @@ -1344,6 +1363,7 @@ static napi_value InitNapiCoverageModule(napi_env env, napi_value exports) { NAPI_METHOD("stringLengths", StringLengths), NAPI_METHOD("copyString", CopyString), NAPI_METHOD("createStrings", CreateStrings), + NAPI_METHOD("createExternalLatin1", CreateExternalLatin1), NAPI_METHOD("stringStatus", StringStatus), NAPI_METHOD("numberParts", NumberParts), NAPI_METHOD("createNumbers", CreateNumbers), diff --git a/TestRunner/app/tests/Marshalling/ReferenceTests.js b/TestRunner/app/tests/Marshalling/ReferenceTests.js index c1810c3b..f468a99e 100644 --- a/TestRunner/app/tests/Marshalling/ReferenceTests.js +++ b/TestRunner/app/tests/Marshalling/ReferenceTests.js @@ -148,6 +148,13 @@ describe(module.id, function () { expect(TNSGetOutput()).toBe("abc"); }); + it("marshals an empty string as an empty C string", function () { + TNSClearOutput(); + var actual = functionWithCharPtr(""); + expect(TNSGetOutput()).toBe(""); + expect(actual[0]).toBe(0); + }); + it("functionWithInt8ArrayBuffer", function () { let array = Int8Array.from([ 97, 98, 99, 0 ]).buffer; let actual = functionWithCharPtr(array); diff --git a/TestRunner/app/tests/NapiCoverageTests.js b/TestRunner/app/tests/NapiCoverageTests.js index 05c77796..bbe4696c 100644 --- a/TestRunner/app/tests/NapiCoverageTests.js +++ b/TestRunner/app/tests/NapiCoverageTests.js @@ -46,6 +46,16 @@ describe("Node-API value surface", function () { expect(created.empty).toBe(""); }); + it("hands an external latin1 string to C as UTF-8", function () { + var external = napi.createExternalLatin1(); + expect(external.copied).toBe(false); + expect(external.text).toBe("café"); + + TNSClearOutput(); + functionWithCharPtr(external.text); + expect(TNSGetOutput()).toBe("café"); + }); + it("honours an explicit length when creating", function () { expect(napi.createStrings().utf8Sized).toBe("abc"); });