From 5e74926738a6a20fdb9747b0bd271ae3b2417e0b Mon Sep 17 00:00:00 2001 From: Yulong Wang Date: Thu, 28 Dec 2017 17:20:28 -0800 Subject: [PATCH] Fix string encoding in Napa (#153) The following components are supporting unicode string now. napa.store APIs FunctionSpec that used in zone.execute() Fix: #144 --- inc/napa/stl/string.h | 15 +++- inc/napa/v8-helpers/conversion.h | 14 ++++ inc/napa/v8-helpers/string.h | 77 ++++++++++++++++--- .../core-modules/napa/call-context-wrap.cpp | 3 +- src/module/core-modules/napa/store-wrap.cpp | 2 +- src/store/store.h | 2 +- test/napa-zone/test.ts | 15 ++++ test/store-test.ts | 58 ++++++++++---- test/transport-test.ts | 4 +- test/zone-test.ts | 38 +++++++++ 10 files changed, 196 insertions(+), 32 deletions(-) diff --git a/inc/napa/stl/string.h b/inc/napa/stl/string.h index ae2b98a..130a2d6 100644 --- a/inc/napa/stl/string.h +++ b/inc/napa/stl/string.h @@ -12,6 +12,7 @@ namespace napa { using BasicString = std::basic_string>; typedef BasicString String; + typedef BasicString U16String; } } @@ -22,13 +23,25 @@ namespace std { template<> struct hash : public __hash_base { size_t operator()(const napa::stl::String& s) const noexcept { - return std::_Hash_impl::hash(s.data(), s.length()); + return std::_Hash_impl::hash(s.data(), s.length() * sizeof(char)); } }; template<> struct __is_fast_hash> : std::false_type { }; + + // std::hash specialization for napa::stl::U16String. + template<> + struct hash : public __hash_base { + size_t operator()(const napa::stl::U16String& s) const noexcept { + return std::_Hash_impl::hash(s.data(), s.length() * sizeof(char16_t)); + } + }; + + template<> + struct __is_fast_hash> : std::false_type { + }; } #endif diff --git a/inc/napa/v8-helpers/conversion.h b/inc/napa/v8-helpers/conversion.h index cbfa7d3..5900de6 100644 --- a/inc/napa/v8-helpers/conversion.h +++ b/inc/napa/v8-helpers/conversion.h @@ -22,6 +22,13 @@ namespace v8_helpers { return std::string(*utf8Value); } + /// Convert a v8 value to std::u16string. + template <> + inline std::u16string V8ValueTo(const v8::Local& value) { + v8::String::Value utf16Value(value); + return std::u16string(reinterpret_cast(*utf16Value)); + } + /// Convert a v8 value to napa::stl::String. template <> inline napa::stl::String V8ValueTo(const v8::Local& value) { @@ -29,6 +36,13 @@ namespace v8_helpers { return napa::stl::String(*utf8Value); } + /// Convert a v8 value to napa::stl::U16String. + template <> + inline napa::stl::U16String V8ValueTo(const v8::Local& value) { + v8::String::Value utf16Value(value); + return napa::stl::U16String(reinterpret_cast(*utf16Value)); + } + /// Convert a v8 value to a Utf8String. template <> inline Utf8String V8ValueTo(const v8::Local& value) { diff --git a/inc/napa/v8-helpers/string.h b/inc/napa/v8-helpers/string.h index d34e3ba..0059d7e 100644 --- a/inc/napa/v8-helpers/string.h +++ b/inc/napa/v8-helpers/string.h @@ -20,35 +20,92 @@ namespace v8_helpers { return MakeV8String(isolate, str.c_str(), static_cast(str.length())); } - /// Make a V8 string from std::string. + /// Make a V8 string from napa::stl::String. inline v8::Local MakeV8String(v8::Isolate *isolate, const napa::stl::String& str) { return MakeV8String(isolate, str.c_str(), static_cast(str.length())); } + /// Make a V8 string by making a copy of const uint16_t*. + inline v8::Local MakeV8String(v8::Isolate *isolate, const uint16_t* str, int length = -1) { + return v8::String::NewFromTwoByte(isolate, str, v8::NewStringType::kNormal, length).ToLocalChecked(); + } + + /// Make a V8 string by making a copy of const char16_t*. + inline v8::Local MakeV8String(v8::Isolate *isolate, const char16_t* str, int length = -1) { + return MakeV8String(isolate, reinterpret_cast(str), length); + } + + /// Make a V8 string from std::u16string. + inline v8::Local MakeV8String(v8::Isolate *isolate, const std::u16string& str) { + return MakeV8String(isolate, str.c_str(), static_cast(str.length())); + } + + /// Make a V8 string from napa::stl::U16String. + inline v8::Local MakeV8String(v8::Isolate *isolate, const napa::stl::U16String& str) { + return MakeV8String(isolate, str.c_str(), static_cast(str.length())); + } + + using ExternalOneByteStringView = v8::ExternalOneByteStringResourceImpl; + class ExternalTwoByteStringView : public v8::String::ExternalStringResource { + public: + ExternalTwoByteStringView() : _data(nullptr), _length(0) {} + ExternalTwoByteStringView(const uint16_t* data, size_t length) : _data(data), _length(length) {} + const uint16_t* data() const { return _data; } + size_t length() const { return _length; } + + private: + const uint16_t* _data; + size_t _length; + }; + /// Make a V8 string from external const char*. - inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const char* data) { - // V8 garbage collection frees ExternalOneByteStringResourceImpl. - auto externalResource = new v8::ExternalOneByteStringResourceImpl(data, std::strlen(data)); + /// The input data should only contains Latin-1 chars. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const char* data, size_t length) { + // V8 garbage collection frees ExternalOneByteStringView. + auto externalResource = new ExternalOneByteStringView(data, length); return v8::String::NewExternalOneByte(isolate, externalResource).ToLocalChecked(); } /// Make a V8 string from external const char*. - inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const char* data, size_t length) { - // V8 garbage collection frees ExternalOneByteStringResourceImpl. - auto externalResource = new v8::ExternalOneByteStringResourceImpl(data, length); - return v8::String::NewExternalOneByte(isolate, externalResource).ToLocalChecked(); + /// The input data should only contains Latin-1 chars. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const char* data) { + return MakeExternalV8String(isolate, data, strlen(data)); } /// Make a V8 string from external std::string. + /// The input data should only contains Latin-1 chars. inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const std::string& str) { return MakeExternalV8String(isolate, str.data(), str.length()); } - /// Make a V8 string from external napa::stl::String. + /// Make a V8 string from external napa::stl::String. + /// The input data should only contains Latin-1 chars. inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const napa::stl::String& str) { return MakeExternalV8String(isolate, str.data(), str.length()); } + /// Make a V8 string from external const uint16_t*. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const uint16_t* data, size_t length) { + // V8 garbage collection frees ExternalTwoByteStringView. + auto externalResource = new ExternalTwoByteStringView(data, length); + return v8::String::NewExternalTwoByte(isolate, externalResource).ToLocalChecked(); + } + + /// Make a V8 string from external const char16_t*. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const char16_t* data, size_t length) { + return MakeExternalV8String(isolate, reinterpret_cast(data), length); + } + + /// Make a V8 string from external std::u16string. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const std::u16string& str) { + return MakeExternalV8String(isolate, str.data(), str.length()); + } + + /// Make a V8 string from external napa::stl::U16String. + inline v8::Local MakeExternalV8String(v8::Isolate *isolate, const napa::stl::U16String& str) { + return MakeExternalV8String(isolate, str.data(), str.length()); + } + /// Converts a V8 string object to a movable Utf8String which supports an allocator. template class Utf8StringWithAllocator { @@ -73,7 +130,7 @@ namespace v8_helpers { if (!val->ToString(context).ToLocal(&str)) { return; } - _length = str->Length(); + _length = str->Utf8Length(); _data = _alloc.allocate(_length + 1); str->WriteUtf8(_data); } diff --git a/src/module/core-modules/napa/call-context-wrap.cpp b/src/module/core-modules/napa/call-context-wrap.cpp index 08e6461..4c9155e 100644 --- a/src/module/core-modules/napa/call-context-wrap.cpp +++ b/src/module/core-modules/napa/call-context-wrap.cpp @@ -117,7 +117,8 @@ void CallContextWrap::GetArgumentsCallback(v8::Local /*propertyName* auto& cppArgs = thisObject->GetRef().GetArguments(); auto jsArgs = v8::Array::New(isolate, static_cast(cppArgs.size())); for (size_t i = 0; i < cppArgs.size(); ++i) { - (void)jsArgs->CreateDataProperty(context, static_cast(i), v8_helpers::MakeExternalV8String(isolate, cppArgs[i])); + // TODO: Switch to 2-bytes external string. + (void)jsArgs->CreateDataProperty(context, static_cast(i), v8_helpers::MakeV8String(isolate, cppArgs[i])); } args.GetReturnValue().Set(jsArgs); } diff --git a/src/module/core-modules/napa/store-wrap.cpp b/src/module/core-modules/napa/store-wrap.cpp index 9f4f152..923850d 100644 --- a/src/module/core-modules/napa/store-wrap.cpp +++ b/src/module/core-modules/napa/store-wrap.cpp @@ -54,7 +54,7 @@ void StoreWrap::SetCallback(const v8::FunctionCallbackInfo& args) { store.Set( v8_helpers::V8ValueTo(args[0]).c_str(), std::make_shared(napa::store::Store::ValueType { - v8_helpers::V8ValueTo(payload.ToLocalChecked()), + v8_helpers::V8ValueTo(payload.ToLocalChecked()), std::move(transportContext) })); } diff --git a/src/store/store.h b/src/store/store.h index 6e3e324..c1f8d88 100644 --- a/src/store/store.h +++ b/src/store/store.h @@ -20,7 +20,7 @@ namespace store { /// Meta-data that is necessary to marshall/unmarshall JS values. struct ValueType { /// JSON string from marshalled JS value. - std::string payload; + std::u16string payload; /// TransportContext that is needed to unmarshall the JS value. napa::transport::TransportContext transportContext; diff --git a/test/napa-zone/test.ts b/test/napa-zone/test.ts index 19a1978..e36381e 100644 --- a/test/napa-zone/test.ts +++ b/test/napa-zone/test.ts @@ -115,6 +115,21 @@ export function executeWithTransportableReturns(id: string): Promise { }); } +export function executeWithArgsContainingUnicodeString(id: string): Promise { + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; + let zone = napa.zone.get(id); + return new Promise((resolve, reject) => { + zone.execute((str: string) => { + var assert = require("assert"); + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; + assert.equal(str, unicodeStr); + return str; + }, [unicodeStr]) + .then ((result: napa.zone.Result) => resolve(result.value)) + .catch((error: any) => reject(error)); + }); +} + /// Memory test helpers. export function crtAllocatorTest(): void { let handle = napa.memory.crtAllocator.allocate(10); diff --git a/test/store-test.ts b/test/store-test.ts index d170a5c..299e762 100644 --- a/test/store-test.ts +++ b/test/store-test.ts @@ -41,8 +41,8 @@ describe('napajs/store', function () { assert.equal(store2.id, 'store2'); }); - it('@napa: store.get', () => { - napaZone.execute('./napa-zone/test', "getStoreTest"); + it('@napa: store.get', async () => { + await napaZone.execute('./napa-zone/test', "getStoreTest"); }); it('simple types: set in node, get in node', () => { @@ -50,14 +50,14 @@ describe('napajs/store', function () { assert.equal(store1.get('a'), 1); }); - it('simple types: set in node, get in napa', () => { + it('simple types: set in node, get in napa', async () => { store1.set('b', 'hi'); - napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'b', 'hi']); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'b', 'hi']); }); - it('simple types: set in napa, get in napa', () => { - napaZone.execute('./napa-zone/test', "storeSet", ['store1', 'c', 1]); - napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'c', 1]); + it('simple types: set in napa, get in napa', async () => { + await napaZone.execute('./napa-zone/test', "storeSet", ['store1', 'c', 1]); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'c', 1]); }); it('simple types: set in napa, get in node', async () => { @@ -68,20 +68,46 @@ describe('napajs/store', function () { }); }); + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; + let store2 = napa.store.create('store2'); + + it('unicode string: set in node, get in node', () => { + store2.set('a', unicodeStr); + assert.equal(store2.get('a'), unicodeStr); + }); + + it('unicode string: set in node, get in napa', async () => { + store2.set('b', unicodeStr); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store2', 'b', unicodeStr]); + }); + + it('unicode string: set in napa, get in napa', async () => { + await napaZone.execute('./napa-zone/test', "storeSet", ['store2', 'c', unicodeStr]); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store2', 'c', unicodeStr]); + }); + + it('unicode string: set in napa, get in node', async () => { + await napaZone.execute('./napa-zone/test', "storeSet", ['store2', 'd', { a: 1, b: unicodeStr}]); + assert.deepEqual(store2.get('d'), { + a: 1, + b: unicodeStr + }); + }); + it('transportable types: set in node, get in node', () => { store1.set('a', napa.memory.crtAllocator); assert.deepEqual(store1.get('a'), napa.memory.crtAllocator); }); - it('transportable types: set in node, get in napa', () => { + it('transportable types: set in node, get in napa', async () => { store1.set('b', napa.memory.defaultAllocator); - napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'b', napa.memory.defaultAllocator]); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'b', napa.memory.defaultAllocator]); }); it('transportable types: set in napa, get in napa', async () => { // We have to compare handle in this case, since napa.memory.defaultAllocator retrieved from napa zone will have 2+ refCount. await napaZone.execute('./napa-zone/test', "storeSet", ['store1', 'e', napa.memory.defaultAllocator]); - napaZone.execute('./napa-zone/test', "storeGetCompareHandle", ['store1', 'e', napa.memory.defaultAllocator.handle]); + await napaZone.execute('./napa-zone/test', "storeGetCompareHandle", ['store1', 'e', napa.memory.defaultAllocator.handle]); }); it('transportable types: set in napa, get in node', async () => { @@ -95,14 +121,14 @@ describe('napajs/store', function () { assert.equal(store1.get('g').toString(), (() => { return 0; }).toString()); }); - it('function type: set in node, get in napa', () => { + it('function type: set in node, get in napa', async () => { store1.set('h', () => { return 0; }); - napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'h', () => { return 0; }]); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'h', () => { return 0; }]); }); it('function type: set in napa, get in napa', async () => { await napaZone.execute('./napa-zone/test', "storeSet", ['store1', 'i', () => { return 0; }]); - napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'i', () => { return 0; }]); + await napaZone.execute('./napa-zone/test', "storeVerifyGet", ['store1', 'i', () => { return 0; }]); }); it('function type: set in napa, get in node', async () => { @@ -118,15 +144,15 @@ describe('napajs/store', function () { store1.delete('not-exist'); }); - it('delete in node, check in napa', () => { + it('delete in node, check in napa', async () => { assert(store1.has('b')); store1.delete('b'); - napaZone.execute('./napa-zone/test', "storeVerifyNotExist", ['store1', 'b']); + await napaZone.execute('./napa-zone/test', "storeVerifyNotExist", ['store1', 'b']); }); it('delete in napa, check in napa', async () => { await napaZone.execute('./napa-zone/test', "storeDelete", ['store1', 'c']); - napaZone.execute('./napa-zone/test', "storeVerifyNotExist", ['store1', 'c']); + await napaZone.execute('./napa-zone/test', "storeVerifyNotExist", ['store1', 'c']); }) it('delete in napa, check in node', async () => { diff --git a/test/transport-test.ts b/test/transport-test.ts index 7d3276c..13b9451 100644 --- a/test/transport-test.ts +++ b/test/transport-test.ts @@ -60,7 +60,7 @@ describe('napajs/transport', () => { }); it('@node: addon transportable', () => { - t.addonTransportTest(); + t.addonTransportTest(); }); it('@napa: addon transportable', () => { @@ -68,7 +68,7 @@ describe('napajs/transport', () => { }); it('@node: function transportable', () => { - t.functionTransportTest(); + t.functionTransportTest(); }); it('@napa: function transportable', () => { diff --git a/test/zone-test.ts b/test/zone-test.ts index 3e3b82f..c143e6f 100644 --- a/test/zone-test.ts +++ b/test/zone-test.ts @@ -508,6 +508,44 @@ describe('napajs/zone', function () { }); }); + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; // len = 92 + + it('@node: -> node zone with args containing unicode string', () => { + return napa.zone.current.execute((str: string) => { + var assert = require("assert"); + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; + assert.equal(str, unicodeStr); + return str; + }, [unicodeStr]).then((result: napa.zone.Result) => { + assert.equal(result.value, unicodeStr); + }); + }); + + it('@node: -> napa zone with args containing unicode string', () => { + return napaZone1.execute((str: string) => { + var assert = require("assert"); + let unicodeStr = "中文 español deutsch English हिन्दी العربية português বাংলা русский 日本語 ਪੰਜਾਬੀ 한국어 தமிழ் עברית"; + assert.equal(str, unicodeStr); + return unicodeStr; + }, [unicodeStr]).then((result: napa.zone.Result) => { + assert.equal(result.value, unicodeStr); + }); + }); + + it('@napa: -> napa zone with args containing unicode string', () => { + return napaZone1.execute('./napa-zone/test', "executeWithArgsContainingUnicodeString", ['napa-zone2']) + .then((result: napa.zone.Result) => { + assert.equal(result.value, unicodeStr); + }); + }); + + it('@napa: -> node zone with args containing unicode string', () => { + return napaZone1.execute('./napa-zone/test', "executeWithArgsContainingUnicodeString", ['node']) + .then((result: napa.zone.Result) => { + assert.equal(result.value, unicodeStr); + }); + }); + it.skip('@node: -> napa zone with timeout and succeed', () => { return napaZone1.execute('./napa-zone/test', 'waitMS', [1], {timeout: 100}); });