Jsrt: Modify signature of JsCopyString

kunalspathak · kunalspathak · commit 356656c26eee · 2017-07-27T14:37:00.000-07:00
Modified signature of `JsCopyString` to also return actual count of UTF8 bytes present in jsString.
With this information, host can simply allocate a buffer assuming all characters are ascii and
based on `writtenLength` and `actualLength` values returned by the API, it can decide if the assumption was correct
i.e. `writtenLength == actualLength` or it should take slow path to call `JsCopyString` again by passing bigger buffer
equal to size of`actualLength`.

Today, if host wants to copy UTF8 representation of javascript string into a buffer, here is how it is done:

```c++
   size_t length = 0;
   JsCopyString(jsString, nullptr, 0, &amp;length);

   char* buffer = malloc(length);

   size_t written = 0;
   JsCopyString(jsString, buffer, length, &amp;written);
   assert(written == length);
```

can be changed to

```c++
   size_t actualLength = 0;
   size_t writtenLength = 0;
   size_t strLength = 0;
   JsStringToPointer(strRef, nullptr, &amp;strLength);
   char* buffer = malloc(strLength + 1);
   JsCopyString(jsString, buffer, strLength, &amp;writtenLength, &amp;actualLength);

   // slow path if jsString contains non-ascii characters
   if(written != actualLength) {
      free(buffer);
      buffer = malloc(actualLength + 1);

      JsCopyString(jsString, buffer, actualLength, &amp;writtenLength, &amp;actualLength);
   }
```
diff --git a/bin/ch/ChakraRtInterface.cpp b/bin/ch/ChakraRtInterface.cpp
@@ -144,6 +144,7 @@ bool ChakraRTInterface::LoadChakraDll(ArgInfo* argInfo, HINSTANCE *outLibrary)
     m_jsApiHooks.pfJsrtParse = (JsAPIHooks::JsrtParse)GetChakraCoreSymbol(library, "JsParse");
     m_jsApiHooks.pfJsrtSerialize = (JsAPIHooks::JsrtSerialize)GetChakraCoreSymbol(library, "JsSerialize");
     m_jsApiHooks.pfJsrtRunSerialized = (JsAPIHooks::JsrtRunSerialized)GetChakraCoreSymbol(library, "JsRunSerialized");
+    m_jsApiHooks.pfJsrtGetStringLength = (JsAPIHooks::JsrtGetStringLength)GetChakraCoreSymbol(library, "JsGetStringLength");
     m_jsApiHooks.pfJsrtCreateString = (JsAPIHooks::JsrtCreateString)GetChakraCoreSymbol(library, "JsCreateString");
     m_jsApiHooks.pfJsrtCreateStringUtf16 = (JsAPIHooks::JsrtCreateStringUtf16)GetChakraCoreSymbol(library, "JsCreateStringUtf16");
     m_jsApiHooks.pfJsrtCopyString = (JsAPIHooks::JsrtCopyString)GetChakraCoreSymbol(library, "JsCopyString");
diff --git a/bin/ch/ChakraRtInterface.h b/bin/ch/ChakraRtInterface.h
@@ -77,7 +77,8 @@ struct JsAPIHooks
     typedef JsErrorCode(WINAPI *JsrtParse)(JsValueRef script, JsSourceContext sourceContext, JsValueRef sourceUrl, JsParseScriptAttributes parseAttributes, JsValueRef *result);
     typedef JsErrorCode(WINAPI *JsrtSerialize)(JsValueRef script, JsValueRef *buffer, JsParseScriptAttributes parseAttributes);
     typedef JsErrorCode(WINAPI *JsrtRunSerialized)(JsValueRef buffer, JsSerializedLoadScriptCallback scriptLoadCallback, JsSourceContext sourceContext, JsValueRef sourceUrl, JsValueRef * result);
-    typedef JsErrorCode(WINAPI *JsrtCopyString)(JsValueRef value, char* buffer, size_t bufferSize, size_t* written);
+    typedef JsErrorCode(WINAPI *JsrtGetStringLength)(JsValueRef value, int *stringLength);
+    typedef JsErrorCode(WINAPI *JsrtCopyString)(JsValueRef value, char* buffer, size_t bufferSize, size_t* writtenLength, size_t* actualLength);
     typedef JsErrorCode(WINAPI *JsrtCreateString)(const char *content, size_t length, JsValueRef *value);
     typedef JsErrorCode(WINAPI *JsrtCreateStringUtf16)(const uint16_t *content, size_t length, JsValueRef *value);
     
