GH-33823: [C++][IPC] Improve error messages when opening files that are the wrong format by RobertLD · Pull Request #49771 · apache/arrow

RobertLD · 2026-04-16T12:35:11Z

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.

PR review suggestions applied:

The file-magic detection in DecodeMessage now compares the decoded metadata length against a constant derived from FromLittleEndian, hopefully avoiding any little/big endianness issues
The magic check now runs before the large file->Read(metadata_length) call, avoiding the 1.3 GB read when an IPC file is fed to the stream reader.
Rewording error message again
Test substring matches use more specific strings

Are these changes tested?

Yes. New tests were added to OpenFileSuggestsFileReader

CorruptDataDoesNotSuggestFileReader
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: [C++] Improve error message when reading Streaming file with File reader and vice versa #33823

github-actions · 2026-04-16T12:35:37Z

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

Pull request overview

This PR improves user-facing error messages in the C++ IPC readers/decoders when callers attempt to open an IPC File as a Stream (or vice versa), providing more actionable guidance aligned with GH-33823.

Changes:

Update the IPC file reader footer/magic check failure message to suggest the streaming reader when appropriate.
Add a heuristic in the IPC stream message decoder to detect an IPC file magic prefix being misinterpreted as a stream message length, and emit a more instructive error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`cpp/src/arrow/ipc/reader.cc`	Improves the “not an Arrow file” error when the footer magic doesn’t match, suggesting the streaming reader.
`cpp/src/arrow/ipc/message.cc`	Adds detection for IPC File magic bytes when decoding a stream message and returns a more targeted “wrong format” error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RobertLD · 2026-04-19T22:55:02Z

Decent feedback Mr. Bot, I'll make changes shortly. Honestly impressed the LLM review had such good context. Good bot

Create more generic message instead of referencing c++ interface Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Similarly, keep messages more generic and not referencing the c++ interface Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

RobertLD · 2026-04-24T18:13:22Z

@kou Comments addressed

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kou · 2026-04-28T03:02:03Z

Could you add tests for these cases?

We want to ensure we're providing advice to the user that is correct/actionable.

RobertLD · 2026-04-29T22:57:56Z

Added a pos/neg case, kept the string matching minimal so the tests weren't as flakey is the messages get reworked in another way in the future. If it's standard practice to match the entire err message I can make that change over a subset match @kou

pitrou · 2026-05-07T13:13:43Z

Thanks for the PR @RobertLD . This is a good idea. In the future I think it would be more convenient if IPC files could read using the IPC stream reader (*), at least with a dedicated option enabled; but this PR is a good start.

(*) This was discussed a bit on the dev ML: https://lists.apache.org/thread/jpxl3yzm96wkxzb1clokxklsy32b3plh

pitrou · 2026-05-07T13:16:07Z

