fix(arrow-ipc): return an error instead of panicking on malformed IPC schema

[bit2swaz]

Which issue does this PR close?

• Closes fb_to_schema panics on malformed IPC schema messages: unwrap() on optional FlatBuffer fields #10575.
• Closes arrow-ipc: FileReader panics (unimplemented!/panic!) on malformed-but-parseable schema instead of returning Err #10437.

Rationale for this change

arrow_ipc::convert::fb_to_schema is infallible and panics on schema messages that the flatbuffer verifier itself accepts. the first thing it does is fb.fields().unwrap(), and fields is optional in the flatbuffer schema, so a message with no fields aborts the process. get_data_type has the same problem deeper down: about 20 more panic!/unimplemented! and 30 unwrap() reachable from decoder input (unknown float precision, Type NONE, out of range enum tags, and so on).

every reader path funnels through fb_to_schema, so there was no way to read Arrow IPC without exposing the process to an abort on untrusted input. it also reaches parquet (an untrusted .parquet whose footer carries the ARROW:schema key) and arrow-flight. StreamReader::try_new returns Result, so malformed input should be an Err, not a process abort.

What changes are included in this PR?

• get_data_type (already pub(crate)) now returns Result<DataType, ArrowError>; every input reachable unwrap/panic!/unimplemented! is now a returned ParseError.
• new pub fn try_fb_to_schema and a private fallible field conversion. the readers in arrow-ipc, the parquet ARROW:schema path, and arrow-flight are switched onto it.
• fb_to_schema and the public From<crate::Field> impl keep their signatures for compatibility and delegate to the fallible path.

Are these changes tested?

yes. a schema message with no fields, one with an unknown float precision, and one with Type NONE now return Err instead of aborting, asserted by new tests in arrow-ipc. the flatbuffer verifier accepts all three, so they exercise the exact gap. existing round trip tests still pass.

Are there any user-facing changes?

no breaking changes. try_fb_to_schema is additive, and fb_to_schema still exists with the same signature.

[bit2swaz]

also, while migrating the readers i hit one related panic i left out of scope here: FileReaderBuilder still does footer.schema().unwrap() at reader.rs#L1252, so a malformed IPC file whose footer has no schema table still aborts

it predates this PR and sits on a different path from the fb_to_schema fix, so i kept it out to keep the diff focused

@alamb pinging you because i think you're the best person for this: do you think i should fold the fix into this PR, or file a separate issue?

[alamb]

@alamb pinging you because i think you're the best person for this: do you think i should fold the fix into this PR, or file a separate issue?

I recommend a separate issue / PR for ease of reviewing -- thank you @bit2swaz

[bit2swaz]

@Jefffrey took your other two notes, pushed

we can deprecate this function

the From<crate::Field> for Field impl cant take #[deprecated] cleanly since its a trait impl on a public conversion path. the real target is fb_to_schema, the panicking fn that try_fb_to_schema now replaces

do you want me to deprecate fb_to_schema? if so ill fold it into this PR since is just the attribute + fixing the in-tree callers

[bit2swaz]

btw, i just added "Closes #10437" to the PR desc

its a separate fuzzer report of the same get_data_type panic (Type NONE not supported) this PR already turns into an Err thats filed apart from #10575 and never linked, so yeah

[Jefffrey]

@Jefffrey took your other two notes, pushed

we can deprecate this function

the From<crate::Field> for Field impl cant take #[deprecated] cleanly since its a trait impl on a public conversion path. the real target is fb_to_schema, the panicking fn that try_fb_to_schema now replaces

do you want me to deprecate fb_to_schema? if so ill fold it into this PR since is just the attribute + fixing the in-tree callers

yes my comment was on fb_to_schema

[bit2swaz]

done and pushed. added #[deprecated(since = "59.2.0")] on fb_to_schema pointing at try_fb_to_schema and moved every in-tree caller (tests, benches, the ipc example, the flight integration client, and two doctests) over to try_fb_to_schema so the build stays clean under -D warnings