commit 3dd95ddc23b77d9e5bf2f3e258d8e7e717a49352
author Tom Sepez <tsepez@chromium.org> Wed Feb 12 21:32:15 2020 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Feb 12 21:32:15 2020 +0000
tree bf3bce31b0844ffcf8e84939694479c56fd066af
parent 0844b9a3b6259ff2af9e77425aa4bd3dcf71d861

Test uncovered comment handling paths in CSS parser.

Tests against current behaviour. At present, these may only be hit
by fuzzers, etc.

-- Add some empty string cases while at it.
-- Remove too-strong asserts hit by empty string tests.

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

core/fxcrt/css/cfx_cssstylesheet.cpp

diff --git a/core/fxcrt/css/cfx_cssstylesheet.cpp b/core/fxcrt/css/cfx_cssstylesheet.cpp
index c7300fb..df44530 100644
--- a/core/fxcrt/css/cfx_cssstylesheet.cpp
+++ b/core/fxcrt/css/cfx_cssstylesheet.cpp
@@ -36,7 +36,6 @@
 
 bool CFX_CSSStyleSheet::LoadBuffer(const wchar_t* pBuffer, int32_t iBufSize) {
   ASSERT(pBuffer);
-  ASSERT(iBufSize > 0);
 
   auto pSyntax = pdfium::MakeUnique<CFX_CSSSyntaxParser>(pBuffer, iBufSize);
   Reset();

core/fxcrt/css/cfx_cssstylesheet_unittest.cpp

diff --git a/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp b/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
index baa002b..5207ea2 100644
--- a/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
+++ b/core/fxcrt/css/cfx_cssstylesheet_unittest.cpp
@@ -27,14 +27,16 @@
 
   void TearDown() override { decl_ = nullptr; }
 
+  void LoadAndVerifyRuleCount(const wchar_t* buf, int rule_count) {
+    ASSERT(sheet_);
+    EXPECT_TRUE(sheet_->LoadBuffer(buf, wcslen(buf)));
+    EXPECT_EQ(sheet_->CountRules(), rule_count);
+  }
+
   void LoadAndVerifyDecl(const wchar_t* buf,
                          const std::vector<WideString>& selectors,
                          size_t decl_count) {
-    ASSERT(sheet_);
-
-    EXPECT_TRUE(sheet_->LoadBuffer(buf, wcslen(buf)));
-    EXPECT_EQ(sheet_->CountRules(), 1);
-
+    LoadAndVerifyRuleCount(buf, 1);
     CFX_CSSStyleRule* style = sheet_->GetRule(0);
     EXPECT_EQ(selectors.size(), style->CountSelectorLists());
 
@@ -86,6 +88,14 @@
   CFX_CSSDeclaration* decl_;
 };
 
+TEST_F(CFX_CSSStyleSheetTest, ParseEmpty) {
+  LoadAndVerifyRuleCount(L"", 0);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseBlankEmpty) {
+  LoadAndVerifyRuleCount(L"  \n\r\t", 0);
+}
+
 TEST_F(CFX_CSSStyleSheetTest, ParseMultipleSelectors) {
   const wchar_t* buf =
       L"a { border: 10px; }\nb { text-decoration: underline; }";
@@ -237,3 +247,34 @@
   VerifyFloat(CFX_CSSProperty::BorderBottomWidth, 2.5,
               CFX_CSSNumberType::Picas);
 }
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithCommentsInSelector) {
+  LoadAndVerifyDecl(L"/**{*/a/**}*/ { border-bottom: 2.5pc; }", {L"a"}, 1);
+  VerifyFloat(CFX_CSSProperty::BorderBottomWidth, 2.5,
+              CFX_CSSNumberType::Picas);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithCommentsInProperty) {
+  LoadAndVerifyDecl(L"a { /*}*/border-bottom: 2.5pc; }", {L"a"}, 1);
+  VerifyFloat(CFX_CSSProperty::BorderBottomWidth, 2.5,
+              CFX_CSSNumberType::Picas);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithCommentsInValue) {
+  LoadAndVerifyDecl(L"a { border-bottom: /*;*/2.5pc;/* color:red;*/ }", {L"a"},
+                    1);
+  VerifyFloat(CFX_CSSProperty::BorderBottomWidth, 2.5,
+              CFX_CSSNumberType::Picas);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithUnterminatedCommentInSelector) {
+  LoadAndVerifyRuleCount(L"a/* { border-bottom: 2.5pc; }", 0);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithUnterminatedCommentInProperty) {
+  LoadAndVerifyRuleCount(L"a { /*border-bottom: 2.5pc; }", 1);
+}
+
+TEST_F(CFX_CSSStyleSheetTest, ParseWithUnterminatedCommentInValue) {
+  LoadAndVerifyRuleCount(L"a { border-bottom: /*2.5pc; }", 1);
+}

core/fxcrt/css/cfx_csssyntaxparser.cpp

diff --git a/core/fxcrt/css/cfx_csssyntaxparser.cpp b/core/fxcrt/css/cfx_csssyntaxparser.cpp
index 523848c..b3f33d2 100644
--- a/core/fxcrt/css/cfx_csssyntaxparser.cpp
+++ b/core/fxcrt/css/cfx_csssyntaxparser.cpp
@@ -35,7 +35,6 @@
     : m_iTextDataLen(0),
       m_eStatus(CFX_CSSSyntaxStatus::None) {
   ASSERT(pBuffer);
-  ASSERT(iBufferSize > 0);
   ASSERT(iTextDatSize > 0);
 
   m_eMode = bOnlyDeclaration ? CFX_CSSSyntaxMode::PropertyName