Skip to content

Commit 053cc44

Browse files
committed
VSO 8294189: Fix two incorrect, high-hitting asserts in RegexHelper::StringReplace.
In the function ``` RegexHelper::StringReplace(JavascriptString* match, JavascriptString* input, JavascriptFunction* replacefn) ``` These two asserts would fail when `replacefn` is cross-site (where `scriptContext` is the context of the `replacefn`): ``` Assert(match->GetScriptContext() == scriptContext); Assert(input->GetScriptContext() == scriptContext); ``` These asserts were overzealous. There is no actual problem here because the `replacefn`'s `ScriptContext` is not used for any outputs from this function and therefore is not observable. The `scriptContext` in which `EntryReplace` is called should match `match->GetScriptContext()` and `input->GetScriptContext()`. The fix for these asserts is to pass in the `scriptContext` from the calling function and check against that `scriptContext`. It is okay for the `scriptContext` of `replacefn` to be different, as the parameters and results will be marshalled correctly in other code. Replaces #2458
1 parent aaf9e82 commit 053cc44

File tree

5 files changed

+38
-4
lines changed

5 files changed

+38
-4
lines changed

lib/Runtime/Library/JavascriptString.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1632,7 +1632,7 @@ namespace Js
16321632
AssertMsg(pMatch != nullptr, "Match string shouldn't be null");
16331633
if (replacefn != nullptr)
16341634
{
1635-
return RegexHelper::StringReplace(pMatch, input, replacefn);
1635+
return RegexHelper::StringReplace(scriptContext, pMatch, input, replacefn);
16361636
}
16371637
else
16381638
{

lib/Runtime/Library/RegexHelper.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,10 +1389,9 @@ namespace Js
13891389
return concatenated.ToString();
13901390
}
13911391

1392-
Var RegexHelper::StringReplace(JavascriptString* match, JavascriptString* input, JavascriptFunction* replacefn)
1392+
Var RegexHelper::StringReplace(ScriptContext* scriptContext, JavascriptString* match, JavascriptString* input, JavascriptFunction* replacefn)
13931393
{
13941394
CharCount indexMatched = JavascriptString::strstr(input, match, true);
1395-
ScriptContext* scriptContext = replacefn->GetScriptContext();
13961395
Assert(match->GetScriptContext() == scriptContext);
13971396
Assert(input->GetScriptContext() == scriptContext);
13981397

@@ -1412,6 +1411,7 @@ namespace Js
14121411
postfixStr, postfixLength);
14131412
return bufferString.ToString();
14141413
}
1414+
14151415
return input;
14161416
}
14171417

lib/Runtime/Library/RegexHelper.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ namespace Js
105105
static Var RegexReplace(ScriptContext* scriptContext, RecyclableObject* thisObj, JavascriptString* input, JavascriptString* replace, bool noResult);
106106
static Var RegexReplaceFunction(ScriptContext* scriptContext, RecyclableObject* thisObj, JavascriptString* input, JavascriptFunction* replacefn);
107107
static Var StringReplace(JavascriptString* regularExpression, JavascriptString* input, JavascriptString* replace);
108-
static Var StringReplace(JavascriptString* regularExpression, JavascriptString* input, JavascriptFunction* replacefn);
108+
static Var StringReplace(ScriptContext* scriptContext, JavascriptString* regularExpression, JavascriptString* input, JavascriptFunction* replacefn);
109109
static Var RegexSplitResultUsed(ScriptContext* scriptContext, JavascriptRegExp* regularExpression, JavascriptString* input, CharCount limit);
110110
static Var RegexSplitResultUsedAndMayBeTemp(void *const stackAllocationPointer, ScriptContext* scriptContext, JavascriptRegExp* regularExpression, JavascriptString* input, CharCount limit);
111111
static Var RegexSplitResultNotUsed(ScriptContext* scriptContext, JavascriptRegExp* regularExpression, JavascriptString* input, CharCount limit);

test/Strings/replace-xsite.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
//-------------------------------------------------------------------------------------------------------
2+
// Copyright (C) Microsoft. All rights reserved.
3+
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
4+
//-------------------------------------------------------------------------------------------------------
5+
6+
WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js");
7+
8+
var tests = [
9+
{
10+
name: "String.Replace ( Pattern as String , ReplaceFunction )",
11+
body: function () {
12+
assert.doesNotThrow(function() {
13+
var str = "AAoooAAooo";
14+
var script = `
15+
function foo(str) {
16+
return {
17+
toString: function () { return str + "ZZ" }
18+
}
19+
}`
20+
21+
var replacer = WScript.LoadScript(script, 'samethread')
22+
var replaced = str.replace("A", replacer.foo);
23+
});
24+
}
25+
}
26+
];
27+
28+
testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" });

test/Strings/rlexe.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,12 @@
115115
<baseline>replace.baseline</baseline>
116116
</default>
117117
</test>
118+
<test>
119+
<default>
120+
<files>replace-xsite.js</files>
121+
<compile-flags>-args summary -endargs</compile-flags>
122+
</default>
123+
</test>
118124
<test>
119125
<default>
120126
<files>trim.js</files>

0 commit comments

Comments
 (0)