@@ -172,6 +173,7 @@ struct JsAPIHooks
     JsrtParse pfJsrtParse;
     JsrtSerialize pfJsrtSerialize;
     JsrtRunSerialized pfJsrtRunSerialized;
+    JsrtGetStringLength pfJsrtGetStringLength;
     JsrtCreateString pfJsrtCreateString;
     JsrtCreateStringUtf16 pfJsrtCreateStringUtf16;
     JsrtCopyString pfJsrtCopyString;
@@ -396,7 +398,8 @@ class ChakraRTInterface
     static JsErrorCode WINAPI JsParse(JsValueRef script, JsSourceContext sourceContext, JsValueRef sourceUrl, JsParseScriptAttributes parseAttributes, JsValueRef *result) { return HOOK_JS_API(Parse(script, sourceContext, sourceUrl, parseAttributes, result)); }
     static JsErrorCode WINAPI JsSerialize(JsValueRef script, JsValueRef *buffer, JsParseScriptAttributes parseAttributes) { return HOOK_JS_API(Serialize(script, buffer, parseAttributes)); }
     static JsErrorCode WINAPI JsRunSerialized(JsValueRef buffer, JsSerializedLoadScriptCallback scriptLoadCallback, JsSourceContext sourceContext, JsValueRef sourceUrl, JsValueRef * result) { return HOOK_JS_API(RunSerialized(buffer, scriptLoadCallback, sourceContext, sourceUrl, result)); }
-    static JsErrorCode WINAPI JsCopyString(JsValueRef value, char* buffer, size_t bufferSize, size_t* written) { return HOOK_JS_API(CopyString(value, buffer, bufferSize, written)); }
+    static JsErrorCode WINAPI JsGetStringLength(JsValueRef value, int *stringLength) { return HOOK_JS_API(GetStringLength(value, stringLength)); }
+    static JsErrorCode WINAPI JsCopyString(JsValueRef value, char* buffer, size_t bufferSize, size_t* writtenLength, size_t* actualLength) { return HOOK_JS_API(CopyString(value, buffer, bufferSize, writtenLength, actualLength)); }
     static JsErrorCode WINAPI JsCreateString(const char *content, size_t length, JsValueRef *value) { return HOOK_JS_API(CreateString(content, length, value)); }
     static JsErrorCode WINAPI JsCreateStringUtf16(const uint16_t *content, size_t length, JsValueRef *value) { return HOOK_JS_API(CreateStringUtf16(content, length, value)); }
     static JsErrorCode WINAPI JsCreatePropertyId(const char *name, size_t length, JsPropertyIdRef *propertyId) { return HOOK_JS_API(CreatePropertyId(name, length, propertyId)); }
diff --git a/bin/ch/stdafx.h b/bin/ch/stdafx.h
@@ -206,18 +206,39 @@ class AutoString
         {
             strValue = value;
         }
+        int strLen = 0;
+        size_t writtenLen = 0;
+        size_t actualLen = 0;
         if (errorCode == JsNoError)
         {
-            size_t len = 0;
-            errorCode = ChakraRTInterface::JsCopyString(strValue, nullptr, 0, &len);
+            errorCode = ChakraRTInterface::JsGetStringLength(strValue, &strLen);
             if (errorCode == JsNoError)
             {
-                data = (char*) malloc((len + 1) * sizeof(char));
-                ChakraRTInterface::JsCopyString(strValue, data, len + 1, &length);
-                AssertMsg(len == length, "If you see this message.. There is something seriously wrong. Good Luck!");
-                *(data + len) = char(0);
+                // Assume ascii characters
+                data = (char*)malloc((strLen + 1) * sizeof(char));
+                errorCode = ChakraRTInterface::JsCopyString(strValue, data, strLen, &writtenLen, &actualLen);
+                if (errorCode == JsNoError)
+                {
+                    // If non-ascii, take slow path
+                    if (writtenLen != actualLen)
+                    {
+                        free(data);
+                        data = (char*)malloc((actualLen + 1) * sizeof(char));
+
+                        errorCode = ChakraRTInterface::JsCopyString(strValue, data, actualLen + 1, &writtenLen, nullptr);
+                        if (errorCode == JsNoError)
+                        {
+                            AssertMsg(actualLen == writtenLen, "If you see this message.. There is something seriously wrong. Good Luck!");
+                            
+                        }
+                    }
+                }
             }
         }
+        if (errorCode == JsNoError)
+        {
+            *(data + actualLen) = char(0);
+        }
         return errorCode;
     }
 
