This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new a99464f9c6 GH-33823: [C++][IPC] Improve error messages when opening
files that are the wrong format (#49771)
a99464f9c6 is described below
commit a99464f9c625c3f85cade9d7d50eb25297ca1f8d
Author: Robert DeRienzo <[email protected]>
AuthorDate: Thu Oct 29 08:24:17 2026 -0400
GH-33823: [C++][IPC] Improve error messages when opening files that are the
wrong format (#49771)
### Rationale for this change
The original error messages did not provide instruction to users on how to
best correct their usage when opening an Arrow IPC file with the wrong reader.
### What changes are included in this PR?
Improved error messages when an IPC file is opened with the stream reader
and vice versa, guiding users to the correct reader.
- Moved the file-magic check out of DecodeMessage and into an
OnMetadata() override on InputStreamMessageReader. It only checks the first
message, so later messages in a valid stream are not flagged.
- Check before metadata read to avoid the 1.3GB read issue
### Are these changes tested?
Yes. New tests were added to read_write_test.cc:
OpenFileFormatSuggestsFileReader
CorruptDataDoesNotSuggestFileReader
FileMagicAfterFirstMessageDoesNotSuggestFileReader
OpenStreamFormatSuggestsStreamReader
Existing Tests executed and passing.
### Are there any user-facing changes?
Yes, error messages when using the wrong IPC reader have been updated.
* GitHub Issue: #33823
Lead-authored-by: Robert DeRienzo <[email protected]>
Co-authored-by: RobertLD <[email protected]>
Co-authored-by: Robert DeRienzo <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/ipc/message.cc | 14 ++++++++
cpp/src/arrow/ipc/read_write_test.cc | 69 ++++++++++++++++++++++++++++++++++++
cpp/src/arrow/ipc/reader.cc | 4 ++-
3 files changed, 86 insertions(+), 1 deletion(-)
diff --git a/cpp/src/arrow/ipc/message.cc b/cpp/src/arrow/ipc/message.cc
index 4123a6b173..662b6bc373 100644
--- a/cpp/src/arrow/ipc/message.cc
+++ b/cpp/src/arrow/ipc/message.cc
@@ -1056,6 +1056,19 @@ class InputStreamMessageReader : public MessageReader,
public MessageDecoderList
Status OnMessageDecoded(std::unique_ptr<Message> message) override {
message_ = std::move(message);
+ first_message_ = false;
+ return Status::OK();
+ }
+
+ Status OnMetadata() override {
+ // An IPC file starts with "ARROW1". Read as a stream, its first four bytes
+ // ("ARRO") decode as a metadata length of 0x4F525241.
+ constexpr int64_t kArrowMagicAsMetadataLength = 0x4F525241;
+ if (first_message_ && decoder_.next_required_size() ==
kArrowMagicAsMetadataLength) {
+ return Status::Invalid(
+ "This appears to be an Arrow IPC file. "
+ "Try the IPC file reader instead of the IPC stream reader.");
+ }
return Status::OK();
}
@@ -1068,6 +1081,7 @@ class InputStreamMessageReader : public MessageReader,
public MessageDecoderList
io::InputStream* stream_;
std::shared_ptr<io::InputStream> owned_stream_;
std::unique_ptr<Message> message_;
+ bool first_message_ = true;
MessageDecoder decoder_;
};
diff --git a/cpp/src/arrow/ipc/read_write_test.cc
b/cpp/src/arrow/ipc/read_write_test.cc
index cfa6a541d5..139e07be05 100644
--- a/cpp/src/arrow/ipc/read_write_test.cc
+++ b/cpp/src/arrow/ipc/read_write_test.cc
@@ -2265,6 +2265,75 @@ TEST(TestRecordBatchStreamReader, MalformedInput) {
ASSERT_RAISES(Invalid, RecordBatchStreamReader::Open(&garbage_reader));
}
+TEST(TestRecordBatchStreamReader, OpenFileFormatSuggestsFileReader) {
+ std::shared_ptr<RecordBatch> batch;
+ ASSERT_OK(MakeIntRecordBatch(&batch));
+
+ FileWriterHelper helper;
+ ASSERT_OK(helper.Init(batch->schema(), IpcWriteOptions::Defaults()));
+ ASSERT_OK(helper.WriteBatch(batch));
+ ASSERT_OK(helper.Finish());
+
+ io::BufferReader reader(helper.buffer_);
+ // Check we mention using the file_reader when we detect file format
+ EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid,
+ ::testing::HasSubstr("Try the IPC file
reader"),
+ RecordBatchStreamReader::Open(&reader));
+}
+
+TEST(TestRecordBatchStreamReader, CorruptDataDoesNotSuggestFileReader) {
+ // Continuation marker + metadata_length = 100, then 8 bytes of non-magic
data.
+ const std::string corrupt(
+ "\xff\xff\xff\xff"
+ "\x64\x00\x00\x00"
+ "ABABABAB",
+ 16);
+ auto buffer = std::make_shared<Buffer>(corrupt);
+ io::BufferReader reader(buffer);
+ // Validate that we don't suggest file reader when file is just corrupt
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, ::testing::Not(::testing::HasSubstr("Try the IPC file reader")),
+ RecordBatchStreamReader::Open(&reader));
+}
+
+TEST(TestRecordBatchStreamReader,
FileMagicAfterFirstMessageDoesNotSuggestFileReader) {
+ std::shared_ptr<RecordBatch> batch;
+ ASSERT_OK(MakeIntRecordBatch(&batch));
+
+ StreamWriterHelper helper;
+ ASSERT_OK(helper.Init(batch->schema(), IpcWriteOptions::Defaults()));
+ ASSERT_OK(helper.WriteBatch(batch));
+ ASSERT_OK(helper.Finish());
+
+ // Replace the 8-byte marker with the IPC file magic.
+ std::string data = helper.buffer_->ToString();
+ data.resize(data.size() - 8);
+ data.append("ARROW1\0\0", 8);
+ io::BufferReader reader(Buffer::FromString(std::move(data)));
+
+ ASSERT_OK_AND_ASSIGN(auto stream_reader,
RecordBatchStreamReader::Open(&reader));
+ ASSERT_OK(stream_reader->Next());
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, ::testing::Not(::testing::HasSubstr("Try the IPC file reader")),
+ stream_reader->Next());
+}
+
+TEST(TestRecordBatchFileReader, OpenStreamFormatSuggestsStreamReader) {
+ std::shared_ptr<RecordBatch> batch;
+ ASSERT_OK(MakeIntRecordBatch(&batch));
+
+ StreamWriterHelper helper;
+ ASSERT_OK(helper.Init(batch->schema(), IpcWriteOptions::Defaults()));
+ ASSERT_OK(helper.WriteBatch(batch));
+ ASSERT_OK(helper.Finish());
+
+ auto buf_reader = std::make_shared<io::BufferReader>(helper.buffer_);
+ // Check we mention using the stream_reader when we detect stream format
+ EXPECT_RAISES_WITH_MESSAGE_THAT(
+ Invalid, ::testing::HasSubstr("use the IPC stream reader"),
+ RecordBatchFileReader::Open(buf_reader.get(), helper.buffer_->size()));
+}
+
class EndlessCollectListener : public CollectListener {
public:
EndlessCollectListener() : CollectListener(), decoder_(nullptr) {}
diff --git a/cpp/src/arrow/ipc/reader.cc b/cpp/src/arrow/ipc/reader.cc
index 25a8778330..96e281c323 100644
--- a/cpp/src/arrow/ipc/reader.cc
+++ b/cpp/src/arrow/ipc/reader.cc
@@ -1914,7 +1914,9 @@ class RecordBatchFileReaderImpl : public
RecordBatchFileReader {
const auto magic_start = buffer->data() + sizeof(int32_t);
if (std::string_view(reinterpret_cast<const char*>(magic_start),
kMagicSize) !=
kArrowMagicBytes) {
- return Status::Invalid("Not an Arrow file");
+ return Status::Invalid(
+ "Not an Arrow file. If this is an Arrow IPC stream, use "
+ "the IPC stream reader instead.");
}
int32_t footer_length = bit_util::FromLittleEndian(
