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.

Android.bp

@@ -117,6 +117,7 @@ cc_test {
         "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",

Makefile

@@ -68,6 +68,7 @@ bsdiff_common_unittests := \
     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 \

brotli_compressor_unittest.cc

@@ -38,9 +38,9 @@ TEST(BrotliCompressorTest, BrotliCompressorSmoke) {
 }
 
 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();

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::SetInputData(const uint8_t* input_data, size_t size) {
 }
 
 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::Read(uint8_t* output_data, size_t bytes_to_output) {
 }
 
 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 @@ bool BrotliDecompressor::Close() {
   }
 
   BrotliDecoderDestroyInstance(brotli_decoder_state_);
+  brotli_decoder_state_ = nullptr;
   return true;
 }
 

brotli_decompressor.h

@@ -15,6 +15,7 @@ class BrotliDecompressor : public DecompressorInterface {
  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;

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',

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 @@ bool BZ2Decompressor::SetInputData(const uint8_t* input_data, size_t size) {
     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 @@ bool BZ2Decompressor::Read(uint8_t* output_data, size_t bytes_to_output) {
       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;
 }
 

bz2_decompressor.h

@@ -14,6 +14,7 @@ namespace bsdiff {
 class BZ2Decompressor : public DecompressorInterface {
  public:
   BZ2Decompressor() = default;
+  ~BZ2Decompressor();
 
   bool SetInputData(const uint8_t* input_data, size_t size) override;
 
@@ -22,7 +23,11 @@ class BZ2Decompressor : public DecompressorInterface {
   bool Close() override;
 
  private:
+  // The low-level bzip2 stream.
   bz_stream stream_;
+
+  // Whether the stream_ is initialized.
+  bool stream_initialized_{false};
 };
 
 }  // namespace bsdiff

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