+        std::string_view(reinterpret_cast<const char*>(metadata->data()),
+                         remaining_magic.size()) == remaining_magic) {


This would only work on a little-endian machine. We need to be endianness-agnostic (see ConsumeInitialData).

pitrou · 2026-05-07T13:19:20Z

  ARROW_ASSIGN_OR_RAISE(auto metadata, file->Read(metadata_length));
  if (metadata->size() != metadata_length) {
+    // The first sizeof(int32_t) bytes of the Arrow file magic ("ARRO") may have been
+    // misread as metadata_length. Check if the remaining bytes complete the magic.


How likely is it to have a metadata message exactly 1330795073 bytes long ("ARRO" decoded as a 32-bit little-endian integer)? Perhaps we can check up front instead of trying to read so much data?

cc @lidavidm @paleolimbot for opinions.

Yeah, a gigabyte of metadata raises flags anyways

In nanoarrow we check the first few bytes for the magic string and skip them (then attempt to read the rest of the input as an IPC stream). We've never run into a complaint about this not working but I'm not sure how widespread the usage is (we could add an option to turn it off or improve the error that occurs if we run into one). I think 1330795073 bytes of metadata would never reasonably occur on purpose.

I moved the check to happen first, and avoid the large read if possible

pitrou · 2026-05-07T13:23:51Z

+
+  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("file reader"),


Can we be a bit more specific and match a longer substring? For example "Try the IPC file reader".

pitrou · 2026-05-07T13:24:06Z

+  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("stream reader"),


pitrou · 2026-05-07T13:25:07Z

Also can you make the PR description a bit more specific @RobertLD ?

RobertLD · 2026-05-07T16:15:41Z

I’ll circle back in a day or two and apply these suggestions. Thanks for the feedback

…

On Thu, May 7, 2026 at 09:45 David Li ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In cpp/src/arrow/ipc/message.cc <#49771 (comment)>: > @@ -565,6 +565,17 @@ Status DecodeMessage(MessageDecoder* decoder, io::InputStream* file) { auto metadata_length = decoder->next_required_size(); ARROW_ASSIGN_OR_RAISE(auto metadata, file->Read(metadata_length)); if (metadata->size() != metadata_length) { + // The first sizeof(int32_t) bytes of the Arrow file magic ("ARRO") may have been + // misread as metadata_length. Check if the remaining bytes complete the magic. Yeah, a gigabyte of *metadata* raises flags anyways — Reply to this email directly, view it on GitHub <#49771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHGZJYRWPFX5H5YEX4XHAM34ZSHO3AVCNFSM6AAAAACX3RU3WGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DENBUGQ4TKMZQGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Fix endianness bug, avoid huge metadata reads and alter err messages

RobertLD · 2026-05-12T16:04:53Z

Also can you make the PR description a bit more specific @RobertLD ?

Addressed this and other PR suggestions with my latest commit
d01cb5d

pitrou · 2026-05-13T12:42:27Z

+    if (peek->size() >= static_cast<int64_t>(kRemainingMagic.size()) &&
+        std::string_view(reinterpret_cast<const char*>(peek->data()),
+                         kRemainingMagic.size()) == kRemainingMagic) {


What happens if this check fails? Are we assuming that the file is a valid IPC stream that happens to have a metadata size exactly 0x4F525241 bytes?

In any case, we have read kRemainingMagic bytes that are discarded below, so we'll be out of sync anyway.

@pitrou Okay I see what you're saying and this was an oversight on my part. My idea is to continue to let it fall through and add a separate check immediately below it to reject metadata over 1GB. I think this is the best solution but it does change the behavior of the library (and I'm concerned there may be a single user somewhere who has 10Gb of metadata) and it's a bit of stretch scope wise considering this ticket is about error messages.

What are your thoughts? I'm thinking I just go forward with the above (or alternatively, just throw a failure with a different message on specifically 0x4F525241 even if it's not IPC)

Letting it fall through means a mythical user with a valid 0x4F525241 bytes metadata block will get a regression with a weird message decoding the Flatbuffers metadata. I'd rather an explicit message.

RobertLD · 2026-05-26T15:24:31Z

I will address this last comment shortly, but am on vacation. Just pinging this so it doesn't get closed as stale

pitrou · 2026-06-16T14:53:41Z

I'm thinking of another problem: this will detect the IPC file marker for every metadata message, but we actually only want to do that at the start of the stream. State::INITIAL would not be enough to detect that, because we go back to INITIAL after each successfully decoded message.

Perhaps DecodeMessage is not the right place for this and this should be done at a higher level in the call tree?

github-actions Bot added Component: C++ awaiting review Awaiting review labels Apr 16, 2026

RobertLD changed the title ~~#33823 Improve err msgs when opening files that are the wrong format~~ GH-33823: [C++] Improve err msgs when opening files that are the wrong format Apr 16, 2026

apache#33823 Improve err msgs when opening files that are the wrong f…

abd2b9c

…ormat

RobertLD force-pushed the 33823-improve-err-msg-for-file-reader branch from d7f5423 to abd2b9c Compare April 16, 2026 12:48

33823 Formatting + linting

db7dd63

RobertLD marked this pull request as ready for review April 16, 2026 14:04

kou requested a review from Copilot April 19, 2026 21:12

Copilot started reviewing on behalf of kou April 19, 2026 21:12 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

Comment thread cpp/src/arrow/ipc/reader.cc Outdated

Comment thread cpp/src/arrow/ipc/message.cc Outdated

RobertLD and others added 2 commits April 24, 2026 10:41

Update cpp/src/arrow/ipc/reader.cc

d025ab0

Create more generic message instead of referencing c++ interface Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update cpp/src/arrow/ipc/message.cc

1d9b536

Similarly, keep messages more generic and not referencing the c++ interface Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

kou requested a review from Copilot April 28, 2026 02:56

kou changed the title ~~GH-33823: [C++] Improve err msgs when opening files that are the wrong format~~ GH-33823: [C++] Improve error messages when opening files that are the wrong format Apr 28, 2026

Copilot started reviewing on behalf of kou April 28, 2026 02:57 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

33823 Add tests to verify new err msgs

407c134

We want to ensure we're providing advice to the user that is correct/actionable.

pitrou reviewed May 7, 2026

View reviewed changes

pitrou changed the title ~~GH-33823: [C++] Improve error messages when opening files that are the wrong format~~ GH-33823: [C++][IPC] Improve error messages when opening files that are the wrong format May 7, 2026

github-actions Bot added awaiting committer review Awaiting committer review awaiting changes Awaiting changes and removed awaiting review Awaiting review awaiting committer review Awaiting committer review labels May 7, 2026

33823 PR suggestions

d01cb5d

Fix endianness bug, avoid huge metadata reads and alter err messages

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels May 12, 2026

pitrou reviewed May 13, 2026

View reviewed changes

		std::string_view(reinterpret_cast<const char*>(metadata->data()),
		remaining_magic.size()) == remaining_magic) {

Uh oh!

Conversation

RobertLD commented Apr 16, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

RobertLD commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RobertLD commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kou commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RobertLD commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pitrou commented May 7, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pitrou commented May 7, 2026

Uh oh!

RobertLD commented May 7, 2026 via email

Uh oh!

RobertLD commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RobertLD commented May 26, 2026

Uh oh!

pitrou commented Jun 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RobertLD commented Apr 16, 2026 •

edited by github-actions Bot

Loading

RobertLD commented Apr 19, 2026 •

edited

Loading

kou commented Apr 28, 2026 •

edited

Loading

RobertLD commented Apr 29, 2026 •

edited

Loading