Change CPDF_CMapParser::GetCodeRange() to return Optional.
Also make the method private/static and friend its unit test instead.
Also make GetCode() private/static and friend its unit test instead.
Change-Id: Ic3e020f826e0bd711e3b5b7cd317f57784c83900
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/59510
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/font/cpdf_cmapparser.cpp b/core/fpdfapi/font/cpdf_cmapparser.cpp
index 1f7c0bc..56efae7 100644
--- a/core/fpdfapi/font/cpdf_cmapparser.cpp
+++ b/core/fpdfapi/font/cpdf_cmapparser.cpp
@@ -116,9 +116,10 @@
return;
if (m_CodeSeq % 2) {
- CPDF_CMap::CodeRange range;
- if (GetCodeRange(range, m_LastWord.AsStringView(), word))
- m_PendingRanges.push_back(range);
+ Optional<CPDF_CMap::CodeRange> range =
+ GetCodeRange(m_LastWord.AsStringView(), word);
+ if (range.has_value())
+ m_PendingRanges.push_back(range.value());
}
m_CodeSeq++;
}
@@ -126,7 +127,8 @@
m_LastWord = word;
}
-uint32_t CPDF_CMapParser::GetCode(ByteStringView word) const {
+// static
+uint32_t CPDF_CMapParser::GetCode(ByteStringView word) {
if (word.IsEmpty())
return 0;
@@ -148,21 +150,24 @@
return num.ValueOrDie();
}
-bool CPDF_CMapParser::GetCodeRange(CPDF_CMap::CodeRange& range,
- ByteStringView first,
- ByteStringView second) const {
+// static
+Optional<CPDF_CMap::CodeRange> CPDF_CMapParser::GetCodeRange(
+ ByteStringView first,
+ ByteStringView second) {
if (first.GetLength() == 0 || first[0] != '<')
- return false;
+ return pdfium::nullopt;
size_t i;
for (i = 1; i < first.GetLength(); ++i) {
if (first[i] == '>')
break;
}
- range.m_CharSize = (i - 1) / 2;
- if (range.m_CharSize > 4)
- return false;
+ size_t char_size = (i - 1) / 2;
+ if (char_size > 4)
+ return pdfium::nullopt;
+ CPDF_CMap::CodeRange range;
+ range.m_CharSize = char_size;
for (i = 0; i < range.m_CharSize; ++i) {
uint8_t digit1 = first[i * 2 + 1];
uint8_t digit2 = first[i * 2 + 2];
@@ -179,7 +184,7 @@
range.m_Upper[i] =
FXSYS_HexCharToInt(digit1) * 16 + FXSYS_HexCharToInt(digit2);
}
- return true;
+ return range;
}
// static
diff --git a/core/fpdfapi/font/cpdf_cmapparser.h b/core/fpdfapi/font/cpdf_cmapparser.h
index 5c8db40..e8ea29b 100644
--- a/core/fpdfapi/font/cpdf_cmapparser.h
+++ b/core/fpdfapi/font/cpdf_cmapparser.h
@@ -13,6 +13,7 @@
#include "core/fpdfapi/font/cpdf_cidfont.h"
#include "core/fpdfapi/font/cpdf_cmap.h"
#include "core/fxcrt/unowned_ptr.h"
+#include "third_party/base/optional.h"
class CPDF_CMapParser {
public:
@@ -27,14 +28,12 @@
return std::move(m_AdditionalCharcodeToCIDMappings);
}
- uint32_t GetCode(ByteStringView word) const;
- bool GetCodeRange(CPDF_CMap::CodeRange& range,
- ByteStringView first,
- ByteStringView second) const;
-
static CIDSet CharsetFromOrdering(ByteStringView ordering);
private:
+ friend class cpdf_cmapparser_GetCode_Test;
+ friend class cpdf_cmapparser_GetCodeRange_Test;
+
enum Status {
kStart,
kProcessingCidChar,
@@ -46,6 +45,10 @@
kProcessingCodeSpaceRange,
};
+ static uint32_t GetCode(ByteStringView word);
+ static Optional<CPDF_CMap::CodeRange> GetCodeRange(ByteStringView first,
+ ByteStringView second);
+
Status m_Status = kStart;
int m_CodeSeq = 0;
UnownedPtr<CPDF_CMap> const m_pCMap;
diff --git a/core/fpdfapi/font/cpdf_cmapparser_unittest.cpp b/core/fpdfapi/font/cpdf_cmapparser_unittest.cpp
index 1a36eb7..da654f3 100644
--- a/core/fpdfapi/font/cpdf_cmapparser_unittest.cpp
+++ b/core/fpdfapi/font/cpdf_cmapparser_unittest.cpp
@@ -8,7 +8,7 @@
namespace {
-bool uint_ranges_equal(uint8_t* a, uint8_t* b, size_t count) {
+bool uint_ranges_equal(const uint8_t* a, const uint8_t* b, size_t count) {
for (size_t i = 0; i < count; ++i) {
if (a[i] != b[i])
return false;
@@ -19,55 +19,57 @@
} // namespace
TEST(cpdf_cmapparser, GetCode) {
- CPDF_CMapParser parser(nullptr);
+ EXPECT_EQ(0u, CPDF_CMapParser::GetCode(""));
+ EXPECT_EQ(0u, CPDF_CMapParser::GetCode("<"));
+ EXPECT_EQ(194u, CPDF_CMapParser::GetCode("<c2"));
+ EXPECT_EQ(162u, CPDF_CMapParser::GetCode("<A2"));
+ EXPECT_EQ(2802u, CPDF_CMapParser::GetCode("<Af2"));
+ EXPECT_EQ(162u, CPDF_CMapParser::GetCode("<A2z"));
- EXPECT_EQ(0u, parser.GetCode(""));
- EXPECT_EQ(0u, parser.GetCode("<"));
- EXPECT_EQ(194u, parser.GetCode("<c2"));
- EXPECT_EQ(162u, parser.GetCode("<A2"));
- EXPECT_EQ(2802u, parser.GetCode("<Af2"));
- EXPECT_EQ(162u, parser.GetCode("<A2z"));
+ EXPECT_EQ(12u, CPDF_CMapParser::GetCode("12"));
+ EXPECT_EQ(12u, CPDF_CMapParser::GetCode("12d"));
+ EXPECT_EQ(128u, CPDF_CMapParser::GetCode("128"));
- EXPECT_EQ(12u, parser.GetCode("12"));
- EXPECT_EQ(12u, parser.GetCode("12d"));
- EXPECT_EQ(128u, parser.GetCode("128"));
-
- EXPECT_EQ(4294967295u, parser.GetCode("<FFFFFFFF"));
+ EXPECT_EQ(4294967295u, CPDF_CMapParser::GetCode("<FFFFFFFF"));
// Overflow a uint32_t.
- EXPECT_EQ(0u, parser.GetCode("<100000000"));
+ EXPECT_EQ(0u, CPDF_CMapParser::GetCode("<100000000"));
}
TEST(cpdf_cmapparser, GetCodeRange) {
- CPDF_CMapParser parser(nullptr);
- CPDF_CMap::CodeRange range;
+ Optional<CPDF_CMap::CodeRange> range;
// Must start with a <
- EXPECT_FALSE(parser.GetCodeRange(range, "", ""));
- EXPECT_FALSE(parser.GetCodeRange(range, "A", ""));
+ range = CPDF_CMapParser::GetCodeRange("", "");
+ EXPECT_FALSE(range.has_value());
+ range = CPDF_CMapParser::GetCodeRange("A", "");
+ EXPECT_FALSE(range.has_value());
// m_CharSize must be <= 4
- EXPECT_FALSE(parser.GetCodeRange(range, "<aaaaaaaaaa>", ""));
- EXPECT_EQ(5u, range.m_CharSize);
+ range = CPDF_CMapParser::GetCodeRange("<aaaaaaaaaa>", "");
+ EXPECT_FALSE(range.has_value());
- EXPECT_TRUE(parser.GetCodeRange(range, "<12345678>", "<87654321>"));
- EXPECT_EQ(4u, range.m_CharSize);
+ range = CPDF_CMapParser::GetCodeRange("<12345678>", "<87654321>");
+ ASSERT_TRUE(range.has_value());
+ ASSERT_EQ(4u, range.value().m_CharSize);
{
- uint8_t lower[4] = {18, 52, 86, 120};
- uint8_t upper[4] = {135, 101, 67, 33};
- EXPECT_TRUE(uint_ranges_equal(lower, range.m_Lower, range.m_CharSize));
- EXPECT_TRUE(uint_ranges_equal(upper, range.m_Upper, range.m_CharSize));
+ constexpr uint8_t kLower[4] = {18, 52, 86, 120};
+ constexpr uint8_t kUpper[4] = {135, 101, 67, 33};
+ EXPECT_TRUE(uint_ranges_equal(kLower, range.value().m_Lower, 4));
+ EXPECT_TRUE(uint_ranges_equal(kUpper, range.value().m_Upper, 4));
}
// Hex characters
- EXPECT_TRUE(parser.GetCodeRange(range, "<a1>", "<F3>"));
- EXPECT_EQ(1u, range.m_CharSize);
- EXPECT_EQ(161, range.m_Lower[0]);
- EXPECT_EQ(243, range.m_Upper[0]);
+ range = CPDF_CMapParser::GetCodeRange("<a1>", "<F3>");
+ ASSERT_TRUE(range.has_value());
+ ASSERT_EQ(1u, range.value().m_CharSize);
+ EXPECT_EQ(161, range.value().m_Lower[0]);
+ EXPECT_EQ(243, range.value().m_Upper[0]);
// The second string should return 0's if it is shorter
- EXPECT_TRUE(parser.GetCodeRange(range, "<a1>", ""));
- EXPECT_EQ(1u, range.m_CharSize);
- EXPECT_EQ(161, range.m_Lower[0]);
- EXPECT_EQ(0, range.m_Upper[0]);
+ range = CPDF_CMapParser::GetCodeRange("<a1>", "");
+ ASSERT_TRUE(range.has_value());
+ ASSERT_EQ(1u, range.value().m_CharSize);
+ EXPECT_EQ(161, range.value().m_Lower[0]);
+ EXPECT_EQ(0, range.value().m_Upper[0]);
}
