commit 10b10c4c29fab79bd1bd4406871cf7f38a143871
author Tom Sepez <tsepez@chromium.org> Wed May 18 20:35:29 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed May 18 20:35:29 2022 +0000
tree 849816603f20bf7c1ab2a56cdbd34629cb9a1591
parent c9c736d4d859762f9c783abf0b5b00327cba8b7c

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>

fxjs/cjs_global.cpp

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