Merge changes Icd4322f2,I441ae663
* changes:
Allow bsdiff to accept multiple compressors
Modify the patch writer to use a list of compressors for each stream
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
diff --git a/patch_reader.cc b/patch_reader.cc
index 9f46857..e863b9a 100644
--- a/patch_reader.cc
+++ b/patch_reader.cc
@@ -7,6 +7,7 @@
#include <string.h>
#include <limits>
+#include <vector>
#include "bsdiff/brotli_decompressor.h"
#include "bsdiff/bspatch.h"
@@ -70,8 +71,10 @@
int64_t ctrl_len = ParseInt64(patch_data + 8);
int64_t diff_len = ParseInt64(patch_data + 16);
int64_t signed_newsize = ParseInt64(patch_data + 24);
+ // We already checked that the patch_size is at least 32 bytes.
if ((ctrl_len < 0) || (diff_len < 0) || (signed_newsize < 0) ||
- (32 + ctrl_len + diff_len > static_cast<int64_t>(patch_size))) {
+ (static_cast<int64_t>(patch_size) - 32 < ctrl_len) ||
+ (static_cast<int64_t>(patch_size) - 32 - ctrl_len < diff_len)) {
LOG(ERROR) << "Corrupt patch. ctrl_len: " << ctrl_len
<< ", data_len: " << diff_len
<< ", new_file_size: " << signed_newsize
diff --git a/patch_reader_unittest.cc b/patch_reader_unittest.cc
index 4526922..e3b32a9 100644
--- a/patch_reader_unittest.cc
+++ b/patch_reader_unittest.cc
@@ -7,6 +7,7 @@
#include <unistd.h>
#include <algorithm>
+#include <limits>
#include <string>
#include <vector>
@@ -51,14 +52,23 @@
EXPECT_TRUE(extra_stream_->Finish());
}
- void ConstructPatchData(std::vector<uint8_t>* patch_data) {
+ void ConstructPatchHeader(int64_t ctrl_size,
+ int64_t diff_size,
+ int64_t new_size,
+ std::vector<uint8_t>* patch_data) {
EXPECT_EQ(static_cast<size_t>(8), patch_data->size());
- // Encode the header
+ // Encode the header.
uint8_t buf[24];
- EncodeInt64(ctrl_stream_->GetCompressedData().size(), buf);
- EncodeInt64(diff_stream_->GetCompressedData().size(), buf + 8);
- EncodeInt64(new_file_size_, buf + 16);
+ EncodeInt64(ctrl_size, buf);
+ EncodeInt64(diff_size, buf + 8);
+ EncodeInt64(new_size, buf + 16);
std::copy(buf, buf + sizeof(buf), std::back_inserter(*patch_data));
+ }
+
+ void ConstructPatchData(std::vector<uint8_t>* patch_data) {
+ ConstructPatchHeader(ctrl_stream_->GetCompressedData().size(),
+ diff_stream_->GetCompressedData().size(),
+ new_file_size_, patch_data);
// Concatenate the three streams into one patch.
std::copy(ctrl_stream_->GetCompressedData().begin(),
@@ -94,6 +104,29 @@
EXPECT_TRUE(patch_reader.Finish());
}
+ // Helper function to check that invalid headers are detected. This method
+ // creates a new header with the passed |ctrl_size|, |diff_size| and
+ // |new_size| and appends after the header |compressed_size| bytes of extra
+ // zeros. It then expects that initializing a PatchReader with this will fail.
+ void InvalidHeaderTestHelper(int64_t ctrl_size,
+ int64_t diff_size,
+ int64_t new_size,
+ size_t compressed_size) {
+ std::vector<uint8_t> patch_data;
+ std::copy(kBSDF2MagicHeader, kBSDF2MagicHeader + 5,
+ std::back_inserter(patch_data));
+ patch_data.push_back(static_cast<uint8_t>(CompressorType::kBrotli));
+ patch_data.push_back(static_cast<uint8_t>(CompressorType::kBrotli));
+ patch_data.push_back(static_cast<uint8_t>(CompressorType::kBrotli));
+ ConstructPatchHeader(ctrl_size, diff_size, new_size, &patch_data);
+ patch_data.resize(patch_data.size() + compressed_size);
+
+ BsdiffPatchReader patch_reader;
+ EXPECT_FALSE(patch_reader.Init(patch_data.data(), patch_data.size()))
+ << "Where ctrl_size=" << ctrl_size << " diff_size=" << diff_size
+ << " new_size=" << new_size << " compressed_size=" << compressed_size;
+ }
+
size_t new_file_size_{500};
std::vector<std::string> diff_data_{"HelloWorld", "BspatchPatchTest",
"BspatchDiffData"};
@@ -141,4 +174,27 @@
VerifyPatch(patch_data);
}
+TEST_F(PatchReaderTest, InvalidHeaderTest) {
+ // Negative values are not allowed.
+ InvalidHeaderTestHelper(-1, 0, 20, 50);
+ InvalidHeaderTestHelper(30, -3, 20, 50);
+ InvalidHeaderTestHelper(30, 8, -20, 50);
+
+ // Values larger than the patch size are also not allowed for ctrl and diff,
+ // or for the sum of both.
+ InvalidHeaderTestHelper(30, 5, 20, 10); // 30 > 10
+ InvalidHeaderTestHelper(5, 30, 20, 10); // 30 > 10
+ InvalidHeaderTestHelper(30, 5, 20, 32); // 30 + 5 > 32
+
+ // Values that overflow int64 are also not allowed when used combined
+ const int64_t kMax64 = std::numeric_limits<int64_t>::max();
+ InvalidHeaderTestHelper(kMax64 - 5, 5, 20, 20);
+ InvalidHeaderTestHelper(5, kMax64 - 5, 20, 20);
+
+ // 2 * (kMax64 - 5) + sizeof(header) is still positive due to overflow, but
+ // the patch size is too small.
+ InvalidHeaderTestHelper(kMax64 - 5, kMax64 - 5, 20, 20);
+}
+
+
} // namespace bsdiff
