From cde0bb06dd4d5af613845112ceeddf72f90f9eed Mon Sep 17 00:00:00 2001 From: "sgjesse@google.com" Date: Wed, 14 Nov 2012 13:01:57 +0000 Subject: [PATCH] Add support for non ASCII strings when communicating with native ports The string representation in a Dart_CObject strructure is now UTF8. All strings read are now converted to UTF8 from either ASCII or UTF16 serialization. All strings posted should be valid UTF8 and are serialized as either ASCII or UTF16 depending on the content. Proper andling of surrogate pairs is missing, but will be added when https://codereview.chromium.org/11368138/ lands. R=ager@google.com, erikcorry@google.com, asiva@google.com BUG= Review URL: https://codereview.chromium.org//11410032 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@14887 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/include/dart_api.h | 2 + runtime/vm/dart_api_impl.cc | 7 +- runtime/vm/dart_api_message.cc | 102 ++++++++++++++++++++++------- runtime/vm/dart_api_message.h | 10 +-- runtime/vm/snapshot_test.cc | 75 +++++++++++++++++---- tests/standalone/standalone.status | 6 +- 6 files changed, 157 insertions(+), 45 deletions(-) diff --git a/runtime/include/dart_api.h b/runtime/include/dart_api.h index 33f1775d7fc..1766a89e7bd 100755 --- a/runtime/include/dart_api.h +++ b/runtime/include/dart_api.h @@ -913,6 +913,8 @@ DART_EXPORT bool Dart_Post(Dart_Port port_id, Dart_Handle object); * data outside the Dart heap. These objects are totally detached from * the Dart heap. Only a subset of the Dart objects have a * representation as a Dart_CObject. + * + * The string encoding in the 'value.as_string' is UTF-8. */ typedef struct _Dart_CObject { enum Type { diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc index f3ea2810f78..c69f875919e 100644 --- a/runtime/vm/dart_api_impl.cc +++ b/runtime/vm/dart_api_impl.cc @@ -1019,10 +1019,13 @@ DART_EXPORT bool Dart_PostIntArray(Dart_Port port_id, } -DART_EXPORT bool Dart_PostCObject(Dart_Port port_id, Dart_CObject* message) { +DART_EXPORT bool Dart_PostCObject(Dart_Port port_id, + Dart_CObject* message) { uint8_t* buffer = NULL; ApiMessageWriter writer(&buffer, allocator); - writer.WriteCMessage(message); + bool success = writer.WriteCMessage(message); + + if (!success) return success; // Post the message at the given port. return PortMap::PostMessage(new Message( diff --git a/runtime/vm/dart_api_message.cc b/runtime/vm/dart_api_message.cc index 2759e099f11..6958098afc8 100644 --- a/runtime/vm/dart_api_message.cc +++ b/runtime/vm/dart_api_message.cc @@ -6,6 +6,7 @@ #include "vm/object.h" #include "vm/snapshot_ids.h" #include "vm/symbols.h" +#include "vm/unicode.h" namespace dart { @@ -355,9 +356,30 @@ Dart_CObject* ApiMessageReader::ReadInternalVMObject(intptr_t class_id, p[len] = '\0'; return object; } - case kTwoByteStringCid: - // Two byte strings not supported. - return AllocateDartCObjectUnsupported(); + case kTwoByteStringCid: { + intptr_t len = ReadSmiValue(); + intptr_t hash = ReadSmiValue(); + USE(hash); + uint16_t *utf16 = + reinterpret_cast(::malloc(len * sizeof(uint16_t))); + intptr_t utf8_len = 0; + for (intptr_t i = 0; i < len; i++) { + utf16[i] = Read(); + // TODO(sgjesse): Check for surrogate pairs. + utf8_len += Utf8::Length(utf16[i]); + } + Dart_CObject* object = AllocateDartCObjectString(utf8_len); + AddBackRef(object_id, object, kIsDeserialized); + char* p = object->value.as_string; + for (intptr_t i = 0; i < len; i++) { + // TODO(sgjesse): Check for surrogate pairs. + p += Utf8::Encode(utf16[i], p); + } + *p = '\0'; + ASSERT(p == object->value.as_string + utf8_len); + ::free(utf16); + return object; + } case kUint8ArrayCid: { intptr_t len = ReadSmiValue(); Dart_CObject* object = AllocateDartCObjectUint8Array(len); @@ -616,11 +638,11 @@ void ApiMessageWriter::WriteInlinedHeader(Dart_CObject* object) { } -void ApiMessageWriter::WriteCObject(Dart_CObject* object) { +bool ApiMessageWriter::WriteCObject(Dart_CObject* object) { if (IsCObjectMarked(object)) { intptr_t object_id = GetMarkedCObjectMark(object); WriteIndexedObject(kMaxPredefinedObjectIds + object_id); - return; + return true; } Dart_CObject::Type type = object->type; @@ -636,19 +658,20 @@ void ApiMessageWriter::WriteCObject(Dart_CObject* object) { WriteNullObject(); // Write out array elements. for (int i = 0; i < object->value.as_array.length; i++) { - WriteCObjectRef(object->value.as_array.values[i]); + bool success = WriteCObjectRef(object->value.as_array.values[i]); + if (!success) return false; } - return; + return true; } - WriteCObjectInlined(object, type); + return WriteCObjectInlined(object, type); } -void ApiMessageWriter::WriteCObjectRef(Dart_CObject* object) { +bool ApiMessageWriter::WriteCObjectRef(Dart_CObject* object) { if (IsCObjectMarked(object)) { intptr_t object_id = GetMarkedCObjectMark(object); WriteIndexedObject(kMaxPredefinedObjectIds + object_id); - return; + return true; } Dart_CObject::Type type = object->type; @@ -661,13 +684,13 @@ void ApiMessageWriter::WriteCObjectRef(Dart_CObject* object) { WriteSmi(object->value.as_array.length); // Add object to forward list so that this object is serialized later. AddToForwardList(object); - return; + return true; } - WriteCObjectInlined(object, type); + return WriteCObjectInlined(object, type); } -void ApiMessageWriter::WriteForwardedCObject(Dart_CObject* object) { +bool ApiMessageWriter::WriteForwardedCObject(Dart_CObject* object) { ASSERT(IsCObjectMarked(object)); Dart_CObject::Type type = static_cast(object->type & kDartCObjectTypeMask); @@ -685,12 +708,14 @@ void ApiMessageWriter::WriteForwardedCObject(Dart_CObject* object) { WriteNullObject(); // Write out array elements. for (int i = 0; i < object->value.as_array.length; i++) { - WriteCObjectRef(object->value.as_array.values[i]); + bool success = WriteCObjectRef(object->value.as_array.values[i]); + if (!success) return false; } + return true; } -void ApiMessageWriter::WriteCObjectInlined(Dart_CObject* object, +bool ApiMessageWriter::WriteCObjectInlined(Dart_CObject* object, Dart_CObject::Type type) { switch (type) { case Dart_CObject::kNull: @@ -734,18 +759,38 @@ void ApiMessageWriter::WriteCObjectInlined(Dart_CObject* object, Write(object->value.as_double); break; case Dart_CObject::kString: { + const uint8_t* utf8_str = + reinterpret_cast(object->value.as_string); + intptr_t utf8_len = strlen(object->value.as_string); + if (!Utf8::IsValid(utf8_str, utf8_len)) { + return false; + } + + Utf8::Type type; + intptr_t len = Utf8::CodePointCount(utf8_str, utf8_len, &type); + // Write out the serialization header value for this object. WriteInlinedHeader(object); // Write out the class and tags information. - WriteIndexedObject(kOneByteStringCid); + WriteIndexedObject(type == Utf8::kAscii ? kOneByteStringCid + : kTwoByteStringCid); WriteIntptrValue(0); // Write string length, hash and content - char* str = object->value.as_string; - intptr_t len = strlen(str); WriteSmi(len); WriteSmi(0); // TODO(sgjesse): Hash - not written. - for (intptr_t i = 0; i < len; i++) { - Write(str[i]); + if (type == Utf8::kAscii) { + for (intptr_t i = 0; i < len; i++) { + Write(utf8_str[i]); + } + } else { + // TODO(sgjesse): Make sure surrogate pairs are handled. + uint16_t* utf16_str = + reinterpret_cast(::malloc(len * sizeof(uint16_t))); + Utf8::DecodeToUTF16(utf8_str, utf8_len, utf16_str, len); + for (intptr_t i = 0; i < len; i++) { + Write(utf16_str[i]); + } + ::free(utf16_str); } break; } @@ -788,18 +833,29 @@ void ApiMessageWriter::WriteCObjectInlined(Dart_CObject* object, default: UNREACHABLE(); } + + return true; } -void ApiMessageWriter::WriteCMessage(Dart_CObject* object) { - WriteCObject(object); +bool ApiMessageWriter::WriteCMessage(Dart_CObject* object) { + bool success = WriteCObject(object); + if (!success) { + UnmarkAllCObjects(object); + return false; + } // Write out all objects that were added to the forward list and have // not been serialized yet. These would typically be fields of arrays. // NOTE: The forward list might grow as we process the list. for (intptr_t i = 0; i < forward_id_; i++) { - WriteForwardedCObject(forward_list_[i]); + success = WriteForwardedCObject(forward_list_[i]); + if (!success) { + UnmarkAllCObjects(object); + return false; + } } UnmarkAllCObjects(object); + return true; } } // namespace dart diff --git a/runtime/vm/dart_api_message.h b/runtime/vm/dart_api_message.h index 57341da9aeb..c37d8fbc250 100644 --- a/runtime/vm/dart_api_message.h +++ b/runtime/vm/dart_api_message.h @@ -131,7 +131,7 @@ class ApiMessageWriter : public BaseWriter { void WriteMessage(intptr_t field_count, intptr_t *data); // Writes a message with a single object. - void WriteCMessage(Dart_CObject* object); + bool WriteCMessage(Dart_CObject* object); private: static const intptr_t kDartCObjectTypeBits = 4; @@ -152,10 +152,10 @@ class ApiMessageWriter : public BaseWriter { void WriteInt32(Dart_CObject* object); void WriteInt64(Dart_CObject* object); void WriteInlinedHeader(Dart_CObject* object); - void WriteCObject(Dart_CObject* object); - void WriteCObjectRef(Dart_CObject* object); - void WriteForwardedCObject(Dart_CObject* object); - void WriteCObjectInlined(Dart_CObject* object, Dart_CObject::Type type); + bool WriteCObject(Dart_CObject* object); + bool WriteCObjectRef(Dart_CObject* object); + bool WriteForwardedCObject(Dart_CObject* object); + bool WriteCObjectInlined(Dart_CObject* object, Dart_CObject::Type type); intptr_t object_id_; Dart_CObject** forward_list_; diff --git a/runtime/vm/snapshot_test.cc b/runtime/vm/snapshot_test.cc index 3ee8ed704ad..083b28fc568 100644 --- a/runtime/vm/snapshot_test.cc +++ b/runtime/vm/snapshot_test.cc @@ -11,6 +11,7 @@ #include "vm/dart_api_state.h" #include "vm/snapshot.h" #include "vm/symbols.h" +#include "vm/unicode.h" #include "vm/unit_test.h" namespace dart { @@ -497,13 +498,12 @@ TEST_CASE(SerializeSingletons) { } -TEST_CASE(SerializeString) { +static void TestString(const char* cstr) { StackZone zone(Isolate::Current()); - + EXPECT(Utf8::IsValid(reinterpret_cast(cstr), strlen(cstr))); // Write snapshot with object content. uint8_t* buffer; MessageWriter writer(&buffer, &zone_allocator); - static const char* cstr = "This string shall be serialized"; String& str = String::Handle(String::New(cstr)); writer.WriteMessage(str); intptr_t buffer_len = writer.BytesWritten(); @@ -525,6 +525,21 @@ TEST_CASE(SerializeString) { } +TEST_CASE(SerializeString) { + TestString("This string shall be serialized"); + TestString("æøå"); // This file is UTF-8 encoded. + char data[] = {0x01, + 0x7f, + 0xc2, 0x80, // 0x80 + 0xdf, 0xbf, // 0x7ff + 0xe0, 0xa0, 0x80, // 0x800 + 0xef, 0xbf, 0xbf, // 0xffff + 0x00}; // String termination. + TestString(data); + // TODO(sgjesse): Add tests with non-BMP characters. +} + + TEST_CASE(SerializeArray) { StackZone zone(Isolate::Current()); @@ -1250,9 +1265,12 @@ UNIT_TEST_CASE(DartGeneratedMessages) { "getBigint() {\n" " return -0x424242424242424242424242424242424242;\n" "}\n" - "getString() {\n" + "getAsciiString() {\n" " return \"Hello, world!\";\n" "}\n" + "getNonAsciiString() {\n" + " return \"Blåbærgrød\";\n" + "}\n" "getList() {\n" " return new List(kArrayLength);\n" "}\n"; @@ -1271,10 +1289,15 @@ UNIT_TEST_CASE(DartGeneratedMessages) { Dart_Handle bigint_result; bigint_result = Dart_Invoke(lib, NewString("getBigint"), 0, NULL); EXPECT_VALID(bigint_result); - Dart_Handle string_result; - string_result = Dart_Invoke(lib, NewString("getString"), 0, NULL); - EXPECT_VALID(string_result); - EXPECT(Dart_IsString(string_result)); + Dart_Handle ascii_string_result; + ascii_string_result = Dart_Invoke(lib, NewString("getAsciiString"), 0, NULL); + EXPECT_VALID(ascii_string_result); + EXPECT(Dart_IsString(ascii_string_result)); + Dart_Handle non_ascii_string_result; + non_ascii_string_result = + Dart_Invoke(lib, NewString("getNonAsciiString"), 0, NULL); + EXPECT_VALID(non_ascii_string_result); + EXPECT(Dart_IsString(non_ascii_string_result)); { DARTSCOPE_NOCHECKS(isolate); @@ -1321,7 +1344,7 @@ UNIT_TEST_CASE(DartGeneratedMessages) { uint8_t* buffer; MessageWriter writer(&buffer, &zone_allocator); String& str = String::Handle(); - str ^= Api::UnwrapHandle(string_result); + str ^= Api::UnwrapHandle(ascii_string_result); writer.WriteMessage(str); intptr_t buffer_len = writer.BytesWritten(); @@ -1334,6 +1357,24 @@ UNIT_TEST_CASE(DartGeneratedMessages) { EXPECT_STREQ("Hello, world!", root->value.as_string); CheckEncodeDecodeMessage(root); } + { + StackZone zone(Isolate::Current()); + uint8_t* buffer; + MessageWriter writer(&buffer, &zone_allocator); + String& str = String::Handle(); + str ^= Api::UnwrapHandle(non_ascii_string_result); + writer.WriteMessage(str); + intptr_t buffer_len = writer.BytesWritten(); + + // Read object back from the snapshot into a C structure. + ApiNativeScope scope; + ApiMessageReader api_reader(buffer, buffer_len, &zone_allocator); + Dart_CObject* root = api_reader.ReadMessage(); + EXPECT_NOTNULL(root); + EXPECT_EQ(Dart_CObject::kString, root->type); + EXPECT_STREQ("Blåbærgrød", root->value.as_string); + CheckEncodeDecodeMessage(root); + } } Dart_ExitScope(); Dart_ShutdownIsolate(); @@ -2013,7 +2054,7 @@ UNIT_TEST_CASE(PostCObject) { " var exception = '';\n" " var port = new ReceivePort();\n" " port.receive((message, replyTo) {\n" - " if (messageCount < 7) {\n" + " if (messageCount < 8) {\n" " exception = '$exception${message}';\n" " } else {\n" " exception = '$exception${message.length}';\n" @@ -2022,7 +2063,7 @@ UNIT_TEST_CASE(PostCObject) { " }\n" " }\n" " messageCount++;\n" - " if (messageCount == 8) throw new Exception(exception);\n" + " if (messageCount == 9) throw new Exception(exception);\n" " });\n" " return port.toSendPort();\n" "}\n"; @@ -2061,6 +2102,16 @@ UNIT_TEST_CASE(PostCObject) { object.value.as_string = const_cast("456"); EXPECT(Dart_PostCObject(send_port_id, &object)); + object.type = Dart_CObject::kString; + object.value.as_string = const_cast("æøå"); + EXPECT(Dart_PostCObject(send_port_id, &object)); + + // Try to post an invalid UTF-8 sequence (lead surrogate). + char data[] = {0xed, 0xa0, 0x80, 0}; // 0xd800 + object.type = Dart_CObject::kString; + object.value.as_string = const_cast(data); + EXPECT(!Dart_PostCObject(send_port_id, &object)); + object.type = Dart_CObject::kDouble; object.value.as_double = 3.14; EXPECT(Dart_PostCObject(send_port_id, &object)); @@ -2091,7 +2142,7 @@ UNIT_TEST_CASE(PostCObject) { result = Dart_RunLoop(); EXPECT(Dart_IsError(result)); EXPECT(Dart_ErrorHasException(result)); - EXPECT_SUBSTRING("Exception: nulltruefalse1234563.14[]100123456789\n", + EXPECT_SUBSTRING("Exception: nulltruefalse123456æøå3.14[]100123456789\n", Dart_GetError(result)); Dart_ExitScope(); diff --git a/tests/standalone/standalone.status b/tests/standalone/standalone.status index 129a73f22dd..17cc926ae3d 100644 --- a/tests/standalone/standalone.status +++ b/tests/standalone/standalone.status @@ -6,9 +6,7 @@ out_of_memory_test: Skip # Issue 2345 package/invalid_uri_test: Fail, OK # Fails intentionally -[ $runtime == vm ] -io/directory_non_ascii_test: Fail # Posting non-ascii strings through ports. -io/file_non_ascii_test: Fail # Posting non-ascii strings through ports. +[ $system == macos ] [ $runtime == vm && $checked ] # These tests have type errors on purpose. @@ -33,6 +31,8 @@ io/socket_many_connections_test: Skip io/file_non_ascii_sync_test: Fail # issue 6702 io/directory_non_ascii_sync_test: Fail # issue 6702 io/process_non_ascii_test: Fail # issue 6702 +io/directory_non_ascii_test: Fail # issue 6702 +io/file_non_ascii_test: Fail # issue 6702 [ $runtime == vm && $system == windows ] io/file_system_links_test: Skip # No links on Windows.