Skip to content

Commit 500e708

Browse files
Check size of non empty domain before access (#5650)
During schema deserialization, when deserializing the `non_empty_domain` the code depends on the existence of the `sizes` list to decide whether or not the range is of variable length and access the array without checking its size. To make the deserialization more robust and because the serialization process is not controlled by core (for example an already serialized schema may be received from a client during a REST call), the size of the `sizes` list should also be checked before accessing the list. --- TYPE: BUG DESC: Check `non_empty_domain.sizes` size before access
1 parent 27e51be commit 500e708

File tree

2 files changed

+57
-1
lines changed

2 files changed

+57
-1
lines changed

tiledb/sm/serialization/capnp_utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ std::pair<Status, std::optional<NDRange>> deserialize_non_empty_domain_rv(
511511
vec[index] = list[index];
512512
}
513513

514-
if (non_empty_domain.hasSizes()) {
514+
if (non_empty_domain.hasSizes() && non_empty_domain.getSizes().size()) {
515515
auto sizes = non_empty_domain.getSizes();
516516
ndRange.emplace_back(vec.data(), vec.size(), sizes[0]);
517517
} else {

tiledb/sm/serialization/test/unit_capnp_nonempty_domain.cc

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,59 @@ TEST_CASE(
7070
REQUIRE(status.ok());
7171
REQUIRE(clone.value()[0].var_size() == false);
7272
}
73+
74+
TEST_CASE(
75+
"Check serialization correctly handles empty sizes",
76+
"[nonemptydomain][serialization]") {
77+
auto dim = make_shared<Dimension>(
78+
HERE(),
79+
"index",
80+
Datatype::UINT32,
81+
tiledb::test::get_test_memory_tracker());
82+
uint32_t domain1[2]{1, 64};
83+
dim->set_domain(&domain1[0]);
84+
85+
NDRange nd_list = {dim->domain()};
86+
87+
// Serialize
88+
::capnp::MallocMessageBuilder message;
89+
tiledb::sm::serialization::capnp::NonEmptyDomainList::Builder builder =
90+
message.initRoot<tiledb::sm::serialization::capnp::NonEmptyDomainList>();
91+
Status st;
92+
93+
auto nonEmptyDomainListBuilder = builder.initNonEmptyDomains(1);
94+
95+
for (uint64_t dimIdx = 0; dimIdx < nd_list.size(); ++dimIdx) {
96+
const auto& dimNonEmptyDomain = nd_list[dimIdx];
97+
98+
auto dim_builder = nonEmptyDomainListBuilder[dimIdx];
99+
dim_builder.setIsEmpty(dimNonEmptyDomain.empty());
100+
101+
if (!dimNonEmptyDomain.empty()) {
102+
auto subarray_builder = dim_builder.initNonEmptyDomain();
103+
st = tiledb::sm::serialization::utils::set_capnp_array_ptr(
104+
subarray_builder,
105+
tiledb::sm::Datatype::UINT8,
106+
dimNonEmptyDomain.data(),
107+
dimNonEmptyDomain.size());
108+
109+
if (dimNonEmptyDomain.start_size() != 0) {
110+
// start_size() is non-zero for var-size dimensions
111+
auto range_start_sizes = dim_builder.initSizes(1);
112+
range_start_sizes.set(0, dimNonEmptyDomain.start_size());
113+
} else {
114+
// sizes can be empty for non var-size dimensions
115+
dim_builder.initSizes(0);
116+
}
117+
}
118+
}
119+
120+
REQUIRE(st.ok());
121+
122+
auto&& [status, clone] =
123+
tiledb::sm::serialization::utils::deserialize_non_empty_domain_rv(
124+
builder);
125+
126+
REQUIRE(status.ok());
127+
REQUIRE(clone.value()[0].var_size() == false);
128+
}

0 commit comments

Comments
 (0)