Support Unicode names/values in CJS globals
Currently, these are manipulated using default ANSI, meaning the strings
are interpreted (on Windows at least) according to whatever code page
is in effect for the system, with non-conforming characters removed (or
replaced by placeholder '?' chars on Windows). These stored strings also
risk being corrupted if the system default code page changes.
These strings all come directly from JS as UTF-16, so we know precisely
what they mean without requiring code page considerations. So convert
everything to UTF-8 and avoid platform differences.
This would be a breaking change if any platform actually implemented
persistence and had previously stored, say, non-ASCII key/values. This
seems unlikely.
-- Downcast name to string rather than invoking toString().
-- Avoid converting from UTF-8 only to reconvert back to UTF-8
under the covers in fxv8.
Change-Id: I0e0298fa8429c59215e2c5a44560308cab72ccb3
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/93772
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fxjs/cjs_global.cpp b/fxjs/cjs_global.cpp
index e117da9..412dd8d 100644
--- a/fxjs/cjs_global.cpp
+++ b/fxjs/cjs_global.cpp
@@ -26,17 +26,8 @@
ByteString ByteStringFromV8Name(v8::Isolate* pIsolate,
v8::Local<v8::Name> name) {
- DCHECK(name->IsString());
-
- // Yes, this looks insane to make a v8 string, a wide string, and then a
- // byte string in separate steps when we could just make a v8 string and
- // then a byte string. But let's pretend there's persistent data somewhere
- // in some embedder, and so we can't change the key name, which currently
- // omits chars > 0xFF. There isn't a ByteString method equivalent.
- return fxv8::ToWideString(
- pIsolate,
- name->ToString(pIsolate->GetCurrentContext()).ToLocalChecked())
- .ToDefANSI();
+ CHECK(name->IsString());
+ return fxv8::ToByteString(pIsolate, name.As<v8::String>());
}
} // namespace
@@ -203,8 +194,8 @@
case CFX_Value::DataType::kBoolean:
return CJS_Result::Success(pRuntime->NewBoolean(pData->bData));
case CFX_Value::DataType::kString:
- return CJS_Result::Success(pRuntime->NewString(
- WideString::FromDefANSI(pData->sData.AsStringView()).AsStringView()));
+ return CJS_Result::Success(
+ pRuntime->NewString(pData->sData.AsStringView()));
case CFX_Value::DataType::kObject:
return CJS_Result::Success(
v8::Local<v8::Object>::New(pRuntime->GetIsolate(), pData->pData));
@@ -231,7 +222,7 @@
}
if (vp->IsString()) {
return SetGlobalVariables(propname, CFX_Value::DataType::kString, 0, false,
- pRuntime->ToWideString(vp).ToDefANSI(),
+ pRuntime->ToByteString(vp),
v8::Local<v8::Object>(), false);
}
if (vp->IsObject()) {
@@ -270,7 +261,7 @@
if (params.size() != 2)
return CJS_Result::Failure(JSMessage::kParamError);
- auto it = m_MapGlobal.find(pRuntime->ToWideString(params[0]).ToDefANSI());
+ auto it = m_MapGlobal.find(pRuntime->ToByteString(params[0]));
if (it == m_MapGlobal.end() || it->second->bDeleted)
return CJS_Result::Failure(JSMessage::kGlobalNotFoundError);
@@ -308,9 +299,7 @@
pData->bPersistent);
pRuntime->PutObjectProperty(
ToV8Object(), pData->data.sKey.AsStringView(),
- pRuntime->NewString(
- WideString::FromUTF8(pData->data.sData.AsStringView())
- .AsStringView()));
+ pRuntime->NewString(pData->data.sData.AsStringView()));
break;
case CFX_Value::DataType::kObject: {
v8::Local<v8::Object> pObj = pRuntime->NewObject();
@@ -399,7 +388,7 @@
continue;
}
if (v->IsString()) {
- ByteString sValue = pRuntime->ToWideString(v).ToDefANSI();
+ ByteString sValue = pRuntime->ToByteString(v);
auto pObjElement = std::make_unique<CFX_KeyValue>();
pObjElement->nType = CFX_Value::DataType::kString;
pObjElement->sKey = sKey;
@@ -445,9 +434,7 @@
case CFX_Value::DataType::kString:
pRuntime->PutObjectProperty(
pObj, pObjData->sKey.AsStringView(),
- pRuntime->NewString(
- WideString::FromUTF8(pObjData->sData.AsStringView())
- .AsStringView()));
+ pRuntime->NewString(pObjData->sData.AsStringView()));
break;
case CFX_Value::DataType::kObject: {
v8::Local<v8::Object> pNewObj = pRuntime->NewObject();
diff --git a/testing/resources/javascript/globals.in b/testing/resources/javascript/globals.in
index 3079213..bbca813 100644
--- a/testing/resources/javascript/globals.in
+++ b/testing/resources/javascript/globals.in
@@ -59,7 +59,11 @@
// Test null and undefined.
{ "name": "null_var", "value": null },
- { "name": "undefined_var", "value": undefined }
+ { "name": "undefined_var", "value": undefined },
+
+ // Test to show unicode currently handled.
+ { "name": "unicode_var", "value": "\u4025\u4026_string" },
+ { "name": "\u4025\u4026_var", "value": "string" },
];
function setup_global() {
diff --git a/testing/resources/javascript/globals_expected.txt b/testing/resources/javascript/globals_expected.txt
index 26c878b..9e80a62 100644
--- a/testing/resources/javascript/globals_expected.txt
+++ b/testing/resources/javascript/globals_expected.txt
@@ -10,6 +10,8 @@
Alert: object_var = undefined
Alert: null_var = undefined
Alert: undefined_var = undefined
+Alert: unicode_var = undefined
+Alert: 䀥䀦_var = undefined
Alert: ************ After Setup ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
@@ -19,7 +21,9 @@
Alert: object_var = [object Object], own property = true
Alert: string_var = This is a string, own property = true
Alert: true_var = true, own property = true
+Alert: unicode_var = 䀥䀦_string, own property = true
Alert: zero_var = 0, own property = true
+Alert: 䀥䀦_var = string, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
@@ -32,6 +36,8 @@
Alert: blue
Alert: null_var = null
Alert: undefined_var = undefined
+Alert: unicode_var = 䀥䀦_string
+Alert: 䀥䀦_var = string
Alert: ************ After Deletion ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
@@ -44,6 +50,8 @@
Alert: object_var = undefined
Alert: null_var = undefined
Alert: undefined_var = undefined
+Alert: unicode_var = undefined
+Alert: 䀥䀦_var = undefined
Alert: For undefined_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: ************ After Setup and Persist false ************
Alert: Enumerable Globals:
@@ -54,7 +62,9 @@
Alert: object_var = [object Object], own property = true
Alert: string_var = This is a string, own property = true
Alert: true_var = true, own property = true
+Alert: unicode_var = 䀥䀦_string, own property = true
Alert: zero_var = 0, own property = true
+Alert: 䀥䀦_var = string, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
@@ -67,6 +77,8 @@
Alert: blue
Alert: null_var = null
Alert: undefined_var = undefined
+Alert: unicode_var = 䀥䀦_string
+Alert: 䀥䀦_var = string
Alert: For true_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: For false_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: For zero_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
@@ -75,6 +87,8 @@
Alert: For object_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: For null_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: For undefined_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
+Alert: For unicode_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
+Alert: For 䀥䀦_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: ************ After Delete and Persist ************
Alert: Enumerable Globals:
Alert: setPersistent = function setPersistent() { [native code] }, own property = true
@@ -87,6 +101,8 @@
Alert: object_var = undefined
Alert: null_var = undefined
Alert: undefined_var = undefined
+Alert: unicode_var = undefined
+Alert: 䀥䀦_var = undefined
Alert: For undefined_var: Set Persistent: ERROR: global.setPersistent: Global value not found.
Alert: ************ After Setup and Persist true ************
Alert: Enumerable Globals:
@@ -97,7 +113,9 @@
Alert: object_var = [object Object], own property = true
Alert: string_var = This is a string, own property = true
Alert: true_var = true, own property = true
+Alert: unicode_var = 䀥䀦_string, own property = true
Alert: zero_var = 0, own property = true
+Alert: 䀥䀦_var = string, own property = true
Alert: Expected Globals:
Alert: true_var = true
Alert: false_var = false
@@ -110,3 +128,5 @@
Alert: blue
Alert: null_var = null
Alert: undefined_var = undefined
+Alert: unicode_var = 䀥䀦_string
+Alert: 䀥䀦_var = string
