Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for JsPointerToString and JsCreateString #6669 #6737

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions ContributionAgreement.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,4 @@ This agreement has been signed by:
|Kevin Cadieux|kevcadieux|
|Aidan Bickford| BickfordA|
|Ryoichi Kaida| camcam-lemon|
|Christopher Raney| dagarxji|
7 changes: 7 additions & 0 deletions bin/NativeTests/JsRTApiTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,13 @@ namespace JsRTApiTest
JsValueRef value2 = JS_INVALID_REFERENCE;
REQUIRE(JsPointerToString(_u("value1"), wcslen(_u("value1")), &value2) == JsNoError);

// Test JsPointerToString and JsCreateString on NULL inputs
JsValueRef nullStr;
REQUIRE(JsPointerToString(NULL, 0, &nullStr) == JsNoError);

JsValueRef nullFname;
REQUIRE(JsCreateString(NULL, 0, &nullFname) == JsNoError);

REQUIRE(JsSetProperty(object, name1, value1, true) == JsNoError);
REQUIRE(JsSetProperty(object, name2, value2, true) == JsNoError);

Expand Down
10 changes: 8 additions & 2 deletions lib/Jsrt/ChakraCommonWindows.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -323,7 +324,12 @@
/// Creates a string value from a string pointer.
/// </summary>
/// <remarks>
/// Requires an active script context.
/// <para>
/// Requires an active script context.
/// </para>
/// <para>
/// Providing length of 0 will ignore input string pointer and return a Js empty string.
/// </para>
/// </remarks>
/// <param name="stringValue">The string pointer to convert to a string value.</param>
/// <param name="stringLength">The length of the string to convert.</param>
Expand All @@ -333,7 +339,7 @@
/// </returns>
CHAKRA_API
JsPointerToString(
_In_reads_(stringLength) const wchar_t *stringValue,
_In_reads_opt_(stringLength) const wchar_t *stringValue,
_In_ size_t stringLength,
_Out_ JsValueRef *value);

Expand Down
6 changes: 5 additions & 1 deletion lib/Jsrt/ChakraCore.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------
/// \mainpage Chakra Hosting API Reference
Expand Down Expand Up @@ -505,6 +506,9 @@ typedef bool (CHAKRA_CALLBACK * JsSerializedLoadScriptCallback)
/// <para>
/// Input string can be either ASCII or Utf8
/// </para>
/// <para>
/// Providing length of 0 will ignore input string pointer and return a Js empty string.
/// </para>
/// </remarks>
/// <param name="content">Pointer to string memory.</param>
/// <param name="length">Number of bytes within the string</param>
Expand All @@ -514,7 +518,7 @@ typedef bool (CHAKRA_CALLBACK * JsSerializedLoadScriptCallback)
/// </returns>
CHAKRA_API
JsCreateString(
_In_ const char *content,
_In_opt_ const char *content,
_In_ size_t length,
_Out_ JsValueRef *value);

Expand Down
22 changes: 15 additions & 7 deletions lib/Jsrt/Core/JsrtCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -868,11 +868,15 @@ static void CastCopy(const SrcChar* src, DstChar* dst, size_t count)
}

CHAKRA_API JsCreateString(
_In_ const char *content,
_In_opt_ const char *content,
_In_ size_t length,
_Out_ JsValueRef *value)
{
PARAM_NOT_NULL(content);
if (length != 0)
{
PARAM_NOT_NULL(content);
}

PARAM_NOT_NULL(value);
*value = JS_INVALID_REFERENCE;

Expand All @@ -887,13 +891,17 @@ CHAKRA_API JsCreateString(
}

return ContextAPINoScriptWrapper([&](Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode {

Js::JavascriptString *stringValue = Js::LiteralStringWithPropertyStringPtr::
NewFromCString(content, (CharCount)length, scriptContext->GetLibrary());

NewFromCString(content, (CharCount)length, scriptContext->GetLibrary());
Comment on lines 894 to +895
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These calls should not happen on the length = 0 path, may be the cause of the test failures

PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTCreateString, stringValue->GetSz(), stringValue->GetLength());

*value = stringValue;
if (length != 0)
{
*value = stringValue;
}
else
{
*value = scriptContext->GetLibrary()->GetEmptyString();
}

PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, value);

Expand Down
22 changes: 15 additions & 7 deletions lib/Jsrt/Jsrt.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//-------------------------------------------------------------------------------------------------------
// Copyright (C) Microsoft. All rights reserved.
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
//-------------------------------------------------------------------------------------------------------
#include "JsrtPch.h"
Expand Down Expand Up @@ -1229,20 +1230,27 @@ CHAKRA_API JsGetStringLength(_In_ JsValueRef value, _Out_ int *length)
END_JSRT_NO_EXCEPTION
}

CHAKRA_API JsPointerToString(_In_reads_(stringLength) const WCHAR *stringValue, _In_ size_t stringLength, _Out_ JsValueRef *string)
CHAKRA_API JsPointerToString(_In_reads_opt_(stringLength) const WCHAR *stringValue, _In_ size_t stringLength, _Out_ JsValueRef *string)
{
return ContextAPINoScriptWrapper([&](Js::ScriptContext *scriptContext, TTDRecorder& _actionEntryPopper) -> JsErrorCode {
PERFORM_JSRT_TTD_RECORD_ACTION(scriptContext, RecordJsRTCreateString, stringValue, stringLength);

PARAM_NOT_NULL(stringValue);

PARAM_NOT_NULL(string);

if (!Js::IsValidCharCount(stringLength))
if (stringLength == 0)
{
Js::JavascriptError::ThrowOutOfMemoryError(scriptContext);
*string = scriptContext->GetLibrary()->GetEmptyString();
}
else
{
PARAM_NOT_NULL(stringValue);

if (!Js::IsValidCharCount(stringLength))
{
Js::JavascriptError::ThrowOutOfMemoryError(scriptContext);
}
*string = Js::JavascriptString::NewCopyBuffer(stringValue, static_cast<charcount_t>(stringLength), scriptContext);
}

*string = Js::JavascriptString::NewCopyBuffer(stringValue, static_cast<charcount_t>(stringLength), scriptContext);

PERFORM_JSRT_TTD_RECORD_ACTION_RESULT(scriptContext, string);

Expand Down