Prevent circular includes between fxcodec and fpdfapi/parser.
Hoist streams ever upwards in JBIG2 to remove the dependency. There
is one (set) of retain ptrs removed, but the callers already are
retaining the streams in their own class.
In turn, testing becomes simpler because we don't have to create
phony proxy objects that didn't belong down at this layer in the
first place.
- Rename one member while were at it to more accurately reflect type.
- Remove one test that is no longer relevant since spans don't make
an empty/null distinction.
Change-Id: I41353a19205e5a32e8428b1d30852d0839996231
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/55332
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/BUILD.gn b/core/fpdfapi/parser/BUILD.gn
index 2e3cebf..6bfac18 100644
--- a/core/fpdfapi/parser/BUILD.gn
+++ b/core/fpdfapi/parser/BUILD.gn
@@ -83,7 +83,7 @@
"../../fxcrt",
"../../fxge",
]
- allow_circular_includes_from = [ "../../fxcodec" ]
+ allow_circular_includes_from = []
visibility = [ "../../../*" ]
if (pdf_use_skia || pdf_use_skia_paths) {
diff --git a/core/fpdfapi/render/cpdf_dibbase.cpp b/core/fpdfapi/render/cpdf_dibbase.cpp
index dc5d1e6..e82ce3c 100644
--- a/core/fpdfapi/render/cpdf_dibbase.cpp
+++ b/core/fpdfapi/render/cpdf_dibbase.cpp
@@ -290,14 +290,28 @@
const CPDF_Stream* pGlobals =
m_pStreamAcc->GetImageParam()->GetStreamFor("JBIG2Globals");
if (pGlobals) {
- m_pGlobalStream = pdfium::MakeRetain<CPDF_StreamAcc>(pGlobals);
- m_pGlobalStream->LoadAllDataFiltered();
+ m_pGlobalAcc = pdfium::MakeRetain<CPDF_StreamAcc>(pGlobals);
+ m_pGlobalAcc->LoadAllDataFiltered();
}
}
+ uint32_t nSrcObjNum = 0;
+ pdfium::span<const uint8_t> pSrcSpan;
+ if (m_pStreamAcc) {
+ pSrcSpan = m_pStreamAcc->GetSpan();
+ if (m_pStreamAcc->GetStream())
+ nSrcObjNum = m_pStreamAcc->GetStream()->GetObjNum();
+ }
+ uint32_t nGlobalObjNum = 0;
+ pdfium::span<const uint8_t> pGlobalSpan;
+ if (m_pGlobalAcc) {
+ pGlobalSpan = m_pGlobalAcc->GetSpan();
+ if (m_pGlobalAcc->GetStream())
+ nGlobalObjNum = m_pGlobalAcc->GetStream()->GetObjNum();
+ }
iDecodeStatus = pJbig2Module->StartDecode(
m_pJbig2Context.get(), m_pDocument->CodecContext(), m_Width, m_Height,
- m_pStreamAcc, m_pGlobalStream, m_pCachedBitmap->GetBuffer(),
- m_pCachedBitmap->GetPitch(), pPause);
+ pSrcSpan, nSrcObjNum, pGlobalSpan, nGlobalObjNum,
+ m_pCachedBitmap->GetBuffer(), m_pCachedBitmap->GetPitch(), pPause);
} else {
iDecodeStatus = pJbig2Module->ContinueDecode(m_pJbig2Context.get(), pPause);
}
@@ -305,7 +319,7 @@
if (iDecodeStatus < 0) {
m_pJbig2Context.reset();
m_pCachedBitmap.Reset();
- m_pGlobalStream.Reset();
+ m_pGlobalAcc.Reset();
return LoadState::kFail;
}
if (iDecodeStatus == FXCODEC_STATUS_DECODE_TOBECONTINUE)
diff --git a/core/fpdfapi/render/cpdf_dibbase.h b/core/fpdfapi/render/cpdf_dibbase.h
index df9e41f..632bddd 100644
--- a/core/fpdfapi/render/cpdf_dibbase.h
+++ b/core/fpdfapi/render/cpdf_dibbase.h
@@ -148,7 +148,7 @@
std::unique_ptr<uint8_t, FxFreeDeleter> m_pMaskedLine;
RetainPtr<CFX_DIBitmap> m_pCachedBitmap;
RetainPtr<CPDF_DIBBase> m_pMask;
- RetainPtr<CPDF_StreamAcc> m_pGlobalStream;
+ RetainPtr<CPDF_StreamAcc> m_pGlobalAcc;
std::unique_ptr<CCodec_ScanlineDecoder> m_pDecoder;
// Must come after |m_pCachedBitmap|.
diff --git a/core/fxcodec/codec/ccodec_jbig2module.cpp b/core/fxcodec/codec/ccodec_jbig2module.cpp
index 16fd0e3..43b3ab0 100644
--- a/core/fxcodec/codec/ccodec_jbig2module.cpp
+++ b/core/fxcodec/codec/ccodec_jbig2module.cpp
@@ -9,7 +9,6 @@
#include <list>
#include <memory>
-#include "core/fpdfapi/parser/cpdf_stream_acc.h"
#include "core/fxcodec/JBig2_DocumentContext.h"
#include "core/fxcodec/jbig2/JBig2_Context.h"
#include "core/fxcodec/jbig2/JBig2_Image.h"
@@ -26,25 +25,23 @@
return pContextHolder->get();
}
-CCodec_Jbig2Context::CCodec_Jbig2Context()
- : m_width(0),
- m_height(0),
- m_pGlobalStream(nullptr),
- m_pSrcStream(nullptr),
- m_dest_buf(0),
- m_dest_pitch(0) {}
+CCodec_Jbig2Context::CCodec_Jbig2Context() = default;
-CCodec_Jbig2Context::~CCodec_Jbig2Context() {}
+CCodec_Jbig2Context::~CCodec_Jbig2Context() = default;
-CCodec_Jbig2Module::~CCodec_Jbig2Module() {}
+CCodec_Jbig2Module::CCodec_Jbig2Module() = default;
+
+CCodec_Jbig2Module::~CCodec_Jbig2Module() = default;
FXCODEC_STATUS CCodec_Jbig2Module::StartDecode(
CCodec_Jbig2Context* pJbig2Context,
std::unique_ptr<JBig2_DocumentContext>* pContextHolder,
uint32_t width,
uint32_t height,
- const RetainPtr<CPDF_StreamAcc>& src_stream,
- const RetainPtr<CPDF_StreamAcc>& global_stream,
+ pdfium::span<const uint8_t> src_span,
+ uint32_t src_objnum,
+ pdfium::span<const uint8_t> global_span,
+ uint32_t global_objnum,
uint8_t* dest_buf,
uint32_t dest_pitch,
PauseIndicatorIface* pPause) {
@@ -55,13 +52,16 @@
GetJBig2DocumentContext(pContextHolder);
pJbig2Context->m_width = width;
pJbig2Context->m_height = height;
- pJbig2Context->m_pSrcStream = src_stream;
- pJbig2Context->m_pGlobalStream = global_stream;
+ pJbig2Context->m_pSrcSpan = src_span;
+ pJbig2Context->m_nSrcObjNum = src_objnum;
+ pJbig2Context->m_pGlobalSpan = global_span;
+ pJbig2Context->m_nGlobalObjNum = global_objnum;
pJbig2Context->m_dest_buf = dest_buf;
pJbig2Context->m_dest_pitch = dest_pitch;
memset(dest_buf, 0, height * dest_pitch);
- pJbig2Context->m_pContext = CJBig2_Context::Create(
- global_stream, src_stream, pJBig2DocumentContext->GetSymbolDictCache());
+ pJbig2Context->m_pContext =
+ CJBig2_Context::Create(global_span, global_objnum, src_span, src_objnum,
+ pJBig2DocumentContext->GetSymbolDictCache());
bool succeeded = pJbig2Context->m_pContext->GetFirstPage(
dest_buf, width, height, dest_pitch, pPause);
return Decode(pJbig2Context, succeeded);
diff --git a/core/fxcodec/codec/ccodec_jbig2module.h b/core/fxcodec/codec/ccodec_jbig2module.h
index 534c0a3..2c370ce 100644
--- a/core/fxcodec/codec/ccodec_jbig2module.h
+++ b/core/fxcodec/codec/ccodec_jbig2module.h
@@ -10,11 +10,10 @@
#include <memory>
#include "core/fxcodec/fx_codec_def.h"
-#include "core/fxcrt/retain_ptr.h"
+#include "third_party/base/span.h"
class CJBig2_Context;
class CJBig2_Image;
-class CPDF_StreamAcc;
class PauseIndicatorIface;
class JBig2_DocumentContext;
@@ -23,18 +22,20 @@
CCodec_Jbig2Context();
~CCodec_Jbig2Context();
- uint32_t m_width;
- uint32_t m_height;
- RetainPtr<CPDF_StreamAcc> m_pGlobalStream;
- RetainPtr<CPDF_StreamAcc> m_pSrcStream;
- uint8_t* m_dest_buf;
- uint32_t m_dest_pitch;
+ uint32_t m_width = 0;
+ uint32_t m_height = 0;
+ uint32_t m_nGlobalObjNum = 0;
+ uint32_t m_nSrcObjNum = 0;
+ pdfium::span<const uint8_t> m_pGlobalSpan;
+ pdfium::span<const uint8_t> m_pSrcSpan;
+ uint8_t* m_dest_buf = nullptr;
+ uint32_t m_dest_pitch = 0;
std::unique_ptr<CJBig2_Context> m_pContext;
};
class CCodec_Jbig2Module {
public:
- CCodec_Jbig2Module() {}
+ CCodec_Jbig2Module();
~CCodec_Jbig2Module();
FXCODEC_STATUS StartDecode(
@@ -42,11 +43,14 @@
std::unique_ptr<JBig2_DocumentContext>* pContextHolder,
uint32_t width,
uint32_t height,
- const RetainPtr<CPDF_StreamAcc>& src_stream,
- const RetainPtr<CPDF_StreamAcc>& global_stream,
+ pdfium::span<const uint8_t> src_span,
+ uint32_t src_objnum,
+ pdfium::span<const uint8_t> global_span,
+ uint32_t global_objnum,
uint8_t* dest_buf,
uint32_t dest_pitch,
PauseIndicatorIface* pPause);
+
FXCODEC_STATUS ContinueDecode(CCodec_Jbig2Context* pJbig2Context,
PauseIndicatorIface* pPause);
diff --git a/core/fxcodec/jbig2/JBig2_BitStream_unittest.cpp b/core/fxcodec/jbig2/JBig2_BitStream_unittest.cpp
index 0e9ead5..8669489 100644
--- a/core/fxcodec/jbig2/JBig2_BitStream_unittest.cpp
+++ b/core/fxcodec/jbig2/JBig2_BitStream_unittest.cpp
@@ -7,21 +7,12 @@
#include <memory>
#include <utility>
-#include "core/fpdfapi/parser/cpdf_dictionary.h"
-#include "core/fpdfapi/parser/cpdf_stream.h"
-#include "core/fpdfapi/parser/cpdf_stream_acc.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/base/ptr_util.h"
TEST(JBig2_BitStream, ReadNBits) {
- std::unique_ptr<uint8_t, FxFreeDeleter> data(FX_Alloc(uint8_t, 1));
- data.get()[0] = 0xb1; // 10110001
-
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(std::move(data), 1, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
+ const uint8_t kData[] = {0xb1}; // 10110001
+ CJBig2_BitStream stream(kData, 0);
uint32_t val1;
EXPECT_EQ(0, stream.readNBits(1, &val1));
@@ -39,14 +30,8 @@
}
TEST(JBig2_BitStream, ReadNBitsLargerThenData) {
- std::unique_ptr<uint8_t, FxFreeDeleter> data(FX_Alloc(uint8_t, 1));
- data.get()[0] = 0xb1;
-
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(std::move(data), 1, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
+ const uint8_t kData[] = {0xb1}; // 10110001
+ CJBig2_BitStream stream(kData, 42);
uint32_t val1;
EXPECT_EQ(0, stream.readNBits(10, &val1));
@@ -54,27 +39,7 @@
}
TEST(JBig2_BitStream, ReadNBitsNullStream) {
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(nullptr, 0, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
-
- uint32_t val1;
- EXPECT_EQ(-1, stream.readNBits(1, &val1));
-
- int32_t val2;
- EXPECT_EQ(-1, stream.readNBits(2, &val2));
-}
-
-TEST(JBig2_BitStream, ReadNBitsEmptyStream) {
- std::unique_ptr<uint8_t, FxFreeDeleter> data(FX_Alloc(uint8_t, 1));
-
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(std::move(data), 0, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
+ CJBig2_BitStream stream({}, 0);
uint32_t val1;
EXPECT_EQ(-1, stream.readNBits(1, &val1));
@@ -84,14 +49,8 @@
}
TEST(JBig2_BitStream, ReadNBitsOutOfBounds) {
- std::unique_ptr<uint8_t, FxFreeDeleter> data(FX_Alloc(uint8_t, 1));
- data.get()[0] = 0xb1; // 10110001
-
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(std::move(data), 1, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
+ const uint8_t kData[] = {0xb1}; // 10110001
+ CJBig2_BitStream stream(kData, 42);
uint32_t val1;
EXPECT_EQ(0, stream.readNBits(8, &val1));
@@ -101,18 +60,8 @@
}
TEST(JBig2_BitStream, ReadNBitsWhereNIs36) {
- std::unique_ptr<uint8_t, FxFreeDeleter> data(FX_Alloc(uint8_t, 5));
- data.get()[0] = 0xb0;
- data.get()[1] = 0x01;
- data.get()[2] = 0x00;
- data.get()[3] = 0x00;
- data.get()[4] = 0x40;
-
- auto in_stream = pdfium::MakeRetain<CPDF_Stream>(std::move(data), 5, nullptr);
- auto acc = pdfium::MakeRetain<CPDF_StreamAcc>(in_stream.Get());
- acc->LoadAllDataFiltered();
-
- CJBig2_BitStream stream(acc->GetSpan(), in_stream->GetObjNum());
+ const uint8_t kData[] = {0xb0, 0x01, 0x00, 0x00, 0x40};
+ CJBig2_BitStream stream(kData, 42);
// This will shift off the top two bits and they end up lost.
uint32_t val1;
diff --git a/core/fxcodec/jbig2/JBig2_Context.cpp b/core/fxcodec/jbig2/JBig2_Context.cpp
index 2a0c09d..1dcaeb4 100644
--- a/core/fxcodec/jbig2/JBig2_Context.cpp
+++ b/core/fxcodec/jbig2/JBig2_Context.cpp
@@ -14,8 +14,6 @@
#include <utility>
#include <vector>
-#include "core/fpdfapi/parser/cpdf_stream.h"
-#include "core/fpdfapi/parser/cpdf_stream_acc.h"
#include "core/fxcodec/jbig2/JBig2_ArithDecoder.h"
#include "core/fxcodec/jbig2/JBig2_BitStream.h"
#include "core/fxcodec/jbig2/JBig2_GrdProc.h"
@@ -53,24 +51,25 @@
// static
std::unique_ptr<CJBig2_Context> CJBig2_Context::Create(
- const RetainPtr<CPDF_StreamAcc>& pGlobalStream,
- const RetainPtr<CPDF_StreamAcc>& pSrcStream,
+ pdfium::span<const uint8_t> pGlobalSpan,
+ uint32_t dwGlobalObjNum,
+ pdfium::span<const uint8_t> pSrcSpan,
+ uint32_t dwSrcObjNum,
std::list<CJBig2_CachePair>* pSymbolDictCache) {
auto result = pdfium::WrapUnique(
- new CJBig2_Context(pSrcStream, pSymbolDictCache, false));
- if (pGlobalStream && pGlobalStream->GetSize() > 0) {
- result->m_pGlobalContext = pdfium::WrapUnique(
- new CJBig2_Context(pGlobalStream, pSymbolDictCache, true));
+ new CJBig2_Context(pSrcSpan, dwSrcObjNum, pSymbolDictCache, false));
+ if (!pGlobalSpan.empty()) {
+ result->m_pGlobalContext = pdfium::WrapUnique(new CJBig2_Context(
+ pGlobalSpan, dwGlobalObjNum, pSymbolDictCache, true));
}
return result;
}
-CJBig2_Context::CJBig2_Context(const RetainPtr<CPDF_StreamAcc>& pSrcStream,
+CJBig2_Context::CJBig2_Context(pdfium::span<const uint8_t> pSrcSpan,
+ uint32_t dwObjNum,
std::list<CJBig2_CachePair>* pSymbolDictCache,
bool bIsGlobal)
- : m_pStream(pdfium::MakeUnique<CJBig2_BitStream>(
- pSrcStream->GetSpan(),
- pSrcStream->GetStream() ? pSrcStream->GetStream()->GetObjNum() : 0)),
+ : m_pStream(pdfium::MakeUnique<CJBig2_BitStream>(pSrcSpan, dwObjNum)),
m_HuffmanTables(CJBig2_HuffmanTable::kNumHuffmanTables),
m_bIsGlobal(bIsGlobal),
m_pSymbolDictCache(pSymbolDictCache) {}
diff --git a/core/fxcodec/jbig2/JBig2_Context.h b/core/fxcodec/jbig2/JBig2_Context.h
index ec9d4a7..49669ed 100644
--- a/core/fxcodec/jbig2/JBig2_Context.h
+++ b/core/fxcodec/jbig2/JBig2_Context.h
@@ -16,7 +16,7 @@
#include "core/fxcodec/jbig2/JBig2_Page.h"
#include "core/fxcodec/jbig2/JBig2_Segment.h"
#include "core/fxcrt/fx_safe_types.h"
-#include "core/fxcrt/retain_ptr.h"
+#include "third_party/base/span.h"
class CJBig2_ArithDecoder;
class CJBig2_GRDProc;
@@ -35,8 +35,10 @@
class CJBig2_Context {
public:
static std::unique_ptr<CJBig2_Context> Create(
- const RetainPtr<CPDF_StreamAcc>& pGlobalStream,
- const RetainPtr<CPDF_StreamAcc>& pSrcStream,
+ pdfium::span<const uint8_t> pGlobalSpan,
+ uint32_t dwGlobalObjNum,
+ pdfium::span<const uint8_t> pSrcSpan,
+ uint32_t dwSrcObjNum,
std::list<CJBig2_CachePair>* pSymbolDictCache);
~CJBig2_Context();
@@ -53,7 +55,8 @@
FXCODEC_STATUS GetProcessingStatus() const { return m_ProcessingStatus; }
private:
- CJBig2_Context(const RetainPtr<CPDF_StreamAcc>& pSrcStream,
+ CJBig2_Context(pdfium::span<const uint8_t> pSrcSpan,
+ uint32_t dwObjNum,
std::list<CJBig2_CachePair>* pSymbolDictCache,
bool bIsGlobal);
diff --git a/testing/fuzzers/pdf_codec_jbig2_fuzzer.cc b/testing/fuzzers/pdf_codec_jbig2_fuzzer.cc
index 82afa20..e559d3d 100644
--- a/testing/fuzzers/pdf_codec_jbig2_fuzzer.cc
+++ b/testing/fuzzers/pdf_codec_jbig2_fuzzer.cc
@@ -4,8 +4,6 @@
#include <cstdint>
-#include "core/fpdfapi/parser/cpdf_stream.h"
-#include "core/fpdfapi/parser/cpdf_stream_acc.h"
#include "core/fxcodec/JBig2_DocumentContext.h"
#include "core/fxcodec/codec/ccodec_jbig2module.h"
#include "core/fxcodec/jbig2/JBig2_Context.h"
@@ -37,17 +35,11 @@
if (!bitmap->Create(width, height, FXDIB_1bppRgb))
return 0;
- auto stream = pdfium::MakeRetain<CPDF_Stream>();
- stream->AsStream()->SetData({data, size});
-
- auto src_stream = pdfium::MakeRetain<CPDF_StreamAcc>(stream->AsStream());
- src_stream->LoadAllDataRaw();
-
CCodec_Jbig2Module module;
CCodec_Jbig2Context jbig2_context;
std::unique_ptr<JBig2_DocumentContext> document_context;
FXCODEC_STATUS status = module.StartDecode(
- &jbig2_context, &document_context, width, height, src_stream, nullptr,
+ &jbig2_context, &document_context, width, height, {data, size}, 1, {}, 0,
bitmap->GetBuffer(), bitmap->GetPitch(), nullptr);
while (status == FXCODEC_STATUS_DECODE_TOBECONTINUE)