diff --git a/lib/Jsrt/ChakraCore.h b/lib/Jsrt/ChakraCore.h
@@ -286,13 +286,23 @@ CHAKRA_API
 ///     <para>
 ///         When size of the `buffer` is unknown,
 ///         `buffer` argument can be nullptr.
-///         In that case, `written` argument will return the length needed.
+///         In that case, `actualLength` argument will return the length needed to
+///         accomodate all the UTF8 decoded bytes present in `value`.
+///         `writtenLength` will only get populated when valid `buffer` is passed as
+///          argument.
 ///     </para>
 /// </remarks>
 /// <param name="value">JavascriptString value</param>
 /// <param name="buffer">Pointer to buffer</param>
 /// <param name="bufferSize">Buffer size</param>
-/// <param name="written">Total number of characters written</param>
+/// <param name="writtenLength">Total number of UTF8 decoded bytes written. This is only
+///                             populated when passed with non-null `buffer`,else is
+///                             set to 0.
+/// </param>
+/// <param name="actualLength">Total number of UTF8 decoded bytes present in `value`.
+///                            Useful to initialize buffer of appropriate size that
+///                            can be passed in to this API.
+/// </param>
 /// <returns>
 ///     The code <c>JsNoError</c> if the operation succeeded, a failure code otherwise.
 /// </returns>
@@ -301,7 +311,8 @@ CHAKRA_API
         _In_ JsValueRef value,
         _Out_opt_ char* buffer,
         _In_ size_t bufferSize,
-        _Out_opt_ size_t* written);
+        _Out_opt_ size_t* writtenLength,
+        _Out_opt_ size_t* actualLength);
 
 /// <summary>
 ///     Write string value into Utf16 string buffer
diff --git a/lib/Jsrt/Jsrt.cpp b/lib/Jsrt/Jsrt.cpp
@@ -4252,7 +4252,8 @@ CHAKRA_API JsCopyString(
     _In_ JsValueRef value,
     _Out_opt_ char* buffer,
     _In_ size_t bufferSize,
-    _Out_opt_ size_t* length)
+    _Out_opt_ size_t* writtenLength,
+    _Out_opt_ size_t* actualLength)
 {
     PARAM_NOT_NULL(value);
     VALIDATE_JSREF(value);
@@ -4266,14 +4267,12 @@ CHAKRA_API JsCopyString(
     }
 
     utf8::WideToNarrow utf8Str(str, strLength);
-    if (!buffer)
+    if (actualLength)
     {
-        if (length)
-        {
-            *length = utf8Str.Length();
-        }
+      *actualLength = utf8Str.Length();
     }
-    else
+
+    if (buffer)
     {
         size_t count = min(bufferSize, utf8Str.Length());
         // Try to copy whole characters if buffer size insufficient
@@ -4284,11 +4283,15 @@ CHAKRA_API JsCopyString(
             (LPCUTF8)(const char*)utf8Str, utf8Str.Length(), maxFitChars);
 
         memmove(buffer, utf8Str, sizeof(char) * count);
-        if (length)
+        if (writtenLength)
         {
-            *length = count;
+            *writtenLength = count;
         }
     }
+    else if (writtenLength)
+    {
+        *writtenLength = 0;
+    }
 
     return JsNoError;
 }
diff --git a/test/native-tests/test-c98/sample.cpp b/test/native-tests/test-c98/sample.cpp
@@ -63,9 +63,9 @@ int main()
     // Project script result back to C++.
     char *resultSTR = nullptr;
     size_t stringLength;
-    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
+    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
     resultSTR = (char*)malloc(stringLength + 1);
-    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
+    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
     resultSTR[stringLength] = 0;
 
     printf("Result -> %s \n", resultSTR);
diff --git a/test/native-tests/test-char/sample.cpp b/test/native-tests/test-char/sample.cpp
@@ -73,9 +73,9 @@ int main()
     // Project script result back to C++.
     char *resultSTR = nullptr;
     size_t stringLength;
-    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
+    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
     resultSTR = (char*)malloc(stringLength + 1);
-    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
+    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
     resultSTR[stringLength] = 0;
 
     printf("Result -> %s \n", resultSTR);
diff --git a/test/native-tests/test-char16/sample.cpp b/test/native-tests/test-char16/sample.cpp
@@ -69,9 +69,9 @@ int main()
     // Project script result back to C++.
     char *resultSTR = nullptr;
     size_t stringLength;
-    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
+    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
     resultSTR = (char*) malloc(stringLength + 1);
-    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
+    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
     resultSTR[stringLength] = 0;
 
     printf("Result -> %s \n", resultSTR);
