commit 1e2d7dfcc0631018aa105f72264a0e6d30a6abe4
author Tom Sepez <tsepez@chromium.org> Wed Apr 07 19:26:06 2021 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Apr 07 19:26:06 2021 +0000
tree b584c229b78c27a68ab99d05b223be3230648003
parent edff9f18c669e3e14209ae07e307369c00b372a3

Rewrite ParsePageRangeString to make saner.

One ill-formatted test now fails.

Change-Id: I1fe6b1a0896643260e319665a3fe7777aa29ab5b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/79023
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

fpdfsdk/cpdfsdk_helpers.cpp

diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index 3711658..2f8c54e 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -454,54 +454,38 @@
 
 std::vector<uint32_t> ParsePageRangeString(const ByteString& bsPageRange,
                                            uint32_t nCount) {
-  ByteString bsStrippedPageRange = bsPageRange;
-  bsStrippedPageRange.Remove(' ');
-  size_t nLength = bsStrippedPageRange.GetLength();
-  if (nLength == 0)
-    return std::vector<uint32_t>();
-
-  static const ByteString cbCompareString("0123456789-,");
-  for (size_t i = 0; i < nLength; ++i) {
-    if (!cbCompareString.Contains(bsStrippedPageRange[i]))
+  ByteStringView alphabet(" 0123456789-,");
+  for (const auto& ch : bsPageRange) {
+    if (!alphabet.Contains(ch))
       return std::vector<uint32_t>();
   }
 
-  ByteString cbMidRange;
-  size_t nStringFrom = 0;
-  size_t nStringTo = 0;
-  std::vector<uint32_t> pageArray;
-  while (nStringTo < nLength) {
-    nStringTo = bsStrippedPageRange.Find(',', nStringFrom).value_or(nLength);
-    cbMidRange =
-        bsStrippedPageRange.Substr(nStringFrom, nStringTo - nStringFrom);
-    Optional<size_t> nDashPosition = cbMidRange.Find('-');
-    if (nDashPosition) {
-      size_t nMid = nDashPosition.value();
-      uint32_t nStartPageNum = pdfium::base::checked_cast<uint32_t>(
-          atoi(cbMidRange.First(nMid).c_str()));
-      if (nStartPageNum == 0)
-        return std::vector<uint32_t>();
+  ByteString bsStrippedPageRange = bsPageRange;
+  bsStrippedPageRange.Remove(' ');
 
-      ++nMid;
-      size_t nEnd = cbMidRange.GetLength() - nMid;
-      if (nEnd == 0)
+  std::vector<uint32_t> results;
+  for (const auto& entry : fxcrt::Split(bsStrippedPageRange, ',')) {
+    std::vector<ByteString> args = fxcrt::Split(entry, '-');
+    if (args.size() == 1) {
+      uint32_t page_num =
+          pdfium::base::checked_cast<uint32_t>(atoi(args[0].c_str()));
+      if (page_num == 0 || page_num > nCount)
         return std::vector<uint32_t>();
-
-      uint32_t nEndPageNum = pdfium::base::checked_cast<uint32_t>(
-          atoi(cbMidRange.Substr(nMid, nEnd).c_str()));
-      if (nStartPageNum > nEndPageNum || nEndPageNum > nCount)
+      results.push_back(page_num);
+    } else if (args.size() == 2) {
+      uint32_t first_num =
+          pdfium::base::checked_cast<uint32_t>(atoi(args[0].c_str()));
+      if (first_num == 0)
         return std::vector<uint32_t>();
-
-      for (uint32_t i = nStartPageNum; i <= nEndPageNum; ++i)
-        pageArray.push_back(i);
+      uint32_t last_num =
+          pdfium::base::checked_cast<uint32_t>(atoi(args[1].c_str()));
+      if (last_num == 0 || first_num > last_num || last_num > nCount)
+        return std::vector<uint32_t>();
+      for (uint32_t i = first_num; i <= last_num; ++i)
+        results.push_back(i);
     } else {
-      uint32_t nPageNum =
-          pdfium::base::checked_cast<uint32_t>(atoi(cbMidRange.c_str()));
-      if (nPageNum <= 0 || nPageNum > nCount)
-        return std::vector<uint32_t>();
-      pageArray.push_back(nPageNum);
+      return std::vector<uint32_t>();
     }
-    nStringFrom = nStringTo + 1;
   }
-  return pageArray;
+  return results;
 }

fpdfsdk/cpdfsdk_helpers_unittest.cpp

diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
index 27afd15..21b0061 100644
--- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -63,6 +63,7 @@
   EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("0,1", 10));
   EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,0", 10));
   EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2,,,,3-4", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2-", 10));
   EXPECT_EQ(std::vector<uint32_t>({1}), ParsePageRangeString("1-1", 10));
   EXPECT_EQ(std::vector<uint32_t>{1}, ParsePageRangeString("1", 1));
   EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4}),
@@ -71,7 +72,6 @@
             ParsePageRangeString("1- 4", 4));
   EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4}),
             ParsePageRangeString("1 -4", 4));
-  EXPECT_EQ(std::vector<uint32_t>({1, 2}), ParsePageRangeString("1-2-", 10));
   EXPECT_EQ(std::vector<uint32_t>({1, 2}), ParsePageRangeString("1,2", 10));
   EXPECT_EQ(std::vector<uint32_t>({2, 1}), ParsePageRangeString("2,1", 10));
   EXPECT_EQ(std::vector<uint32_t>({1, 50, 2}),