Behavioral difference between google/brotli and dropbox/rust-brotli

[durin42]

I've been tinkering with replacing brotli with rust-brotli, and I've hit an interesting behavioral difference. The included C++ program works correctly when using the C brotli implementation, but segfaults with the Rust one:

#include <fstream>
#include <iostream>
#include <streambuf>
#include <string>
#include <string_view>

#include "brotli/encode.h"

int main(int argc, char** argv) {
  if (argc < 2) {
    std::cerr << "pass a file to compress" << std::endl;
    return 1;
  }
  std::string path = argv[1];
  std::cout << "attempting to brotli-ize " << path << std::endl;

  std::ifstream input_file(path);
  std::string data((std::istreambuf_iterator<char>(input_file)),
                   std::istreambuf_iterator<char>());
  std::string_view dv(data);

  auto* state = BrotliEncoderCreateInstance(nullptr, nullptr, nullptr);
  if (state == nullptr) {
    std::cerr << "failed to make brotli encoder" << std::endl;
    return 1;
  }
  if (!BrotliEncoderSetParameter(state, BROTLI_PARAM_SIZE_HINT,
                                 dv.size())) {
    std::cerr << "failed to give brotli a size hint" << std::endl;
    return 1;
  }
  size_t in_left, out_left, written = 0;
  BROTLI_BOOL result = BROTLI_TRUE;
  while (dv.size() && result) {
    in_left = dv.size();
    out_left = 0;
    const uint8_t* next_in = reinterpret_cast<const uint8_t*>(dv.data());
    std::cout << "attempt to compress " << dv.size() << std::endl;
    result =
        BrotliEncoderCompressStream(state, BROTLI_OPERATION_PROCESS, &in_left,
                                    &next_in, &out_left, nullptr, nullptr);
    size_t buffer_size = 0;
    /*const uint8_t* buf = */ BrotliEncoderTakeOutput(state, &buffer_size);
    if (buffer_size) {
      written += buffer_size;
    }
    dv.remove_prefix(dv.size() - in_left);
  }
  while (result && !BrotliEncoderIsFinished(state)) {
    in_left = 0;
    out_left = 0;
    const uint8_t* next_in = nullptr;
    result = BrotliEncoderCompressStream(
        state, BROTLI_OPERATION_FINISH, &in_left, &next_in,
        &out_left, nullptr, nullptr);
    size_t buffer_size = 0;
    /*const uint8_t* buffer = */BrotliEncoderTakeOutput(state, &buffer_size);
    if (buffer_size > 0) {
      written += buffer_size;
    }
  }
  BrotliEncoderDestroyInstance(state);
  std::cout << "brotli'd down to " << written << " bytes" << std::endl;
  return 0;
}

I'm not familiar enough with the internals of rust-brotli to figure this out quickly, but I was hoping by filing a bug with a reduced test case might help someone figure this out. I think this might be related to #44, but I don't appear to need multiple chunks to trigger the segfault. As far as I can tell, the way this test program uses brotli is supported per the comments in encode.h, so I assume that the crasher in Rust is a bug of missing functionality and not merely a guard against an unexpected NULL.

[alex]

diff --git a/src/ffi/compressor.rs b/src/ffi/compressor.rs
index 3dc88d3..df6b045 100755
--- a/src/ffi/compressor.rs
+++ b/src/ffi/compressor.rs
@@ -282,7 +282,11 @@ pub unsafe extern fn BrotliEncoderCompressStream(
     };
     {
       let input_buf = slice_from_raw_parts_or_nil(*input_buf_ptr, *available_in);
-      let output_buf = slice_from_raw_parts_or_nil_mut(*output_buf_ptr, *available_out);
+      let output_buf = if *available_out != 0 {
+        slice_from_raw_parts_or_nil_mut(*output_buf_ptr, *available_out)
+      } else {
+        &mut []
+      };
       let mut to = Some(0usize);
       result = ::enc::encode::BrotliEncoderCompressStream(
         &mut (*state_ptr).compressor,

should fix the segfault, however I don't know if it fixes the actual issue or just causes a problem somewhere else.

[danielrh]

your fix is good! Thanks. I didn't expect people to pass in a pointer with 0 size available plus a nullptr to the encoder library