[1.7>master] [MERGE #3433 @kunalspathak] Jsrt: Modify signature of Js…

…CopyString

Merge pull request #3433 from pr/kunalspathak/1.7

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, &length);

   char* buffer = malloc(length);

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

can be changed to

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

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

      JsCopyString(jsString, buffer, actualLength, &writtenLength, &actualLength);
   }
```

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");

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)); }

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

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

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

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

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

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

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))

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

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