Convert BasicModule::A85Encode() to use DataVector. Bug: pdfium:1872 Change-Id: I7b7e6265988947b072f5e04c2cce73fb3e3aedc2 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97532 Commit-Queue: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxcodec/basic/a85_unittest.cpp b/core/fxcodec/basic/a85_unittest.cpp index 93b121e..e016a93 100644 --- a/core/fxcodec/basic/a85_unittest.cpp +++ b/core/fxcodec/basic/a85_unittest.cpp
@@ -9,155 +9,122 @@ #include <memory> #include "core/fxcodec/basic/basicmodule.h" -#include "core/fxcrt/fx_memory_wrappers.h" +#include "core/fxcrt/data_vector.h" #include "testing/gtest/include/gtest/gtest.h" -TEST(fxcodec, A85TestBadInputs) { - const uint8_t src_buf[] = {1, 2, 3, 4}; - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; - - // Error codes, not segvs, should callers pass us a nullptr pointer. - EXPECT_FALSE(BasicModule::A85Encode(src_buf, &dest_buf, nullptr)); - EXPECT_FALSE(BasicModule::A85Encode(src_buf, nullptr, &dest_size)); - EXPECT_FALSE(BasicModule::A85Encode({}, &dest_buf, &dest_size)); +TEST(fxcodec, A85EmptyInput) { + EXPECT_TRUE(BasicModule::A85Encode({}).empty()); } // No leftover bytes, just translate 2 sets of symbols. -TEST(fxcodec, A85TestBasic) { +TEST(fxcodec, A85Basic) { // Make sure really big values don't break. const uint8_t src_buf[] = {1, 2, 3, 4, 255, 255, 255, 255}; - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; - EXPECT_TRUE(BasicModule::A85Encode(src_buf, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf); // Should have 5 chars for each set of 4 and 2 terminators. const uint8_t expected_out[] = {33, 60, 78, 63, 43, 115, 56, 87, 45, 33, 126, 62}; - ASSERT_EQ(std::size(expected_out), dest_size); + ASSERT_EQ(std::size(expected_out), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out[i], dest_buf[i]) << " at " << i; } // Leftover bytes. -TEST(fxcodec, A85TestLeftoverBytes) { - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; - +TEST(fxcodec, A85LeftoverBytes) { { // 1 Leftover Byte: const uint8_t src_buf_1leftover[] = {1, 2, 3, 4, 255}; - EXPECT_TRUE( - BasicModule::A85Encode(src_buf_1leftover, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf_1leftover); // 5 chars for first symbol + 2 + 2 terminators. uint8_t expected_out_1leftover[] = {33, 60, 78, 63, 43, 114, 114, 126, 62}; - ASSERT_EQ(std::size(expected_out_1leftover), dest_size); + ASSERT_EQ(std::size(expected_out_1leftover), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out_1leftover[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out_1leftover[i], dest_buf[i]) << " at " << i; } { // 2 Leftover bytes: const uint8_t src_buf_2leftover[] = {1, 2, 3, 4, 255, 254}; - dest_buf.reset(); - dest_size = 0; - EXPECT_TRUE( - BasicModule::A85Encode(src_buf_2leftover, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf_2leftover); // 5 chars for first symbol + 3 + 2 terminators. const uint8_t expected_out_2leftover[] = {33, 60, 78, 63, 43, 115, 56, 68, 126, 62}; - ASSERT_EQ(std::size(expected_out_2leftover), dest_size); + ASSERT_EQ(std::size(expected_out_2leftover), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out_2leftover[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out_2leftover[i], dest_buf[i]) << " at " << i; } { // 3 Leftover bytes: const uint8_t src_buf_3leftover[] = {1, 2, 3, 4, 255, 254, 253}; - dest_buf.reset(); - dest_size = 0; - EXPECT_TRUE( - BasicModule::A85Encode(src_buf_3leftover, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf_3leftover); // 5 chars for first symbol + 4 + 2 terminators. const uint8_t expected_out_3leftover[] = {33, 60, 78, 63, 43, 115, 56, 77, 114, 126, 62}; - ASSERT_EQ(std::size(expected_out_3leftover), dest_size); + ASSERT_EQ(std::size(expected_out_3leftover), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out_3leftover[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out_3leftover[i], dest_buf[i]) << " at " << i; } } // Test all zeros comes through as "z". -TEST(fxcodec, A85TestZeros) { - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; - +TEST(fxcodec, A85Zeros) { { // Make sure really big values don't break. const uint8_t src_buf[] = {1, 2, 3, 4, 0, 0, 0, 0}; - EXPECT_TRUE(BasicModule::A85Encode(src_buf, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf); // Should have 5 chars for first set of 4 + 1 for z + 2 terminators. const uint8_t expected_out[] = {33, 60, 78, 63, 43, 122, 126, 62}; - ASSERT_EQ(std::size(expected_out), dest_size); + ASSERT_EQ(std::size(expected_out), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out[i], dest_buf[i]) << " at " << i; } { // Should also work if it is at the start: const uint8_t src_buf_2[] = {0, 0, 0, 0, 1, 2, 3, 4}; - dest_buf.reset(); - dest_size = 0; - EXPECT_TRUE(BasicModule::A85Encode(src_buf_2, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf_2); // Should have 5 chars for set of 4 + 1 for z + 2 terminators. const uint8_t expected_out_2[] = {122, 33, 60, 78, 63, 43, 126, 62}; - ASSERT_EQ(std::size(expected_out_2), dest_size); + ASSERT_EQ(std::size(expected_out_2), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out_2[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out_2[i], dest_buf[i]) << " at " << i; } { // Try with 2 leftover zero bytes. Make sure we don't get a "z". const uint8_t src_buf_3[] = {1, 2, 3, 4, 0, 0}; - dest_buf.reset(); - dest_size = 0; - EXPECT_TRUE(BasicModule::A85Encode(src_buf_3, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf_3); // Should have 5 chars for set of 4 + 3 for last 2 + 2 terminators. const uint8_t expected_out_leftover[] = {33, 60, 78, 63, 43, 33, 33, 33, 126, 62}; - ASSERT_EQ(std::size(expected_out_leftover), dest_size); + ASSERT_EQ(std::size(expected_out_leftover), dest_buf.size()); // Check the output - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - for (uint32_t i = 0; i < dest_size; i++) - EXPECT_EQ(expected_out_leftover[i], dest_buf_span[i]) << " at " << i; + for (uint32_t i = 0; i < dest_buf.size(); i++) + EXPECT_EQ(expected_out_leftover[i], dest_buf[i]) << " at " << i; } } // Make sure we get returns in the expected locations. -TEST(fxcodec, A85TestLineBreaks) { +TEST(fxcodec, A85LineBreaks) { // Make sure really big values don't break. uint8_t src_buf[131] = {0}; // 1 full line + most of a line of normal symbols. @@ -174,21 +141,18 @@ src_buf[k + 2] = 3; src_buf[k + 3] = 4; } - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; // Should succeed. - EXPECT_TRUE(BasicModule::A85Encode(src_buf, &dest_buf, &dest_size)); + DataVector<uint8_t> dest_buf = BasicModule::A85Encode(src_buf); // Should have 75 chars in the first row plus 2 char return, // 76 chars in the second row plus 2 char return, // and 9 chars in the last row with 2 terminators. - ASSERT_EQ(166u, dest_size); + ASSERT_EQ(166u, dest_buf.size()); // Check for the returns. - auto dest_buf_span = pdfium::make_span(dest_buf.get(), dest_size); - EXPECT_EQ(13, dest_buf_span[75]); - EXPECT_EQ(10, dest_buf_span[76]); - EXPECT_EQ(13, dest_buf_span[153]); - EXPECT_EQ(10, dest_buf_span[154]); + EXPECT_EQ(13, dest_buf[75]); + EXPECT_EQ(10, dest_buf[76]); + EXPECT_EQ(13, dest_buf[153]); + EXPECT_EQ(10, dest_buf[154]); }
diff --git a/core/fxcodec/basic/basicmodule.cpp b/core/fxcodec/basic/basicmodule.cpp index d9285d0..ab2a003 100644 --- a/core/fxcodec/basic/basicmodule.cpp +++ b/core/fxcodec/basic/basicmodule.cpp
@@ -308,17 +308,11 @@ } // static -bool BasicModule::A85Encode(pdfium::span<const uint8_t> src_span, - std::unique_ptr<uint8_t, FxFreeDeleter>* dest_buf, - uint32_t* dest_size) { - // Check inputs. - if (!dest_buf || !dest_size) - return false; - - if (src_span.empty()) { - *dest_size = 0; - return false; - } +DataVector<uint8_t> BasicModule::A85Encode( + pdfium::span<const uint8_t> src_span) { + DataVector<uint8_t> result; + if (src_span.empty()) + return result; // Worst case: 5 output for each 4 input (plus up to 4 from leftover), plus // 2 character new lines each 75 output chars plus 2 termination chars. May @@ -329,10 +323,10 @@ estimated_size += 4; estimated_size += src_span.size() / 30; estimated_size += 2; - dest_buf->reset(FX_Alloc(uint8_t, estimated_size.ValueOrDie())); + result.resize(estimated_size.ValueOrDie()); // Set up pointers. - uint8_t* out = dest_buf->get(); + uint8_t* out = result.data(); uint32_t pos = 0; uint32_t line_length = 0; while (src_span.size() >= 4 && pos < src_span.size() - 3) { @@ -345,8 +339,8 @@ line_length++; } else { // Compute base 85 characters and add 33. for (int i = 4; i >= 0; i--) { - out[i] = (uint8_t)(val % 85) + 33; - val = val / 85; + out[i] = (val % 85) + 33; + val /= 85; } out += 5; line_length += 5; @@ -367,8 +361,8 @@ } for (int i = 4; i >= 0; i--) { if (i <= count) - out[i] = (uint8_t)(val % 85) + 33; - val = val / 85; + out[i] = (val % 85) + 33; + val /= 85; } out += count + 1; } @@ -377,8 +371,10 @@ out[0] = '~'; out[1] = '>'; out += 2; - *dest_size = pdfium::base::checked_cast<uint32_t>(out - dest_buf->get()); - return true; + size_t new_size = out - result.data(); + CHECK_LE(new_size, result.size()); + result.resize(new_size); + return result; } } // namespace fxcodec
diff --git a/core/fxcodec/basic/basicmodule.h b/core/fxcodec/basic/basicmodule.h index a44621f..674c8ff 100644 --- a/core/fxcodec/basic/basicmodule.h +++ b/core/fxcodec/basic/basicmodule.h
@@ -12,7 +12,6 @@ #include <memory> #include "core/fxcrt/data_vector.h" -#include "core/fxcrt/fx_memory_wrappers.h" #include "third_party/base/span.h" namespace fxcodec { @@ -31,9 +30,7 @@ static DataVector<uint8_t> RunLengthEncode( pdfium::span<const uint8_t> src_span); - static bool A85Encode(pdfium::span<const uint8_t> src_span, - std::unique_ptr<uint8_t, FxFreeDeleter>* dest_buf, - uint32_t* dest_size); + static DataVector<uint8_t> A85Encode(pdfium::span<const uint8_t> src_span); BasicModule() = delete; BasicModule(const BasicModule&) = delete;
diff --git a/core/fxge/win32/cfx_psrenderer.cpp b/core/fxge/win32/cfx_psrenderer.cpp index 0a6b9e8..8685915 100644 --- a/core/fxge/win32/cfx_psrenderer.cpp +++ b/core/fxge/win32/cfx_psrenderer.cpp
@@ -879,13 +879,10 @@ } void CFX_PSRenderer::WritePSBinary(pdfium::span<const uint8_t> data) { - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size; - if (m_pEncoderIface->pA85EncodeFunc(data, &dest_buf, &dest_size)) { - m_Output.write(reinterpret_cast<const char*>(dest_buf.get()), dest_size); - } else { - m_Output.write(reinterpret_cast<const char*>(data.data()), data.size()); - } + DataVector<uint8_t> encoded_data = m_pEncoderIface->pA85EncodeFunc(data); + pdfium::span<const uint8_t> result = + encoded_data.empty() ? data : encoded_data; + m_Output.write(reinterpret_cast<const char*>(result.data()), result.size()); } void CFX_PSRenderer::WriteStream(fxcrt::ostringstream& stream) {
diff --git a/core/fxge/win32/cfx_psrenderer.h b/core/fxge/win32/cfx_psrenderer.h index 2031fd5..991716a 100644 --- a/core/fxge/win32/cfx_psrenderer.h +++ b/core/fxge/win32/cfx_psrenderer.h
@@ -36,9 +36,7 @@ struct FXDIB_ResampleOptions; struct EncoderIface { - bool (*pA85EncodeFunc)(pdfium::span<const uint8_t> src_buf, - std::unique_ptr<uint8_t, FxFreeDeleter>* dest_buf, - uint32_t* dest_size); + DataVector<uint8_t> (*pA85EncodeFunc)(pdfium::span<const uint8_t> src_buf); void (*pFaxEncodeFunc)(const uint8_t* src_buf, int width, int height,
diff --git a/testing/fuzzers/pdf_codec_a85_fuzzer.cc b/testing/fuzzers/pdf_codec_a85_fuzzer.cc index f9ae1fe..3502955 100644 --- a/testing/fuzzers/pdf_codec_a85_fuzzer.cc +++ b/testing/fuzzers/pdf_codec_a85_fuzzer.cc
@@ -3,14 +3,10 @@ // found in the LICENSE file. #include <cstdint> -#include <memory> #include "core/fxcodec/basic/basicmodule.h" -#include "core/fxcrt/fx_memory_wrappers.h" extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { - std::unique_ptr<uint8_t, FxFreeDeleter> dest_buf; - uint32_t dest_size = 0; - BasicModule::A85Encode({data, size}, &dest_buf, &dest_size); + BasicModule::A85Encode({data, size}); return 0; }