diff --git a/test/native-tests/test-python/helloWorld.py b/test/native-tests/test-python/helloWorld.py
@@ -56,12 +56,12 @@
 
 stringLength = c_size_t();
 # Get buffer size needed for the result string
-chakraCore.JsCopyString(resultJSString, 0, 0, byref(stringLength));
+chakraCore.JsCopyString(resultJSString, 0, 0, 0, byref(stringLength));
 
 resultSTR = create_string_buffer(stringLength.value + 1); # buffer is big enough to store the result
 
 # Get String from JsValueRef
-chakraCore.JsCopyString(resultJSString, byref(resultSTR), stringLength.value + 1, 0);
+chakraCore.JsCopyString(resultJSString, byref(resultSTR), stringLength.value + 1, 0, 0);
 
 # Set `null-ending` to the end
 resultSTRLastByte = (c_char * stringLength.value).from_address(addressof(resultSTR))
diff --git a/test/native-tests/test-shared-basic/sample.cpp b/test/native-tests/test-shared-basic/sample.cpp
@@ -57,9 +57,9 @@ int main()
     // Project script result back to C++.
     char *resultSTR = nullptr;
     size_t stringLength;
-    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
+    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
     resultSTR = (char*) malloc(stringLength + 1);
-    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
+    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
     resultSTR[stringLength] = 0;
 
     printf("Result -> %s \n", resultSTR);
diff --git a/test/native-tests/test-static-native/sample.cpp b/test/native-tests/test-static-native/sample.cpp
@@ -57,9 +57,9 @@ int main()
     // Project script result back to C++.
     char *resultSTR = nullptr;
     size_t stringLength;
-    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, &stringLength));
+    FAIL_CHECK(JsCopyString(resultJSString, nullptr, 0, nullptr, &stringLength));
     resultSTR = (char*) malloc(stringLength + 1);
-    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr));
+    FAIL_CHECK(JsCopyString(resultJSString, resultSTR, stringLength + 1, nullptr, nullptr));
     resultSTR[stringLength] = 0;
 
     printf("Result -> %s \n", resultSTR);

Original file line number	Diff line number	Diff line change
`@@ -4252,7 +4252,8 @@ CHAKRA_API JsCopyString(`
`4252`	`4252`	`_In_ JsValueRef value,`
`4253`	`4253`	`_Out_opt_ char* buffer,`
`4254`	`4254`	`_In_ size_t bufferSize,`
`4255`		`- _Out_opt_ size_t* length)`
	`4255`	`+ _Out_opt_ size_t* writtenLength,`
	`4256`	`+ _Out_opt_ size_t* actualLength)`
`4256`	`4257`	`{`
`4257`	`4258`	`PARAM_NOT_NULL(value);`
`4258`	`4259`	`VALIDATE_JSREF(value);`
`@@ -4266,14 +4267,12 @@ CHAKRA_API JsCopyString(`
`4266`	`4267`	`}`
`4267`	`4268`
`4268`	`4269`	`utf8::WideToNarrow utf8Str(str, strLength);`
`4269`		`- if (!buffer)`
	`4270`	`+ if (actualLength)`
`4270`	`4271`	`{`
`4271`		`- if (length)`
`4272`		`- {`
`4273`		`- *length = utf8Str.Length();`
`4274`		`- }`
	`4272`	`+ *actualLength = utf8Str.Length();`
`4275`	`4273`	`}`
`4276`		`- else`
	`4274`	`+`
	`4275`	`+ if (buffer)`
`4277`	`4276`	`{`
`4278`	`4277`	`size_t count = min(bufferSize, utf8Str.Length());`
`4279`	`4278`	`// Try to copy whole characters if buffer size insufficient`
`@@ -4284,11 +4283,15 @@ CHAKRA_API JsCopyString(`
`4284`	`4283`	`(LPCUTF8)(const char*)utf8Str, utf8Str.Length(), maxFitChars);`
`4285`	`4284`
`4286`	`4285`	`memmove(buffer, utf8Str, sizeof(char) * count);`
`4287`		`- if (length)`
	`4286`	`+ if (writtenLength)`
`4288`	`4287`	`{`
`4289`		`- *length = count;`
	`4288`	`+ *writtenLength = count;`
`4290`	`4289`	`}`
`4291`	`4290`	`}`
	`4291`	`+ else if (writtenLength)`
	`4292`	`+ {`
	`4293`	`+ *writtenLength = 0;`
	`4294`	`+ }`
`4292`	`4295`
`4293`	`4296`	`return JsNoError;`
`4294`	`4297`	`}`