Explicitly allow IFX_WriteStream::WriteBlock() calls with size of 0.
Make it clear that the interface allows this. Fix implementations to be
consistent with the interface, and remove a few checks that skipped
WriteBlock() calls because the size is 0.
At a test case to make sure CFX_MemoryStream::WriteBlock() works as
expected.
Also consistently not return false in WriteBlock() implementation when
the data pointer is null.
Change-Id: I4962f56b66c31c53339feb120f5a33d440fa72d7
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98090
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index c7f7424..76c9e93 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -74,9 +74,10 @@
}
bool CFX_FileBufferArchive::WriteBlock(const void* pBuf, size_t size) {
- DCHECK(pBuf);
- DCHECK(size > 0);
+ if (size == 0)
+ return true;
+ DCHECK(pBuf);
auto* pSrc = reinterpret_cast<const uint8_t*>(pBuf);
pdfium::span<const uint8_t> src_span(pSrc, size);
while (!src_span.empty()) {
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 96cc237..674f0ba 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -212,7 +212,7 @@
if (!archive->WriteString("stream\r\n"))
return false;
- if (size && !archive->WriteBlock(data.data(), size))
+ if (!archive->WriteBlock(data.data(), size))
return false;
if (!archive->WriteString("\r\nendstream"))
diff --git a/core/fxcrt/cfx_memorystream.cpp b/core/fxcrt/cfx_memorystream.cpp
index 5ca97ed..4361fdb 100644
--- a/core/fxcrt/cfx_memorystream.cpp
+++ b/core/fxcrt/cfx_memorystream.cpp
@@ -64,9 +64,13 @@
bool CFX_MemoryStream::WriteBlockAtOffset(const void* buffer,
FX_FILESIZE offset,
size_t size) {
- if (!buffer || offset < 0 || !size)
+ if (offset < 0)
return false;
+ if (size == 0)
+ return true;
+
+ DCHECK(buffer);
FX_SAFE_SIZE_T safe_new_pos = size;
safe_new_pos += offset;
if (!safe_new_pos.IsValid())
diff --git a/core/fxcrt/cfx_memorystream_unittest.cpp b/core/fxcrt/cfx_memorystream_unittest.cpp
index da71e27..716e47b 100644
--- a/core/fxcrt/cfx_memorystream_unittest.cpp
+++ b/core/fxcrt/cfx_memorystream_unittest.cpp
@@ -48,3 +48,20 @@
ASSERT_TRUE(stream->ReadBlockAtOffset(buffer, 2, sizeof(buffer)));
ASSERT_THAT(buffer, testing::ElementsAre('c', '\0', '\0', 'a'));
}
+
+TEST(CFXMemoryStreamTest, WriteZeroBytes) {
+ auto stream = pdfium::MakeRetain<CFX_MemoryStream>();
+ const uint8_t kData1[] = {'a', 'b', 'c'};
+ ASSERT_TRUE(stream->WriteBlock(kData1, sizeof(kData1)));
+ auto stream_contents =
+ pdfium::make_span(stream->GetBuffer(), stream->GetSize());
+ ASSERT_THAT(stream_contents, testing::ElementsAre('a', 'b', 'c'));
+
+ ASSERT_TRUE(stream->WriteBlock(kData1, 0));
+ stream_contents = pdfium::make_span(stream->GetBuffer(), stream->GetSize());
+ ASSERT_THAT(stream_contents, testing::ElementsAre('a', 'b', 'c'));
+
+ ASSERT_TRUE(stream->WriteBlock(nullptr, 0));
+ stream_contents = pdfium::make_span(stream->GetBuffer(), stream->GetSize());
+ ASSERT_THAT(stream_contents, testing::ElementsAre('a', 'b', 'c'));
+}
diff --git a/core/fxcrt/fx_stream.h b/core/fxcrt/fx_stream.h
index f82b264..0154b978 100644
--- a/core/fxcrt/fx_stream.h
+++ b/core/fxcrt/fx_stream.h
@@ -16,6 +16,8 @@
class IFX_WriteStream {
public:
+ // When `size` is 0, treat it as a no-op and return true. That is also the
+ // only time when `pData` can be null.
virtual bool WriteBlock(const void* pData, size_t size) = 0;
bool WriteString(ByteStringView str);
diff --git a/fpdfsdk/fpdf_ppo_embeddertest.cpp b/fpdfsdk/fpdf_ppo_embeddertest.cpp
index ee776ba..8c518cc 100644
--- a/fpdfsdk/fpdf_ppo_embeddertest.cpp
+++ b/fpdfsdk/fpdf_ppo_embeddertest.cpp
@@ -24,7 +24,7 @@
int FakeBlockWriter(FPDF_FILEWRITE* pThis,
const void* pData,
unsigned long size) {
- return size;
+ return 1; // Always succeeds.
}
constexpr int kRectanglesMultiPagesPageCount = 2;
