Skip to content

Commit a4e93f4

Browse files
MikeHolmanMikeHolman
committed
[MERGE #5966 @MikeHolman] fix JsGetAndClearExceptionWithMetadata in scripts with unicode
Merge pull request #5966 from MikeHolman:unicodeexception Line offset was wrong for the first line with a unicode character Fixes #5805
2 parents 4a18b8a + e997d44 commit a4e93f4

File tree

2 files changed

+53
-87
lines changed

2 files changed

+53
-87
lines changed

‎bin/NativeTests/JsRTApiTest.cpp‎

Lines changed: 46 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,6 +1005,41 @@ namespace JsRTApiTest
10051005
JsRTApiTest::RunWithAttributes(JsRTApiTest::EngineFlagTest);
10061006
}
10071007

1008+
void CheckExceptionMetadata(JsValueRef exceptionMetadata)
1009+
{
1010+
JsPropertyIdRef property = JS_INVALID_REFERENCE;
1011+
JsValueRef metadataValue = JS_INVALID_REFERENCE;
1012+
JsValueType type;
1013+
REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
1014+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1015+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1016+
CHECK(type == JsError);
1017+
1018+
REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
1019+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1020+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1021+
CHECK(type == JsNumber);
1022+
1023+
REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
1024+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1025+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1026+
CHECK(type == JsNumber);
1027+
1028+
REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
1029+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1030+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1031+
CHECK(type == JsNumber);
1032+
1033+
REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
1034+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1035+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1036+
CHECK(type == JsString);
1037+
1038+
REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
1039+
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1040+
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1041+
CHECK(type == JsString);
1042+
}
10081043
void ExceptionHandlingTest(JsRuntimeAttributes attributes, JsRuntimeHandle runtime)
10091044
{
10101045
bool value;
@@ -1039,31 +1074,7 @@ namespace JsRTApiTest
10391074
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
10401075
CHECK(metadataValue == exception);
10411076

1042-
REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
1043-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1044-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1045-
CHECK(type == JsNumber);
1046-
1047-
REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
1048-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1049-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1050-
CHECK(type == JsNumber);
1051-
1052-
REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
1053-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1054-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1055-
CHECK(type == JsNumber);
1056-
1057-
REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
1058-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1059-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1060-
CHECK(type == JsString);
1061-
1062-
REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
1063-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1064-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1065-
CHECK(type == JsString);
1066-
1077+
CheckExceptionMetadata(exceptionMetadata);
10671078

10681079
REQUIRE(JsHasException(&value) == JsNoError);
10691080
CHECK(value == false);
@@ -1079,35 +1090,18 @@ namespace JsRTApiTest
10791090
REQUIRE(JsHasException(&value) == JsNoError);
10801091
CHECK(value == false);
10811092

1082-
REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
1083-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1084-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1085-
CHECK(type == JsError);
1086-
1087-
REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
1088-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1089-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1090-
CHECK(type == JsNumber);
1093+
CheckExceptionMetadata(exceptionMetadata);
10911094

1092-
REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
1093-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1094-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1095-
CHECK(type == JsNumber);
1096-
1097-
REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
1098-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1099-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1100-
CHECK(type == JsNumber);
1095+
// Test unicode characters
1096+
REQUIRE(JsRunScript(_u("function main() {\n var x = '\u20ac' + test();\n}\nmain();"), JS_SOURCE_CONTEXT_NONE, _u(""), nullptr) == JsErrorScriptException);
1097+
REQUIRE(JsHasException(&value) == JsNoError);
1098+
CHECK(value == true);
11011099

1102-
REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
1103-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1104-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1105-
CHECK(type == JsString);
1100+
REQUIRE(JsGetAndClearExceptionWithMetadata(&exceptionMetadata) == JsNoError);
1101+
REQUIRE(JsHasException(&value) == JsNoError);
1102+
CHECK(value == false);
11061103

1107-
REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
1108-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1109-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1110-
CHECK(type == JsString);
1104+
CheckExceptionMetadata(exceptionMetadata);
11111105

11121106
// Following requires eval to be enabled - no point in testing it if we've disabled eval
11131107
if (!(attributes & JsRuntimeAttributeDisableEval))
@@ -1120,35 +1114,7 @@ namespace JsRTApiTest
11201114
REQUIRE(JsHasException(&value) == JsNoError);
11211115
CHECK(value == false);
11221116

1123-
REQUIRE(JsGetPropertyIdFromName(_u("exception"), &property) == JsNoError);
1124-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1125-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1126-
CHECK(type == JsError);
1127-
1128-
REQUIRE(JsGetPropertyIdFromName(_u("line"), &property) == JsNoError);
1129-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1130-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1131-
CHECK(type == JsNumber);
1132-
1133-
REQUIRE(JsGetPropertyIdFromName(_u("column"), &property) == JsNoError);
1134-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1135-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1136-
CHECK(type == JsNumber);
1137-
1138-
REQUIRE(JsGetPropertyIdFromName(_u("length"), &property) == JsNoError);
1139-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1140-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1141-
CHECK(type == JsNumber);
1142-
1143-
REQUIRE(JsGetPropertyIdFromName(_u("url"), &property) == JsNoError);
1144-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1145-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1146-
CHECK(type == JsString);
1147-
1148-
REQUIRE(JsGetPropertyIdFromName(_u("source"), &property) == JsNoError);
1149-
REQUIRE(JsGetProperty(exceptionMetadata, property, &metadataValue) == JsNoError);
1150-
REQUIRE(JsGetValueType(metadataValue, &type) == JsNoError);
1151-
CHECK(type == JsString);
1117+
CheckExceptionMetadata(exceptionMetadata);
11521118
}
11531119
}
11541120

‎lib/Runtime/Base/LineOffsetCache.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,18 +217,18 @@ namespace Js
217217
void LineOffsetCache::AddLine(Recycler * allocator, charcount_t characterOffset, charcount_t byteOffset)
218218
{
219219
LineOffsetCacheList * characterOffsetList = (LineOffsetCacheList *)(LineOffsetCacheReadOnlyList*)this->lineCharacterOffsetCacheList;
220-
characterOffsetList->Add(characterOffset);
221220
LineOffsetCacheList * byteOffsetList = (LineOffsetCacheList *)(LineOffsetCacheReadOnlyList*)this->lineByteOffsetCacheList;
222-
if (byteOffsetList == nullptr && characterOffset != byteOffset)
221+
if (characterOffset != byteOffset && byteOffsetList == nullptr)
223222
{
224-
byteOffsetList = RecyclerNew(allocator, LineOffsetCacheList, allocator);
225-
byteOffsetList->Copy(characterOffsetList);
226-
this->lineByteOffsetCacheList = byteOffsetList;
223+
byteOffsetList = RecyclerNew(allocator, LineOffsetCacheList, allocator);
224+
byteOffsetList->Copy(characterOffsetList);
225+
this->lineByteOffsetCacheList = byteOffsetList;
227226
}
228-
else if (byteOffsetList != nullptr)
227+
if (byteOffsetList != nullptr)
229228
{
230229
byteOffsetList->Add(byteOffset);
231-
}
230+
}
231+
characterOffsetList->Add(characterOffset);
232232

233233
#if DBG
234234
Assert(this->lineByteOffsetCacheList == nullptr || this->lineByteOffsetCacheList->Count() == this->lineCharacterOffsetCacheList->Count());

0 commit comments

Comments
 (0)