Skip to content

Commit 91f6618

Browse files
authored
GH-48238: [C++] Actually write IPC schema endianness, not host endianness (#48239)
### Rationale for this change `Schema` objects have an endianness, but we were ignoring it when serializing a `Schema` to IPC, and instead writing out the host's endianness. ### Are these changes tested? Yes, by additional test. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.) * GitHub Issue: #48238 Authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
1 parent 6f62c2a commit 91f6618

File tree

2 files changed

+30
-15
lines changed

2 files changed

+30
-15
lines changed

cpp/src/arrow/ipc/message_internal_test.cc

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "arrow/ipc/dictionary.h"
2424
#include "arrow/ipc/metadata_internal.h"
2525
#include "arrow/ipc/options.h"
26+
#include "arrow/ipc/reader.h"
2627
#include "arrow/testing/gtest_util.h"
2728
#include "arrow/util/key_value_metadata.h"
2829

@@ -38,8 +39,6 @@ using FBB = flatbuffers::FlatBufferBuilder;
3839
// lead to unnecessary platform- or toolchain-specific differences in
3940
// serialization.
4041
TEST(TestMessageInternal, TestByteIdentical) {
41-
FBB fbb;
42-
flatbuffers::Offset<org::apache::arrow::flatbuf::Schema> fb_schema;
4342
DictionaryFieldMapper mapper;
4443

4544
// Create a simple Schema with just two metadata KVPs
@@ -78,4 +77,29 @@ TEST(TestMessageInternal, TestByteIdentical) {
7877

7978
AssertBufferEqual(expected_buffer, *out_buffer);
8079
}
80+
81+
TEST(TestMessageInternal, TestEndiannessRoundtrip) {
82+
DictionaryFieldMapper mapper;
83+
84+
for (const auto endianness : {Endianness::Little, Endianness::Big}) {
85+
// Create a simple Schema with just two metadata KVPs
86+
auto f0 = field("f0", int64());
87+
auto f1 = field("f1", int64());
88+
std::vector<std::shared_ptr<Field>> fields = {f0, f1};
89+
std::shared_ptr<KeyValueMetadata> metadata =
90+
KeyValueMetadata::Make({"key_1", "key_2"}, {"key_1_value", "key_2_value"});
91+
auto schema = ::arrow::schema({f0}, endianness, metadata);
92+
93+
// Serialize the Schema to a Buffer
94+
std::shared_ptr<Buffer> out_buffer;
95+
ASSERT_OK(
96+
WriteSchemaMessage(*schema, mapper, IpcWriteOptions::Defaults(), &out_buffer));
97+
98+
// Re-open to a new Message and parse Schema
99+
ASSERT_OK_AND_ASSIGN(auto message, Message::Open(out_buffer, /*body=*/nullptr));
100+
ASSERT_OK_AND_ASSIGN(auto parsed_schema, ReadSchema(*message, nullptr));
101+
AssertSchemaEqual(*schema, *parsed_schema, /*check_metadata=*/true);
102+
}
103+
}
104+
81105
} // namespace arrow::ipc::internal

cpp/src/arrow/ipc/metadata_internal.cc

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -933,18 +933,6 @@ Status FieldFromFlatbuffer(const flatbuf::Field* field, FieldPosition field_pos,
933933
return Status::OK();
934934
}
935935

936-
// will return the endianness of the system we are running on
937-
// based the NUMPY_API function. See NOTICE.txt
938-
flatbuf::Endianness endianness() {
939-
union {
940-
uint32_t i;
941-
char c[4];
942-
} bint = {0x01020304};
943-
944-
return bint.c[0] == 1 ? flatbuf::Endianness::Endianness_Big
945-
: flatbuf::Endianness::Endianness_Little;
946-
}
947-
948936
flatbuffers::Offset<KVVector> SerializeCustomMetadata(
949937
FBB& fbb, const std::shared_ptr<const KeyValueMetadata>& metadata) {
950938
std::vector<KeyValueOffset> key_values;
@@ -970,7 +958,10 @@ Status SchemaToFlatbuffer(FBB& fbb, const Schema& schema,
970958
}
971959

972960
auto fb_offsets = fbb.CreateVector(field_offsets);
973-
*out = flatbuf::CreateSchema(fbb, endianness(), fb_offsets,
961+
auto fb_endianness = schema.endianness() == Endianness::Little
962+
? flatbuf::Endianness::Endianness_Little
963+
: flatbuf::Endianness::Endianness_Big;
964+
*out = flatbuf::CreateSchema(fbb, fb_endianness, fb_offsets,
974965
SerializeCustomMetadata(fbb, schema.metadata()));
975966
return Status::OK();
976967
}

0 commit comments

Comments
 (0)