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; }