From c89cd979fb8654d85b822909cba9263d5f13760c Mon Sep 17 00:00:00 2001 From: Wei Li Date: Tue, 15 Dec 2015 16:08:49 -0800 Subject: Fix free entry handling for cross reference v4 reading. While reading free entries in v4 cross reference table, changing the start_objnum caused crossref table built wrong. BUG=569795 R=jun_fang@foxitsoftware.com, thestig@chromium.org Review URL: https://codereview.chromium.org/1527823003 . --- core/include/fpdfapi/fpdf_parser.h | 5 +- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 34 +--- .../fpdf_parser/fpdf_parser_parser_unittest.cpp | 187 +++++++++++++++++++-- 3 files changed, 180 insertions(+), 46 deletions(-) diff --git a/core/include/fpdfapi/fpdf_parser.h b/core/include/fpdfapi/fpdf_parser.h index a266b20ba2..de0aa6196c 100644 --- a/core/include/fpdfapi/fpdf_parser.h +++ b/core/include/fpdfapi/fpdf_parser.h @@ -476,10 +476,7 @@ class CPDF_Parser { FX_BOOL LoadAllCrossRefV5(FX_FILESIZE pos); - bool LoadCrossRefV4(FX_FILESIZE pos, - FX_FILESIZE streampos, - FX_BOOL bSkip, - FX_BOOL bFirst); + bool LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, FX_BOOL bSkip); FX_BOOL LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef); diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index f18b733ab7..bdbb939148 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -344,7 +344,7 @@ FX_FILESIZE CPDF_Parser::GetObjectOffset(FX_DWORD objnum) { } FX_BOOL CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { - if (!LoadCrossRefV4(xrefpos, 0, TRUE, FALSE)) { + if (!LoadCrossRefV4(xrefpos, 0, TRUE)) { return FALSE; } m_pTrailer = LoadTrailerV4(); @@ -370,7 +370,7 @@ FX_BOOL CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { xrefpos = newxrefpos; while (xrefpos) { CrossRefList.InsertAt(0, xrefpos); - LoadCrossRefV4(xrefpos, 0, TRUE, FALSE); + LoadCrossRefV4(xrefpos, 0, TRUE); nonstd::unique_ptr> pDict( LoadTrailerV4()); if (!pDict) @@ -388,7 +388,7 @@ FX_BOOL CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { m_Trailers.Add(pDict.release()); } for (int32_t i = 0; i < CrossRefList.GetSize(); i++) { - if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE, i == 0)) + if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE)) return FALSE; } return TRUE; @@ -412,7 +412,7 @@ FX_BOOL CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, xrefpos = GetDirectInteger(m_pTrailer, "Prev"); while (xrefpos) { CrossRefList.InsertAt(0, xrefpos); - LoadCrossRefV4(xrefpos, 0, TRUE, FALSE); + LoadCrossRefV4(xrefpos, 0, TRUE); CPDF_Dictionary* pDict = LoadTrailerV4(); if (!pDict) { return FALSE; @@ -422,7 +422,7 @@ FX_BOOL CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, m_Trailers.Add(pDict); } for (int32_t i = 1; i < CrossRefList.GetSize(); i++) - if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE, i == 0)) { + if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE)) { return FALSE; } return TRUE; @@ -497,8 +497,7 @@ bool CPDF_Parser::FindPosInOffsets(FX_FILESIZE pos) const { bool CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, - FX_BOOL bSkip, - FX_BOOL bFirst) { + FX_BOOL bSkip) { m_Syntax.RestorePos(pos); if (m_Syntax.GetKeyword() != "xref") return false; @@ -527,17 +526,13 @@ bool CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos, FX_DWORD count = m_Syntax.GetDirectNum(); m_Syntax.ToNextWord(); SavedPos = m_Syntax.SavePos(); - FX_BOOL bFirstItem = FALSE; const int32_t recordsize = 20; - if (bFirst) - bFirstItem = TRUE; m_dwXrefStartObjNum = start_objnum; if (!bSkip) { std::vector buf(1024 * recordsize + 1); char* pBuf = pdfium::vector_as_array(&buf); pBuf[1024 * recordsize] = '\0'; int32_t nBlocks = count / 1024 + 1; - FX_BOOL bFirstBlock = TRUE; for (int32_t block = 0; block < nBlocks; block++) { int32_t block_size = block == nBlocks - 1 ? count % 1024 : 1024; m_Syntax.ReadBlock(reinterpret_cast(pBuf), @@ -546,18 +541,6 @@ bool CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos, FX_DWORD objnum = start_objnum + block * 1024 + i; char* pEntry = pBuf + i * recordsize; if (pEntry[17] == 'f') { - if (bFirstItem) { - objnum = 0; - bFirstItem = FALSE; - } - if (bFirstBlock) { - FX_FILESIZE offset = (FX_FILESIZE)FXSYS_atoi64(pEntry); - int32_t version = FXSYS_atoi(pEntry + 11); - if (offset == 0 && version == 65535 && start_objnum != 0) { - start_objnum--; - objnum = 0; - } - } m_CrossRef.SetAtGrow(objnum, 0); m_V5Type.SetAtGrow(objnum, 0); } else { @@ -580,9 +563,6 @@ bool CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos, } m_V5Type.SetAtGrow(objnum, 1); } - if (bFirstBlock) { - bFirstBlock = FALSE; - } } } } @@ -1574,7 +1554,7 @@ FX_DWORD CPDF_Parser::StartAsynParse(IFX_FileRead* pFileAccess, FX_FILESIZE dwFirstXRefOffset = m_Syntax.SavePos(); FX_BOOL bXRefRebuilt = FALSE; FX_BOOL bLoadV4 = FALSE; - if (!(bLoadV4 = LoadCrossRefV4(dwFirstXRefOffset, 0, FALSE, FALSE)) && + if (!(bLoadV4 = LoadCrossRefV4(dwFirstXRefOffset, 0, FALSE)) && !LoadCrossRefV5(&dwFirstXRefOffset, TRUE)) { if (!RebuildCrossRef()) { return PDFPARSE_ERROR_FORMAT; diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp index 640feac9aa..aba639c850 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp @@ -7,12 +7,65 @@ #include "testing/gtest/include/gtest/gtest.h" #include "testing/utils/path_service.h" +// Functions to help test an array's content against expected results. +template +bool CompareArray(const CFX_ArrayTemplate& array1, + const TYPE* array2, + size_t size) { + if (array1.GetSize() != size) + return false; + + for (int i = 0; i < size; ++i) + if (array1.GetAt(i) != array2[i]) + return false; + return true; +} +template bool CompareArray(const CFX_ByteArray&, + const uint8_t*, + size_t); +template bool CompareArray(const CFX_WordArray&, + const FX_WORD*, + size_t); +template bool CompareArray(const CFX_DWordArray&, + const FX_DWORD*, + size_t); +template bool CompareArray(const CFX_FileSizeArray&, + const FX_FILESIZE*, + size_t); + +// Provide a way to read test data from a buffer instead of a file. +class CFX_TestBufferRead : public IFX_FileRead { + public: + CFX_TestBufferRead(const unsigned char* buffer_in, size_t buf_size) + : buffer_(buffer_in), total_size_(buf_size) {} + + // IFX_Stream + void Release() override { delete this; } + + // IFX_FileRead + virtual FX_BOOL ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) { + if (offset < 0 || offset + size > total_size_) { + return FALSE; + } + + memcpy(buffer, buffer_ + offset, size); + return TRUE; + } + virtual FX_FILESIZE GetSize() { return (FX_FILESIZE)total_size_; }; + + protected: + const unsigned char* buffer_; + size_t total_size_; +}; + +// A wrapper class to help test member functions of CPDF_Parser. class CPDF_TestParser : public CPDF_Parser { public: CPDF_TestParser() {} ~CPDF_TestParser() {} - bool InitTest(const FX_CHAR* path) { + // Setup reading from a file and initial states. + bool InitTestFromFile(const FX_CHAR* path) { IFX_FileRead* pFileAccess = FX_CreateFileRead(path); if (!pFileAccess) return false; @@ -22,11 +75,23 @@ class CPDF_TestParser : public CPDF_Parser { return true; } + // Setup reading from a buffer and initial states. + bool InitTestFromBuffer(const unsigned char* buffer, size_t len) { + CFX_TestBufferRead* buffer_reader = new CFX_TestBufferRead(buffer, len); + + // For the test file, the header is set at the beginning. + m_Syntax.InitParser(buffer_reader, 0); + return true; + } + private: - // Add test case as private friend so that RebuildCrossRef in CPDF_Parser - // can be accessed. + // Add test cases here as private friend so that protected members in + // CPDF_Parser can be accessed by test cases. + // Need to access RebuildCrossRef. FRIEND_TEST(fpdf_parser_parser, RebuildCrossRefCorrectly); FRIEND_TEST(fpdf_parser_parser, RebuildCrossRefFailed); + // Need to access LoadCrossRefV4. + FRIEND_TEST(fpdf_parser_parser, LoadCrossRefV4); }; // TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal. @@ -197,20 +262,14 @@ TEST(fpdf_parser_parser, RebuildCrossRefCorrectly) { std::string test_file; ASSERT_TRUE(PathService::GetTestFilePath("parser_rebuildxref_correct.pdf", &test_file)); - ASSERT_TRUE(parser.InitTest(test_file.c_str())) << test_file; + ASSERT_TRUE(parser.InitTestFromFile(test_file.c_str())) << test_file; ASSERT_TRUE(parser.RebuildCrossRef()); const FX_FILESIZE offsets[] = {0, 15, 61, 154, 296, 374, 450}; const FX_WORD versions[] = {0, 0, 2, 4, 6, 8, 0}; - static_assert(FX_ArraySize(offsets) == FX_ArraySize(versions), - "numbers of offsets and versions should be same."); - ASSERT_EQ(FX_ArraySize(offsets), parser.m_CrossRef.GetSize()); - ASSERT_EQ(FX_ArraySize(versions), parser.m_ObjVersion.GetSize()); - - for (int i = 0; i < FX_ArraySize(offsets); ++i) { - EXPECT_EQ(offsets[i], parser.m_CrossRef.GetAt(i)); - EXPECT_EQ(versions[i], parser.m_ObjVersion.GetAt(i)); - } + ASSERT_TRUE(CompareArray(parser.m_CrossRef, offsets, FX_ArraySize(offsets))); + ASSERT_TRUE( + CompareArray(parser.m_ObjVersion, versions, FX_ArraySize(versions))); } TEST(fpdf_parser_parser, RebuildCrossRefFailed) { @@ -218,7 +277,105 @@ TEST(fpdf_parser_parser, RebuildCrossRefFailed) { std::string test_file; ASSERT_TRUE(PathService::GetTestFilePath( "parser_rebuildxref_error_notrailer.pdf", &test_file)); - ASSERT_TRUE(parser.InitTest(test_file.c_str())) << test_file; + ASSERT_TRUE(parser.InitTestFromFile(test_file.c_str())) << test_file; ASSERT_FALSE(parser.RebuildCrossRef()); -} \ No newline at end of file +} + +TEST(fpdf_parser_parser, LoadCrossRefV4) { + { + const unsigned char xref_table[] = + "xref \n" + "0 6 \n" + "0000000003 65535 f \n" + "0000000017 00000 n \n" + "0000000081 00000 n \n" + "0000000000 00007 f \n" + "0000000331 00000 n \n" + "0000000409 00000 n \n" + "trail"; // Needed to end cross ref table reading. + CPDF_TestParser parser; + ASSERT_TRUE( + parser.InitTestFromBuffer(xref_table, FX_ArraySize(xref_table))); + + ASSERT_TRUE(parser.LoadCrossRefV4(0, 0, FALSE)); + const FX_FILESIZE offsets[] = {0, 17, 81, 0, 331, 409}; + const uint8_t types[] = {0, 1, 1, 0, 1, 1}; + ASSERT_TRUE( + CompareArray(parser.m_CrossRef, offsets, FX_ArraySize(offsets))); + ASSERT_TRUE(CompareArray(parser.m_V5Type, types, FX_ArraySize(types))); + } + { + const unsigned char xref_table[] = + "xref \n" + "0 1 \n" + "0000000000 65535 f \n" + "3 1 \n" + "0000025325 00000 n \n" + "8 2 \n" + "0000025518 00002 n \n" + "0000025635 00000 n \n" + "12 1 \n" + "0000025777 00000 n \n" + "trail"; // Needed to end cross ref table reading. + CPDF_TestParser parser; + ASSERT_TRUE( + parser.InitTestFromBuffer(xref_table, FX_ArraySize(xref_table))); + + ASSERT_TRUE(parser.LoadCrossRefV4(0, 0, FALSE)); + const FX_FILESIZE offsets[] = {0, 0, 0, 25325, 0, 0, 0, + 0, 25518, 25635, 0, 0, 25777}; + const uint8_t types[] = {0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 0, 0, 1}; + ASSERT_TRUE( + CompareArray(parser.m_CrossRef, offsets, FX_ArraySize(offsets))); + ASSERT_TRUE(CompareArray(parser.m_V5Type, types, FX_ArraySize(types))); + } + { + const unsigned char xref_table[] = + "xref \n" + "0 1 \n" + "0000000000 65535 f \n" + "3 1 \n" + "0000025325 00000 n \n" + "8 2 \n" + "0000000000 65535 f \n" + "0000025635 00000 n \n" + "12 1 \n" + "0000025777 00000 n \n" + "trail"; // Needed to end cross ref table reading. + CPDF_TestParser parser; + ASSERT_TRUE( + parser.InitTestFromBuffer(xref_table, FX_ArraySize(xref_table))); + + ASSERT_TRUE(parser.LoadCrossRefV4(0, 0, FALSE)); + const FX_FILESIZE offsets[] = {0, 0, 0, 25325, 0, 0, 0, + 0, 0, 25635, 0, 0, 25777}; + const uint8_t types[] = {0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 1}; + ASSERT_TRUE( + CompareArray(parser.m_CrossRef, offsets, FX_ArraySize(offsets))); + ASSERT_TRUE(CompareArray(parser.m_V5Type, types, FX_ArraySize(types))); + } + { + const unsigned char xref_table[] = + "xref \n" + "0 7 \n" + "0000000002 65535 f \n" + "0000000023 00000 n \n" + "0000000003 65535 f \n" + "0000000004 65535 f \n" + "0000000000 65535 f \n" + "0000000045 00000 n \n" + "0000000179 00000 n \n" + "trail"; // Needed to end cross ref table reading. + CPDF_TestParser parser; + ASSERT_TRUE( + parser.InitTestFromBuffer(xref_table, FX_ArraySize(xref_table))); + + ASSERT_TRUE(parser.LoadCrossRefV4(0, 0, FALSE)); + const FX_FILESIZE offsets[] = {0, 23, 0, 0, 0, 45, 179}; + const uint8_t types[] = {0, 1, 0, 0, 0, 1, 1}; + ASSERT_TRUE( + CompareArray(parser.m_CrossRef, offsets, FX_ArraySize(offsets))); + ASSERT_TRUE(CompareArray(parser.m_V5Type, types, FX_ArraySize(types))); + } +} -- cgit v1.2.3