Use length to ensure null chars do not cause early termination of C string copies/reads

v8go.cc

@@ -808,12 +808,13 @@ ValuePtr NewValueIntegerFromUnsigned(IsolatePtr iso, uint32_t v) {
   return tracked_value(ctx, val);
 }
 
-RtnValue NewValueString(IsolatePtr iso, const char* v) {
+RtnValue NewValueString(IsolatePtr iso, const char* v, int v_length) {
   ISOLATE_SCOPE_INTERNAL_CONTEXT(iso);
   TryCatch try_catch(iso);
   RtnValue rtn = {};
   Local<String> str;
-  if (!String::NewFromUtf8(iso, v).ToLocal(&str)) {
+  if (!String::NewFromUtf8(iso, v, NewStringType::kNormal, v_length)
+           .ToLocal(&str)) {
     rtn.error = ExceptionError(try_catch, iso, ctx->ptr.Get(iso));
     return rtn;
   }
@@ -948,18 +949,23 @@ RtnString ValueToDetailString(ValuePtr ptr) {
     return rtn;
   }
   String::Utf8Value ds(iso, str);
-  rtn.string = CopyString(ds);
+  rtn.data = CopyString(ds);
   return rtn;
 }
 
-const char* ValueToString(ValuePtr ptr) {
+RtnString ValueToString(ValuePtr ptr) {
   LOCAL_VALUE(ptr);
+  RtnString rtn = {0};
   // String::Utf8Value will result in an empty string if conversion to a string
   // fails
   // TODO: Consider propagating the JS error. A fallback value could be returned
   // in Value.String()
-  String::Utf8Value utf8(iso, value);
-  return CopyString(utf8);
+  String::Utf8Value src(iso, value);
+  char* data = static_cast<char*>(malloc(src.length()));
+  memcpy(data, *src, src.length());
+  rtn.data = data;
+  rtn.length = src.length();
+  return rtn;
 }
 
 uint32_t ValueToUint32(ValuePtr ptr) {

v8go.h

@@ -106,7 +106,8 @@ typedef struct {
 } RtnValue;
 
 typedef struct {
-  const char* string;
+  const char* data;
+  int length;
   RtnError error;
 } RtnString;
 
@@ -193,7 +194,7 @@ extern ValuePtr NewValueNull(IsolatePtr iso_ptr);
 extern ValuePtr NewValueUndefined(IsolatePtr iso_ptr);
 extern ValuePtr NewValueInteger(IsolatePtr iso_ptr, int32_t v);
 extern ValuePtr NewValueIntegerFromUnsigned(IsolatePtr iso_ptr, uint32_t v);
-extern RtnValue NewValueString(IsolatePtr iso_ptr, const char* v);
+extern RtnValue NewValueString(IsolatePtr iso_ptr, const char* v, int v_length);
 extern ValuePtr NewValueBoolean(IsolatePtr iso_ptr, int v);
 extern ValuePtr NewValueNumber(IsolatePtr iso_ptr, double v);
 extern ValuePtr NewValueBigInt(IsolatePtr iso_ptr, int64_t v);
@@ -202,7 +203,7 @@ extern RtnValue NewValueBigIntFromWords(IsolatePtr iso_ptr,
                                         int sign_bit,
                                         int word_count,
                                         const uint64_t* words);
-const char* ValueToString(ValuePtr ptr);
+extern RtnString ValueToString(ValuePtr ptr);
 const uint32_t* ValueToArrayIndex(ValuePtr ptr);
 int ValueToBoolean(ValuePtr ptr);
 int32_t ValueToInt32(ValuePtr ptr);

value.go

@@ -73,7 +73,7 @@ func NewValue(iso *Isolate, val interface{}) (*Value, error) {
 	case string:
 		cstr := C.CString(v)
 		defer C.free(unsafe.Pointer(cstr))
-		rtn := C.NewValueString(iso.ptr, cstr)
+		rtn := C.NewValueString(iso.ptr, cstr, C.int(len(v)))
 		return valueResult(nil, rtn)
 	case int32:
 		rtnVal = &Value{
@@ -199,11 +199,11 @@ func (v *Value) Boolean() bool {
 // DetailString provide a string representation of this value usable for debugging.
 func (v *Value) DetailString() string {
 	rtn := C.ValueToDetailString(v.ptr)
-	if rtn.string == nil {
+	if rtn.data == nil {
 		err := newJSError(rtn.error)
 		panic(err) // TODO: Return a fallback value
 	}
-	s := rtn.string
+	s := rtn.data
 	defer C.free(unsafe.Pointer(s))
 	return C.GoString(s)
 }
@@ -242,8 +242,8 @@ func (v *Value) Object() *Object {
 // print their definition.
 func (v *Value) String() string {
 	s := C.ValueToString(v.ptr)
-	defer C.free(unsafe.Pointer(s))
-	return C.GoString(s)
+	defer C.free(unsafe.Pointer(s.data))
+	return C.GoStringN(s.data, C.int(s.length))
 }
 
 // Uint32 perform the equivalent of `Number(value)` in JS and convert the result to an

value_test.go

@@ -81,6 +81,7 @@ func TestValueString(t *testing.T) {
 	}{
 		{"Number", `13 * 2`, "26"},
 		{"String", `"string"`, "string"},
+		{"String with null terminators and Unicode", "String.fromCharCode(0, 55358, 56614, 8205, 9794, 65039, 0, 1040, 1041, 1042, 0, 97, 98, 99, 100)", "\x00\U0001f926\u200d\u2642\ufe0f\x00АБВ\x00abcd"},
 		{"Object", `let obj = {}; obj`, "[object Object]"},
 		{"Function", `let fn = function(){}; fn`, "function(){}"},
 	}
@@ -97,6 +98,44 @@ func TestValueString(t *testing.T) {
 	}
 }
 
+func TestValueString_GoToJSAndBack(t *testing.T) {
+	t.Parallel()
+	ctx := v8.NewContext(nil)
+	iso := ctx.Isolate()
+	defer iso.Dispose()
+	defer ctx.Close()
+
+	str := "s\x00s\x00"
+	jsStr, err := v8.NewValue(iso, str)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Test whether the go->js keeps the null chars
+	val, err := ctx.RunScript("(str) => { return str === String.fromCharCode(115, 0, 115, 0)}", "test.js")
+	if err != nil {
+		t.Fatal(err)
+	}
+	fn, err := val.AsFunction()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := fn.Call(ctx.Global(), jsStr)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.Boolean() {
+		t.Fatal("unexpected result: expected true, got false")
+	}
+
+	// Test whether the js->go keeps the null chars
+	goStr := jsStr.String()
+	if goStr != str {
+		t.Errorf("unexpected result: expected %q, got %q", str, goStr)
+	}
+}
+
 func TestValueDetailString(t *testing.T) {
 	t.Parallel()
 	ctx := v8.NewContext(nil)