Move ParsePageRangeString to fpdfsdk_helpers.cpp.

Then add tests to existing unit tests file.

- Return array by value.
- Return empty array on error rather than bool
  (since only caller returns empty array on false).

Change-Id: I47cd714e809d4537d3b244cf3de99d3d80f541ab
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/79011
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index e629865..3711658 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -21,6 +21,7 @@
 #include "core/fxcrt/unowned_ptr.h"
 #include "fpdfsdk/cpdfsdk_formfillenvironment.h"
 #include "third_party/base/check.h"
+#include "third_party/base/numerics/safe_conversions.h"
 
 namespace {
 
@@ -450,3 +451,57 @@
   color_scheme.text_stroke_color = pColorScheme->text_stroke_color;
   pRenderOptions->SetColorScheme(color_scheme);
 }
+
+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]))
+      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>();
+
+      ++nMid;
+      size_t nEnd = cbMidRange.GetLength() - nMid;
+      if (nEnd == 0)
+        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)
+        return std::vector<uint32_t>();
+
+      for (uint32_t i = nStartPageNum; i <= nEndPageNum; ++i)
+        pageArray.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);
+    }
+    nStringFrom = nStringTo + 1;
+  }
+  return pageArray;
+}
diff --git a/fpdfsdk/cpdfsdk_helpers.h b/fpdfsdk/cpdfsdk_helpers.h
index 0895ce0..925c001 100644
--- a/fpdfsdk/cpdfsdk_helpers.h
+++ b/fpdfsdk/cpdfsdk_helpers.h
@@ -7,6 +7,8 @@
 #ifndef FPDFSDK_CPDFSDK_HELPERS_H_
 #define FPDFSDK_CPDFSDK_HELPERS_H_
 
+#include <vector>
+
 #include "build/build_config.h"
 #include "core/fpdfapi/page/cpdf_page.h"
 #include "core/fpdfapi/parser/cpdf_parser.h"
@@ -278,4 +280,7 @@
 void SetColorFromScheme(const FPDF_COLORSCHEME* pColorScheme,
                         CPDF_RenderOptions* pRenderOptions);
 
+std::vector<uint32_t> ParsePageRangeString(const ByteString& bsPageRange,
+                                           uint32_t nCount);
+
 #endif  // FPDFSDK_CPDFSDK_HELPERS_H_
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
index 6b65fa8..27afd15 100644
--- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp
+++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -38,3 +38,52 @@
     EXPECT_EQ(empty, ByteString(buf));
   }
 }
+
+TEST(CPDFSDK_HelpersTest, ParsePageRangeString) {
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("", 1));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(" ", 1));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams", 1));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("0", 0));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1", 0));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(",1", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1,clams", 1));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams,1", 1));
+  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-5", 4));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-11,", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString(",1-1", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-,", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("-2,", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-clams", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("clams-1,", 10));
+  EXPECT_EQ(std::vector<uint32_t>({}), ParsePageRangeString("1-2clams", 10));
+  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>({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}),
+            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, 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}),
+            ParsePageRangeString("1,50,2", 100));
+  EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 50}),
+            ParsePageRangeString("1-4,50", 100));
+  EXPECT_EQ(std::vector<uint32_t>({50, 1, 2}),
+            ParsePageRangeString("50,1-2", 100));
+  EXPECT_EQ(std::vector<uint32_t>({50, 1, 2}),
+            ParsePageRangeString("5  0, 1-2 ", 100));  // ???
+  EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 5, 6}),
+            ParsePageRangeString("1-3,4-6", 10));
+  EXPECT_EQ(std::vector<uint32_t>({1, 2, 3, 4, 3, 4, 5, 6}),
+            ParsePageRangeString("1-4,3-6", 10));
+}
diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp
index 5c478cb..5c0fa6f 100644
--- a/fpdfsdk/fpdf_ppo.cpp
+++ b/fpdfsdk/fpdf_ppo.cpp
@@ -202,72 +202,16 @@
   return true;
 }
 
-bool ParsePageRangeString(const ByteString& bsPageRange,
-                          uint32_t nCount,
-                          std::vector<uint32_t>* pageArray) {
-  ByteString bsStrippedPageRange = bsPageRange;
-  bsStrippedPageRange.Remove(' ');
-  size_t nLength = bsStrippedPageRange.GetLength();
-  if (nLength == 0)
-    return true;
-
-  static const ByteString cbCompareString("0123456789-,");
-  for (size_t i = 0; i < nLength; ++i) {
-    if (!cbCompareString.Contains(bsStrippedPageRange[i]))
-      return false;
-  }
-
-  ByteString cbMidRange;
-  size_t nStringFrom = 0;
-  size_t nStringTo = 0;
-  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 false;
-
-      ++nMid;
-      size_t nEnd = cbMidRange.GetLength() - nMid;
-      if (nEnd == 0)
-        return false;
-
-      uint32_t nEndPageNum = pdfium::base::checked_cast<uint32_t>(
-          atoi(cbMidRange.Substr(nMid, nEnd).c_str()));
-      if (nStartPageNum > nEndPageNum || nEndPageNum > nCount) {
-        return false;
-      }
-      for (uint32_t i = nStartPageNum; i <= nEndPageNum; ++i) {
-        pageArray->push_back(i);
-      }
-    } else {
-      uint32_t nPageNum =
-          pdfium::base::checked_cast<uint32_t>(atoi(cbMidRange.c_str()));
-      if (nPageNum <= 0 || nPageNum > nCount)
-        return false;
-      pageArray->push_back(nPageNum);
-    }
-    nStringFrom = nStringTo + 1;
-  }
-  return true;
-}
-
 std::vector<uint32_t> GetPageNumbers(const CPDF_Document& doc,
                                      const ByteString& bsPageRange) {
-  std::vector<uint32_t> page_numbers;
   uint32_t nCount = doc.GetPageCount();
-  if (bsPageRange.IsEmpty()) {
-    for (uint32_t i = 1; i <= nCount; ++i)
-      page_numbers.push_back(i);
-  } else {
-    if (!ParsePageRangeString(bsPageRange, nCount, &page_numbers))
-      page_numbers.clear();
-  }
+  if (!bsPageRange.IsEmpty())
+    return ParsePageRangeString(bsPageRange, nCount);
+
+  std::vector<uint32_t> page_numbers;
+  for (uint32_t i = 1; i <= nCount; ++i)
+    page_numbers.push_back(i);
+
   return page_numbers;
 }