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

@@ -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 {

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(

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<uint16_t*>(::malloc(len * sizeof(uint16_t)));
+      intptr_t utf8_len = 0;
+      for (intptr_t i = 0; i < len; i++) {
+        utf16[i] = Read<uint16_t>();
+        // 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<Dart_CObject::Type>(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<double>(object->value.as_double);
       break;
     case Dart_CObject::kString: {
+      const uint8_t* utf8_str =
+          reinterpret_cast<const uint8_t*>(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<uint8_t>(str[i]);
+      if (type == Utf8::kAscii) {
+        for (intptr_t i = 0; i < len; i++) {
+          Write<uint8_t>(utf8_str[i]);
+        }
+      } else {
+        // TODO(sgjesse): Make sure surrogate pairs are handled.
+        uint16_t* utf16_str =
+            reinterpret_cast<uint16_t*>(::malloc(len * sizeof(uint16_t)));
+        Utf8::DecodeToUTF16(utf8_str, utf8_len, utf16_str, len);
+        for (intptr_t i = 0; i < len; i++) {
+          Write<uint16_t>(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

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_;

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<const uint8_t*>(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<char*>("456");
   EXPECT(Dart_PostCObject(send_port_id, &object));
 
+  object.type = Dart_CObject::kString;
+  object.value.as_string = const_cast<char*>("æøå");
+  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<char*>(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();

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.