Fix checks for 1D barcodes that do not accept letters. - Enable disabled tests for barcodes that do not accept ASCII. - Put unused CBC_OneDimWriter::CheckContentValidity() to use, to fix the tests. - Merge validity checks in various Encode() methods into CBC_OneDimWriter::HasValidContentSize(), and call it in CheckContentValidity() instead. - Fix some nits along the way. Bug: pdfium:1172 Change-Id: I60e0b99795178bac035202f4814792403f9e13e2 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/68970 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>

diff --git a/fxbarcode/cbc_codabar.cpp b/fxbarcode/cbc_codabar.cpp
index ad4d21c..70500f4 100644
--- a/fxbarcode/cbc_codabar.cpp
+++ b/fxbarcode/cbc_codabar.cpp

@@ -30,18 +30,18 @@
 CBC_Codabar::CBC_Codabar()
     : CBC_OneCode(pdfium::MakeUnique<CBC_OnedCodaBarWriter>()) {}
 
-CBC_Codabar::~CBC_Codabar() {}
+CBC_Codabar::~CBC_Codabar() = default;
 
 bool CBC_Codabar::Encode(WideStringView contents) {
-  if (contents.IsEmpty() || contents.GetLength() > kMaxInputLengthBytes)
+  auto* pWriter = GetOnedCodaBarWriter();
+  if (!pWriter->CheckContentValidity(contents))
     return false;
 
   BCFORMAT format = BCFORMAT_CODABAR;
   int32_t outWidth = 0;
   int32_t outHeight = 0;
-  m_renderContents = GetOnedCodaBarWriter()->FilterContents(contents);
+  m_renderContents = pWriter->FilterContents(contents);
   ByteString byteString = m_renderContents.ToUTF8();
-  auto* pWriter = GetOnedCodaBarWriter();
   std::unique_ptr<uint8_t, FxFreeDeleter> data(
       pWriter->Encode(byteString, format, outWidth, outHeight));
   return data && pWriter->RenderResult(m_renderContents.AsStringView(),

diff --git a/fxbarcode/cbc_code128.cpp b/fxbarcode/cbc_code128.cpp
index c363c94..4c872bc 100644
--- a/fxbarcode/cbc_code128.cpp
+++ b/fxbarcode/cbc_code128.cpp

@@ -30,16 +30,16 @@
 CBC_Code128::CBC_Code128(BC_TYPE type)
     : CBC_OneCode(pdfium::MakeUnique<CBC_OnedCode128Writer>(type)) {}
 
-CBC_Code128::~CBC_Code128() {}
+CBC_Code128::~CBC_Code128() = default;
 
 bool CBC_Code128::Encode(WideStringView contents) {
-  if (contents.IsEmpty() || contents.GetLength() > kMaxInputLengthBytes)
+  auto* pWriter = GetOnedCode128Writer();
+  if (!pWriter->CheckContentValidity(contents))
     return false;
 
   BCFORMAT format = BCFORMAT_CODE_128;
   int32_t outWidth = 0;
   int32_t outHeight = 0;
-  auto* pWriter = GetOnedCode128Writer();
   WideString content(contents);
   if (contents.GetLength() % 2 && pWriter->GetType() == BC_CODE128_C)
     content += '0';

diff --git a/fxbarcode/cbc_code39.cpp b/fxbarcode/cbc_code39.cpp
index 2138d18..f6600c4 100644
--- a/fxbarcode/cbc_code39.cpp
+++ b/fxbarcode/cbc_code39.cpp

@@ -30,16 +30,16 @@
 CBC_Code39::CBC_Code39()
     : CBC_OneCode(pdfium::MakeUnique<CBC_OnedCode39Writer>()) {}
 
-CBC_Code39::~CBC_Code39() {}
+CBC_Code39::~CBC_Code39() = default;
 
 bool CBC_Code39::Encode(WideStringView contents) {
-  if (contents.IsEmpty() || contents.GetLength() > kMaxInputLengthBytes)
+  auto* pWriter = GetOnedCode39Writer();
+  if (!pWriter->CheckContentValidity(contents))
     return false;
 
   BCFORMAT format = BCFORMAT_CODE_39;
   int32_t outWidth = 0;
   int32_t outHeight = 0;
-  auto* pWriter = GetOnedCode39Writer();
   WideString filtercontents = pWriter->FilterContents(contents);
   m_renderContents = pWriter->RenderTextContents(contents);
   ByteString byteString = filtercontents.ToUTF8();

diff --git a/fxbarcode/cbc_eancode.cpp b/fxbarcode/cbc_eancode.cpp
index 6e44f81..f3ec3f1 100644
--- a/fxbarcode/cbc_eancode.cpp
+++ b/fxbarcode/cbc_eancode.cpp

@@ -21,7 +21,8 @@
 }
 
 bool CBC_EANCode::Encode(WideStringView contents) {
-  if (contents.IsEmpty() || contents.GetLength() > kMaxInputLengthBytes)
+  auto* pWriter = GetOneDimEANWriter();
+  if (!pWriter->CheckContentValidity(contents))
     return false;
 
   BCFORMAT format = GetFormat();
@@ -29,7 +30,6 @@
   int32_t out_height = 0;
   m_renderContents = Preprocess(contents);
   ByteString str = m_renderContents.ToUTF8();
-  auto* pWriter = GetOneDimEANWriter();
   pWriter->InitEANWriter();
   std::unique_ptr<uint8_t, FxFreeDeleter> data(
       pWriter->Encode(str, format, out_width, out_height));

diff --git a/fxbarcode/cbc_onecode.h b/fxbarcode/cbc_onecode.h
index 7072386..7ccaafe 100644
--- a/fxbarcode/cbc_onecode.h
+++ b/fxbarcode/cbc_onecode.h

@@ -18,10 +18,6 @@
 
 class CBC_OneCode : public CBC_CodeBase {
  public:
-  // Limit the size of 1D barcodes. Typical 1D barcodes are short so this should
-  // be sufficient for most use cases.
-  static constexpr size_t kMaxInputLengthBytes = 8192;
-
   explicit CBC_OneCode(std::unique_ptr<CBC_Writer> pWriter);
   ~CBC_OneCode() override;
 

diff --git a/fxbarcode/cfx_barcode_unittest.cpp b/fxbarcode/cfx_barcode_unittest.cpp
index 3d4206a..571c6ca 100644
--- a/fxbarcode/cfx_barcode_unittest.cpp
+++ b/fxbarcode/cfx_barcode_unittest.cpp

@@ -94,7 +94,7 @@
   EXPECT_EQ("5fad4fc19f099001a0fe83c89430c977", BitmapChecksum());
 }
 
-TEST_F(BarcodeTest, DISABLED_CodaBarLetters) {
+TEST_F(BarcodeTest, CodaBarLetters) {
   Create(BC_CODABAR);
   EXPECT_FALSE(barcode()->Encode(L"clams"));
 }
@@ -164,7 +164,7 @@
   EXPECT_EQ("aff88491ac46ca6217d780d185300cde", BitmapChecksum());
 }
 
-TEST_F(BarcodeTest, DISABLED_Ean8Letters) {
+TEST_F(BarcodeTest, Ean8Letters) {
   Create(BC_EAN8);
   EXPECT_FALSE(barcode()->Encode(L"clams"));
 }
@@ -182,7 +182,7 @@
   EXPECT_EQ("fe26a5714cff7ffe3f9b02183efc435b", BitmapChecksum());
 }
 
-TEST_F(BarcodeTest, DISABLED_UPCALetters) {
+TEST_F(BarcodeTest, UPCALetters) {
   Create(BC_UPCA);
   EXPECT_FALSE(barcode()->Encode(L"clams"));
 }
@@ -200,7 +200,7 @@
   EXPECT_EQ("72d2190b98d635c32834bf67552e561e", BitmapChecksum());
 }
 
-TEST_F(BarcodeTest, DISABLED_Ean13Letters) {
+TEST_F(BarcodeTest, Ean13Letters) {
   Create(BC_EAN13);
   EXPECT_FALSE(barcode()->Encode(L"clams"));
 }

diff --git a/fxbarcode/oned/BC_OneDimWriter.cpp b/fxbarcode/oned/BC_OneDimWriter.cpp
index 5a02cdf..33d6ebf 100644
--- a/fxbarcode/oned/BC_OneDimWriter.cpp
+++ b/fxbarcode/oned/BC_OneDimWriter.cpp

@@ -36,6 +36,16 @@
 #include "core/fxge/text_char_pos.h"
 #include "fxbarcode/BC_Writer.h"
 
+// static
+bool CBC_OneDimWriter::HasValidContentSize(WideStringView contents) {
+  // Limit the size of 1D barcodes. Typical 1D barcodes are short so this should
+  // be sufficient for most use cases.
+  static constexpr size_t kMaxInputLengthBytes = 8192;
+
+  size_t size = contents.GetLength();
+  return size > 0 && size <= kMaxInputLengthBytes;
+}
+
 CBC_OneDimWriter::CBC_OneDimWriter() = default;
 
 CBC_OneDimWriter::~CBC_OneDimWriter() = default;

diff --git a/fxbarcode/oned/BC_OneDimWriter.h b/fxbarcode/oned/BC_OneDimWriter.h
index 3e0ccc9..338e34e 100644
--- a/fxbarcode/oned/BC_OneDimWriter.h
+++ b/fxbarcode/oned/BC_OneDimWriter.h

@@ -22,6 +22,8 @@
 
 class CBC_OneDimWriter : public CBC_Writer {
  public:
+  static bool HasValidContentSize(WideStringView contents);
+
   CBC_OneDimWriter();
   ~CBC_OneDimWriter() override;
 

diff --git a/fxbarcode/oned/BC_OnedCodaBarWriter.cpp b/fxbarcode/oned/BC_OnedCodaBarWriter.cpp
index eb330a9..d87125f 100644
--- a/fxbarcode/oned/BC_OnedCodaBarWriter.cpp
+++ b/fxbarcode/oned/BC_OnedCodaBarWriter.cpp

@@ -99,9 +99,10 @@
 }
 
 bool CBC_OnedCodaBarWriter::CheckContentValidity(WideStringView contents) {
-  return std::all_of(
-      contents.begin(), contents.end(),
-      [this](const wchar_t& ch) { return this->FindChar(ch, false); });
+  return HasValidContentSize(contents) &&
+         std::all_of(
+             contents.begin(), contents.end(),
+             [this](const wchar_t& ch) { return this->FindChar(ch, false); });
 }
 
 WideString CBC_OnedCodaBarWriter::FilterContents(WideStringView contents) {

diff --git a/fxbarcode/oned/BC_OnedCode128Writer.cpp b/fxbarcode/oned/BC_OnedCode128Writer.cpp
index b01abb5..fa51f4a 100644
--- a/fxbarcode/oned/BC_OnedCode128Writer.cpp
+++ b/fxbarcode/oned/BC_OnedCode128Writer.cpp

@@ -75,6 +75,11 @@
 const int32_t CODE_START_C = 105;
 const int32_t CODE_STOP = 106;
 
+bool IsInOnedCode128Alphabet(wchar_t ch) {
+  int32_t index = static_cast<int32_t>(ch);
+  return index >= 32 && index <= 126 && index != 34;
+}
+
 }  // namespace
 
 CBC_OnedCode128Writer::CBC_OnedCode128Writer(BC_TYPE type)
@@ -85,12 +90,8 @@
 CBC_OnedCode128Writer::~CBC_OnedCode128Writer() = default;
 
 bool CBC_OnedCode128Writer::CheckContentValidity(WideStringView contents) {
-  for (const auto& ch : contents) {
-    int32_t patternIndex = static_cast<int32_t>(ch);
-    if (patternIndex < 32 || patternIndex > 126 || patternIndex == 34)
-      return false;
-  }
-  return true;
+  return HasValidContentSize(contents) &&
+         std::all_of(contents.begin(), contents.end(), IsInOnedCode128Alphabet);
 }
 
 WideString CBC_OnedCode128Writer::FilterContents(WideStringView contents) {

diff --git a/fxbarcode/oned/BC_OnedCode128Writer_unittest.cpp b/fxbarcode/oned/BC_OnedCode128Writer_unittest.cpp
index 19d91f9..c85cdfd 100644
--- a/fxbarcode/oned/BC_OnedCode128Writer_unittest.cpp
+++ b/fxbarcode/oned/BC_OnedCode128Writer_unittest.cpp

@@ -84,9 +84,9 @@
 TEST(OnedCode128WriterTest, CheckContentValidity) {
   {
     CBC_OnedCode128Writer writer(BC_CODE128_B);
-    EXPECT_TRUE(writer.CheckContentValidity(L""));
     EXPECT_TRUE(writer.CheckContentValidity(L"foo"));
     EXPECT_TRUE(writer.CheckContentValidity(L"xyz"));
+    EXPECT_FALSE(writer.CheckContentValidity(L""));
     EXPECT_FALSE(writer.CheckContentValidity(L"\""));
     EXPECT_FALSE(writer.CheckContentValidity(L"f\x10oo"));
     EXPECT_FALSE(writer.CheckContentValidity(L"bar\x7F"));
@@ -94,9 +94,9 @@
   }
   {
     CBC_OnedCode128Writer writer(BC_CODE128_C);
-    EXPECT_TRUE(writer.CheckContentValidity(L""));
     EXPECT_TRUE(writer.CheckContentValidity(L"foo"));
     EXPECT_TRUE(writer.CheckContentValidity(L"xyz"));
+    EXPECT_FALSE(writer.CheckContentValidity(L""));
     EXPECT_FALSE(writer.CheckContentValidity(L"\""));
     EXPECT_FALSE(writer.CheckContentValidity(L"f\x10oo"));
     EXPECT_FALSE(writer.CheckContentValidity(L"bar\x7F"));

diff --git a/fxbarcode/oned/BC_OnedCode39Writer.cpp b/fxbarcode/oned/BC_OnedCode39Writer.cpp
index 97ee881..42c031a 100644
--- a/fxbarcode/oned/BC_OnedCode39Writer.cpp
+++ b/fxbarcode/oned/BC_OnedCode39Writer.cpp

@@ -66,7 +66,8 @@
 CBC_OnedCode39Writer::~CBC_OnedCode39Writer() = default;
 
 bool CBC_OnedCode39Writer::CheckContentValidity(WideStringView contents) {
-  return std::all_of(contents.begin(), contents.end(), IsInOnedCode39Alphabet);
+  return HasValidContentSize(contents) &&
+         std::all_of(contents.begin(), contents.end(), IsInOnedCode39Alphabet);
 }
 
 WideString CBC_OnedCode39Writer::FilterContents(WideStringView contents) {

diff --git a/fxbarcode/oned/BC_OnedEAN13Writer.cpp b/fxbarcode/oned/BC_OnedEAN13Writer.cpp
index f2e88dd..871efa8 100644
--- a/fxbarcode/oned/BC_OnedEAN13Writer.cpp
+++ b/fxbarcode/oned/BC_OnedEAN13Writer.cpp

@@ -56,10 +56,11 @@
   m_bLeftPadding = true;
   m_codeWidth = 3 + (7 * 6) + 5 + (7 * 6) + 3;
 }
-CBC_OnedEAN13Writer::~CBC_OnedEAN13Writer() {}
+CBC_OnedEAN13Writer::~CBC_OnedEAN13Writer() = default;
 
 bool CBC_OnedEAN13Writer::CheckContentValidity(WideStringView contents) {
-  return std::all_of(contents.begin(), contents.end(),
+  return HasValidContentSize(contents) &&
+         std::all_of(contents.begin(), contents.end(),
                      [](wchar_t c) { return FXSYS_IsDecimalDigit(c); });
 }
 

diff --git a/fxbarcode/oned/BC_OnedEAN8Writer.cpp b/fxbarcode/oned/BC_OnedEAN8Writer.cpp
index 62c760c..1825491 100644
--- a/fxbarcode/oned/BC_OnedEAN8Writer.cpp
+++ b/fxbarcode/oned/BC_OnedEAN8Writer.cpp

@@ -65,7 +65,8 @@
 }
 
 bool CBC_OnedEAN8Writer::CheckContentValidity(WideStringView contents) {
-  return std::all_of(contents.begin(), contents.end(),
+  return HasValidContentSize(contents) &&
+         std::all_of(contents.begin(), contents.end(),
                      [](wchar_t c) { return FXSYS_IsDecimalDigit(c); });
 }
 

diff --git a/fxbarcode/oned/BC_OnedUPCAWriter.cpp b/fxbarcode/oned/BC_OnedUPCAWriter.cpp
index 666b037..cf6a4dc 100644
--- a/fxbarcode/oned/BC_OnedUPCAWriter.cpp
+++ b/fxbarcode/oned/BC_OnedUPCAWriter.cpp

@@ -22,6 +22,7 @@
 
 #include "fxbarcode/oned/BC_OnedUPCAWriter.h"
 
+#include <algorithm>
 #include <vector>
 
 #include "core/fxcrt/fx_extension.h"
@@ -37,14 +38,12 @@
   m_bRightPadding = true;
 }
 
-CBC_OnedUPCAWriter::~CBC_OnedUPCAWriter() {}
+CBC_OnedUPCAWriter::~CBC_OnedUPCAWriter() = default;
 
 bool CBC_OnedUPCAWriter::CheckContentValidity(WideStringView contents) {
-  for (size_t i = 0; i < contents.GetLength(); ++i) {
-    if (contents[i] < '0' || contents[i] > '9')
-      return false;
-  }
-  return true;
+  return HasValidContentSize(contents) &&
+         std::all_of(contents.begin(), contents.end(),
+                     [](wchar_t c) { return FXSYS_IsDecimalDigit(c); });
 }
 
 WideString CBC_OnedUPCAWriter::FilterContents(WideStringView contents) {