Merge changes Iaee8a218,I11e703f8,Ia2664bba,I29d3e996
* changes:
Fix infinite loop when providing a partial input bz2 stream.
Fix -Wzero-length-array warning in unittest code.
Destroy the brotli decompressor on destruction.
Destroy the bz2 decompressor on destruction.
diff --git a/Android.bp b/Android.bp
index 7390a11..8d2ec0f 100644
--- a/Android.bp
+++ b/Android.bp
@@ -117,6 +117,7 @@
"bsdiff_arguments_unittest.cc",
"bsdiff_unittest.cc",
"bspatch_unittest.cc",
+ "bz2_decompressor_unittest.cc",
"diff_encoder_unittest.cc",
"endsley_patch_writer_unittest.cc",
"extents_file_unittest.cc",
diff --git a/Makefile b/Makefile
index e721e7d..7846a4f 100644
--- a/Makefile
+++ b/Makefile
@@ -68,6 +68,7 @@
bsdiff_arguments_unittest.cc \
bsdiff_unittest.cc \
bspatch_unittest.cc \
+ bz2_decompressor_unittest.cc \
diff_encoder_unittest.cc \
endsley_patch_writer_unittest.cc \
extents_file_unittest.cc \
diff --git a/brotli_compressor_unittest.cc b/brotli_compressor_unittest.cc
index cfa66f6..41572dd 100644
--- a/brotli_compressor_unittest.cc
+++ b/brotli_compressor_unittest.cc
@@ -38,9 +38,9 @@
}
TEST(BrotliCompressorTest, BrotliCompressorEmptyStream) {
- uint8_t empty_buffer[] = {};
+ uint8_t unused_value = 0;
BrotliCompressor brotli_compressor(11);
- EXPECT_TRUE(brotli_compressor.Write(empty_buffer, sizeof(empty_buffer)));
+ EXPECT_TRUE(brotli_compressor.Write(&unused_value, 0));
EXPECT_TRUE(brotli_compressor.Finish());
std::vector<uint8_t> compressed_data = brotli_compressor.GetCompressedData();
diff --git a/brotli_decompressor.cc b/brotli_decompressor.cc
index 1bc7722..90216c6 100644
--- a/brotli_decompressor.cc
+++ b/brotli_decompressor.cc
@@ -8,6 +8,11 @@
namespace bsdiff {
+BrotliDecompressor::~BrotliDecompressor() {
+ if (brotli_decoder_state_)
+ BrotliDecoderDestroyInstance(brotli_decoder_state_);
+}
+
bool BrotliDecompressor::SetInputData(const uint8_t* input_data, size_t size) {
brotli_decoder_state_ =
BrotliDecoderCreateInstance(nullptr, nullptr, nullptr);
@@ -21,6 +26,10 @@
}
bool BrotliDecompressor::Read(uint8_t* output_data, size_t bytes_to_output) {
+ if (!brotli_decoder_state_) {
+ LOG(ERROR) << "BrotliDecompressor not initialized";
+ return false;
+ }
auto next_out = output_data;
size_t available_out = bytes_to_output;
@@ -45,6 +54,10 @@
}
bool BrotliDecompressor::Close() {
+ if (!brotli_decoder_state_) {
+ LOG(ERROR) << "BrotliDecompressor not initialized";
+ return false;
+ }
// In some cases, the brotli compressed stream could be empty. As a result,
// the function BrotliDecoderIsFinished() will return false because we never
// start the decompression. When that happens, we just destroy the decoder
@@ -56,6 +69,7 @@
}
BrotliDecoderDestroyInstance(brotli_decoder_state_);
+ brotli_decoder_state_ = nullptr;
return true;
}
diff --git a/brotli_decompressor.h b/brotli_decompressor.h
index 0c63d8f..3c837c6 100644
--- a/brotli_decompressor.h
+++ b/brotli_decompressor.h
@@ -15,6 +15,7 @@
public:
BrotliDecompressor()
: brotli_decoder_state_(nullptr), next_in_(nullptr), available_in_(0) {}
+ ~BrotliDecompressor();
// DecompressorInterface overrides.
bool SetInputData(const uint8_t* input_data, size_t size) override;
diff --git a/bsdiff.gyp b/bsdiff.gyp
index 40bad2a..629b9e0 100644
--- a/bsdiff.gyp
+++ b/bsdiff.gyp
@@ -155,6 +155,7 @@
'bsdiff_arguments_unittest.cc',
'bsdiff_unittest.cc',
'bspatch_unittest.cc',
+ 'bz2_decompressor_unittest.cc',
'diff_encoder_unittest.cc',
'endsley_patch_writer_unittest.cc',
'extents_file_unittest.cc',
diff --git a/bz2_decompressor.cc b/bz2_decompressor.cc
index 1a3636e..4504ac5 100644
--- a/bz2_decompressor.cc
+++ b/bz2_decompressor.cc
@@ -13,6 +13,12 @@
namespace bsdiff {
+BZ2Decompressor::~BZ2Decompressor() {
+ // Release the memory on destruction if needed.
+ if (stream_initialized_)
+ BZ2_bzDecompressEnd(&stream_);
+}
+
bool BZ2Decompressor::SetInputData(const uint8_t* input_data, size_t size) {
// TODO(xunchang) update the avail_in for size > 2GB.
if (size > std::numeric_limits<unsigned int>::max()) {
@@ -30,16 +36,22 @@
LOG(ERROR) << "Failed to bzinit control stream: " << bz2err;
return false;
}
+ stream_initialized_ = true;
return true;
}
bool BZ2Decompressor::Read(uint8_t* output_data, size_t bytes_to_output) {
+ if (!stream_initialized_) {
+ LOG(ERROR) << "BZ2Decompressor not initialized";
+ return false;
+ }
stream_.next_out = reinterpret_cast<char*>(output_data);
while (bytes_to_output > 0) {
size_t output_size = std::min<size_t>(
std::numeric_limits<unsigned int>::max(), bytes_to_output);
stream_.avail_out = output_size;
+ size_t prev_avail_in = stream_.avail_in;
int bz2err = BZ2_bzDecompress(&stream_);
if (bz2err != BZ_OK && bz2err != BZ_STREAM_END) {
LOG(ERROR) << "Failed to decompress " << output_size
@@ -47,16 +59,27 @@
return false;
}
bytes_to_output -= (output_size - stream_.avail_out);
+ if (bytes_to_output && prev_avail_in == stream_.avail_in &&
+ output_size == stream_.avail_out) {
+ LOG(ERROR) << "BZ2Decompressor made no progress, pending "
+ << bytes_to_output << " bytes to decompress";
+ return false;
+ }
}
return true;
}
bool BZ2Decompressor::Close() {
+ if (!stream_initialized_) {
+ LOG(ERROR) << "BZ2Decompressor not initialized";
+ return false;
+ }
int bz2err = BZ2_bzDecompressEnd(&stream_);
if (bz2err != BZ_OK) {
LOG(ERROR) << "BZ2_bzDecompressEnd returns with " << bz2err;
return false;
}
+ stream_initialized_ = false;
return true;
}
diff --git a/bz2_decompressor.h b/bz2_decompressor.h
index 9b2acdc..f752b5e 100644
--- a/bz2_decompressor.h
+++ b/bz2_decompressor.h
@@ -14,6 +14,7 @@
class BZ2Decompressor : public DecompressorInterface {
public:
BZ2Decompressor() = default;
+ ~BZ2Decompressor();
bool SetInputData(const uint8_t* input_data, size_t size) override;
@@ -22,7 +23,11 @@
bool Close() override;
private:
+ // The low-level bzip2 stream.
bz_stream stream_;
+
+ // Whether the stream_ is initialized.
+ bool stream_initialized_{false};
};
} // namespace bsdiff
diff --git a/bz2_decompressor_unittest.cc b/bz2_decompressor_unittest.cc
new file mode 100644
index 0000000..6fa819c
--- /dev/null
+++ b/bz2_decompressor_unittest.cc
@@ -0,0 +1,58 @@
+// Copyright 2018 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "bsdiff/bz2_decompressor.h"
+
+#include <memory>
+
+#include <gtest/gtest.h>
+
+namespace {
+
+// echo -n "Hello!" | bzip2 -9 | hexdump -v -e '" " 11/1 "0x%02x, " "\n"'
+constexpr uint8_t kBZ2Hello[] = {
+ 0x42, 0x5a, 0x68, 0x39, 0x31, 0x41, 0x59, 0x26, 0x53, 0x59, 0x1a,
+ 0xea, 0x74, 0xba, 0x00, 0x00, 0x00, 0x95, 0x00, 0x20, 0x00, 0x00,
+ 0x40, 0x02, 0x04, 0xa0, 0x00, 0x21, 0x83, 0x41, 0x9a, 0x02, 0x5c,
+ 0x2e, 0x2e, 0xe4, 0x8a, 0x70, 0xa1, 0x20, 0x35, 0xd4, 0xe9, 0x74,
+};
+
+} // namespace
+
+namespace bsdiff {
+
+class BZ2DecompressorTest : public testing::Test {
+ protected:
+ void SetUp() {
+ decompressor_.reset(new BZ2Decompressor());
+ EXPECT_NE(nullptr, decompressor_.get());
+ }
+
+ std::unique_ptr<BZ2Decompressor> decompressor_;
+};
+
+TEST_F(BZ2DecompressorTest, ReadingFromEmptyFileTest) {
+ uint8_t data = 0;
+ EXPECT_TRUE(decompressor_->SetInputData(&data, 0));
+
+ uint8_t output_data[10];
+ EXPECT_FALSE(decompressor_->Read(output_data, sizeof(output_data)));
+}
+
+// Check that we fail to read from a truncated file.
+TEST_F(BZ2DecompressorTest, ReadingFromTruncatedFileTest) {
+ // We feed only half of the compressed file.
+ EXPECT_TRUE(decompressor_->SetInputData(kBZ2Hello, sizeof(kBZ2Hello) / 2));
+ uint8_t output_data[6];
+ EXPECT_FALSE(decompressor_->Read(output_data, sizeof(output_data)));
+}
+
+// Check that we fail to read more than it is available in the file.
+TEST_F(BZ2DecompressorTest, ReadingMoreThanAvailableTest) {
+ EXPECT_TRUE(decompressor_->SetInputData(kBZ2Hello, sizeof(kBZ2Hello)));
+ uint8_t output_data[1000];
+ EXPECT_FALSE(decompressor_->Read(output_data, sizeof(output_data)));
+}
+
+} // namespace bsdiff
