Added test for util.printx() and make it match spec.

fpdfsdk/javascript/util.cpp

Index: fpdfsdk/javascript/util.cpp
diff --git a/fpdfsdk/javascript/util.cpp b/fpdfsdk/javascript/util.cpp
index 30656cd76659c3f8176abd077fcbb1c182a55efc..671a862964452fbb497535f945b06bdb5023c43b 100644
--- a/fpdfsdk/javascript/util.cpp
+++ b/fpdfsdk/javascript/util.cpp
@@ -316,89 +316,109 @@ FX_BOOL util::printx(IJS_Context* cc,
   CFX_WideString sSource = params[1].ToCFXWideString();
   std::string cFormat = CFX_ByteString::FromUnicode(sFormat).c_str();
   std::string cSource = CFX_ByteString::FromUnicode(sSource).c_str();
-  std::string cDest;
-  printx(cFormat, cSource, cDest);
-  vRet = cDest.c_str();
+  vRet = printx(cFormat, cSource).c_str();
   return TRUE;
 }
 
-void util::printx(const std::string& cFormat,
-                  const std::string& cSource2,
-                  std::string& cPurpose) {
-  std::string cSource(cSource2);
-  if (!cPurpose.empty())
-    // cPurpose.clear();
-    cPurpose.erase();
-  int itSource = 0;
-  int iSize = cSource.size();
-  for (int iIndex = 0; iIndex < (int)cFormat.size() && itSource < iSize;
-       iIndex++) {
-    char letter = cFormat[iIndex];
-    switch (letter) {
-      case '?':
-        cPurpose += cSource[itSource];
-        itSource++;
-        break;
+enum CaseMode { kPreserveCase, kUpperCase, kLowerCase };
+
+static char TranslateCase(char input, CaseMode eMode) {
+  if (eMode == kLowerCase && input >= 'A' && input <= 'Z')
+    return input | 0x20;

[dsinclair, 2016/03/28 18:39:01]

Why not toupper and tolower for these?

[Tom Sepez, 2016/03/28 19:03:41]

ToUpper() isn't safe if |input| isn't an unsigned char value (undefined
behaviour per spec), and we're passing (possibly) signed chars here from
std::string operator[], and we might get a negative value.  Sometimes I think
the RTL is designed to hit you with gotchas on purpose.

+  if (eMode == kUpperCase && input >= 'a' && input <= 'z')
+    return input & ~0x20;
+  return input;
+}
+
+std::string util::printx(const std::string& cFormat,
+                         const std::string& cSource) {
+  std::string cResult;
+  size_t iSourceIndex = 0;
+  size_t iFormatIndex = 0;
+  CaseMode eCaseMode = kPreserveCase;
+  bool bEscaped = false;
+  while (iFormatIndex < cFormat.size()) {
+    if (bEscaped) {
+      bEscaped = false;
+      cResult += cFormat[iFormatIndex];
+      ++iFormatIndex;
+      continue;
+    }
+    switch (cFormat[iFormatIndex]) {
+      case '\\': {
+        bEscaped = true;
+        ++iFormatIndex;
+      } break;
+      case '<': {
+        eCaseMode = kUpperCase;

[dsinclair, 2016/03/28 18:39:01]

The original code did tolower for < and toupper for > was that a bug that's
fixed here?

[Tom Sepez, 2016/03/28 19:03:41]

No, I botched it.  Good catch.

+        ++iFormatIndex;
+      } break;
+      case '>': {
+        eCaseMode = kLowerCase;
+        ++iFormatIndex;
+      } break;
+      case '=': {
+        eCaseMode = kPreserveCase;
+        ++iFormatIndex;
+      } break;
+      case '?': {
+        if (iSourceIndex < cSource.size()) {
+          cResult += TranslateCase(cSource[iSourceIndex], eCaseMode);
+          ++iSourceIndex;
+        }
+        ++iFormatIndex;
+      } break;
       case 'X': {
-        while (itSource < iSize) {
-          if (std::isdigit(cSource[itSource]) ||
-              (cSource[itSource] >= 'a' && cSource[itSource] <= 'z') ||
-              (cSource[itSource] >= 'A' && cSource[itSource] <= 'Z')) {
-            cPurpose += cSource[itSource];
-            itSource++;
-            break;
+        if (iSourceIndex < cSource.size()) {
+          if (std::isdigit(cSource[iSourceIndex]) ||

[Tom Sepez, 2016/03/28 19:03:41]

This should also be avoided due to same issue.

+              (cSource[iSourceIndex] >= 'a' && cSource[iSourceIndex] <= 'z') ||

[dsinclair, 2016/03/28 18:39:01]

std::islower() and std::isupper() ?

[Tom Sepez, 2016/03/28 19:03:41]

Same issue.

+              (cSource[iSourceIndex] >= 'A' && cSource[iSourceIndex] <= 'Z')) {
+            cResult += TranslateCase(cSource[iSourceIndex], eCaseMode);
+            ++iFormatIndex;
           }
-          itSource++;
+          ++iSourceIndex;
+        } else {
+          ++iFormatIndex;
         }
-        break;
       } break;
       case 'A': {
-        while (itSource < iSize) {
-          if ((cSource[itSource] >= 'a' && cSource[itSource] <= 'z') ||
-              (cSource[itSource] >= 'A' && cSource[itSource] <= 'Z')) {
-            cPurpose += cSource[itSource];
-            itSource++;
-            break;
+        if (iSourceIndex < cSource.size()) {
+          if ((cSource[iSourceIndex] >= 'a' && cSource[iSourceIndex] <= 'z') ||

[dsinclair, 2016/03/28 18:39:01]

ditto

[Tom Sepez, 2016/03/28 19:03:41]

ditto.

+              (cSource[iSourceIndex] >= 'A' && cSource[iSourceIndex] <= 'Z')) {
+            cResult += TranslateCase(cSource[iSourceIndex], eCaseMode);
+            ++iFormatIndex;
           }
-          itSource++;
+          ++iSourceIndex;
+        } else {
+          ++iFormatIndex;
         }
-        break;
       } break;
       case '9': {
-        while (itSource < iSize) {
-          if (std::isdigit(cSource[itSource])) {
-            cPurpose += cSource[itSource];
-            itSource++;
-            break;
+        if (iSourceIndex < cSource.size()) {
+          if (std::isdigit(cSource[iSourceIndex])) {
+            cResult += cSource[iSourceIndex];
+            ++iFormatIndex;
           }
-          itSource++;
+          ++iSourceIndex;
+        } else {
+          ++iFormatIndex;
         }
-        break;
-      }
+      } break;
       case '*': {
-        cPurpose.append(cSource, itSource, iSize - itSource);
-        itSource = iSize - 1;
-        break;
-      }
-      case '\\':
-        break;
-      case '>': {
-        for (char& c : cSource)
-          c = toupper(c);
-        break;
-      }
-      case '<': {
-        for (char& c : cSource)
-          c = tolower(c);
-        break;
-      }
-      case '=':
-        break;
-      default:
-        cPurpose += letter;
-        break;
+        if (iSourceIndex < cSource.size()) {
+          cResult += TranslateCase(cSource[iSourceIndex], eCaseMode);
+          ++iSourceIndex;
+        } else {
+          ++iFormatIndex;
+        }
+      } break;
+      default: {
+        cResult += cFormat[iFormatIndex];
+        ++iFormatIndex;
+      } break;
     }
   }
+  return cResult;
 }
 
 FX_BOOL util::scand(IJS_Context* cc,