[C++] BinaryBuilder and ChunkedBinaryBuilder silently truncate values of 2 GiB or more

[ngoldbaum]

Describe the bug, including details regarding any error messages, version, and platform.

BaseBinaryBuilder::Append(std::string_view) and ChunkedBinaryBuilder::Append(std::string_view) narrow the value size to int32_t before calling the (const uint8_t*, int32_t) overload. For a value of 2 GiB or more the narrowed length is negative or wraps, and BaseBinaryBuilder::Append only copies data when the length is positive. The result is a non-null value holding the wrong bytes. ValidateFull also passes.

BinaryViewBuilder handles the same input correctly with CapacityError: BinaryView or StringView elements cannot reference strings larger than 2GB, and LargeBinaryBuilder stores the full value. I would expect the two 32-bit-offset builders to return a CapacityError the way BinaryViewBuilder does.

The callers in pyarrow guard against this on their own, so I did not find a way to trigger it from Python today. I'm not fixing this in the followup PR I'm opening to supersede #42018 with, because this is a problem with the Arrow C++ API that should be fixed separately. However, adding support for NumPy StringDType does make it possible to hit this from pure Python.

Reproducer, run against a build of main (54ea6d5), macOS arm64:

#include <iostream>
#include <string>
#include <string_view>

#include <arrow/api.h>

template <typename Builder>
void Check(const char* name, Builder& builder, std::string_view value) {
  auto status = builder.Append(value);
  std::cout << name << ": Append -> " << status.ToString();
  if (!status.ok()) {
    std::cout << std::endl;
    return;
  }
  std::shared_ptr<arrow::Array> array;
  if constexpr (std::is_same_v<Builder, arrow::internal::ChunkedBinaryBuilder>) {
    arrow::ArrayVector chunks;
    if (auto s = builder.Finish(&chunks); !s.ok()) { std::cout << s.ToString() << std::endl; return; }
    if (auto s = arrow::Concatenate(chunks).Value(&array); !s.ok()) { std::cout << s.ToString() << std::endl; return; }
  } else {
    if (auto s = builder.Finish(&array); !s.ok()) { std::cout << s.ToString() << std::endl; return; }
  }
  auto scalar = std::static_pointer_cast<arrow::BaseBinaryScalar>(array->GetScalar(0).ValueOrDie());
  std::cout << ", length " << array->length() << ", stored value size "
            << scalar->value->size() << ", ValidateFull -> "
            << array->ValidateFull().ToString() << std::endl;
}

int main() {
  const std::string value(static_cast<size_t>(1) << 31, 'x');
  std::cout << "value size " << value.size() << std::endl;

  arrow::internal::ChunkedBinaryBuilder chunked(1 << 24);
  Check("ChunkedBinaryBuilder", chunked, value);

  arrow::BinaryBuilder binary;
  Check("BinaryBuilder", binary, value);

  arrow::BinaryViewBuilder view;
  Check("BinaryViewBuilder", view, value);

  arrow::LargeBinaryBuilder large;
  Check("LargeBinaryBuilder", large, value);
  return 0;
}

$ clang++ -std=c++20 -I $ARROW_HOME/include repro.cc -o repro -L $ARROW_HOME/lib -larrow -Wl,-rpath,$ARROW_HOME/lib
$ ./repro
value size 2147483648
ChunkedBinaryBuilder: Append -> OK, length 1, stored value size 0, ValidateFull -> OK
BinaryBuilder: Append -> OK, length 1, stored value size 0, ValidateFull -> OK
BinaryViewBuilder: Append -> Capacity error: BinaryView or StringView elements cannot reference strings larger than 2GB
LargeBinaryBuilder: Append -> OK, length 1, stored value size 2147483648, ValidateFull -> OK

If I set the value size like this instead:

const std::string value((static_cast<size_t>(1) << 32) + 8, 'x');

(e.g. 4 GB + 8 bytes), I get:

value size 4294967304
ChunkedBinaryBuilder: Append -> OK, length 1, stored value size 8, ValidateFull -> OK
BinaryBuilder: Append -> OK, length 1, stored value size 8, ValidateFull -> OK
BinaryViewBuilder: Append -> Capacity error: BinaryView or StringView elements cannot reference strings larger than 2GB
LargeBinaryBuilder: Append -> OK, length 1, stored value size 4294967304, ValidateFull -> OK

Component(s)

C++