8000 fix UB in DistributeExecutor by jsteemann · Pull Request #14774 · arangodb/arangodb · GitHub
[go: up one dir, main page]
Skip to content

fix UB in DistributeExecutor #14774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 15, 2021
Merged

fix UB in DistributeExecutor #14774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
261225e
fix UB in DistributeExecutor
jsteemann Sep 14, 2021
036f76e
Merge branch 'devel' of github.com:arangodb/arangodb into bug-fix/fix…
jsteemann Sep 15, 2021
393b77f
remove comment
jsteemann Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 30 additions & 34 deletions arangod/Aql/DistributeExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,24 +125,37 @@ auto DistributeExecutor::distributeBlock(SharedAqlItemBlockPtr const& block, Ski
choosenMap[key].emplace_back(i);
}
} else {
auto input = extractInput(block, i);
if (!input.isNone()) {
// NONE is ignored.
// Object is processd
// All others throw.
TRI_ASSERT(input.isObject());
if (_infos.shouldDistributeToAll(input)) {
// This input should be added to all clients
for (auto const& [key, value] : blockMap) {
choosenMap[key].emplace_back(i);
}
} else {
auto client = getClient(input);
if (!client.empty()) {
// We can only have clients we are prepared for
TRI_ASSERT(blockMap.find(client) != blockMap.end());
choosenMap[client].emplace_back(i);
// check first input register
AqlValue val = InputAqlItemRow{block, i}.getValue(_infos.registerId());

VPackSlice input = val.slice(); // will throw when wrong type
if (input.isNone()) {
continue;
}

if (!input.isObject()) {
THROW_ARANGO_EXCEPTION_MESSAGE(
TRI_ERROR_INTERNAL, "DistributeExecutor requires an object as input");
}
// NONE is ignored.
// Object is processd
// All others throw.
TRI_ASSERT(input.isObject());
if (_infos.shouldDistributeToAll(input)) {
// This input should be added to all clients
for (auto const& [key, value] : blockMap) {
choosenMap[key].emplace_back(i);
}
} else {
auto client = getClient(input);
if (!client.empty()) {
// We can only have clients we are prepared for
TRI_ASSERT(blockMap.find(client) != blockMap.end());
if (ADB_UNLIKELY(blockMap.find(client) == blockMap.end())) {
THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL,
std::string("unexpected client id '") + client + "' found in blockMap");
}
choosenMap[client].emplace_back(i);
}
}
}
Expand All @@ -168,23 +181,6 @@ auto DistributeExecutor::distributeBlock(SharedAqlItemBlockPtr const& block, Ski
}
}

auto DistributeExecutor::extractInput(SharedAqlItemBlockPtr const& block,
size_t rowIndex) const -> VPackSlice {
// check first input register
AqlValue val = InputAqlItemRow{block, rowIndex}.getValue(_infos.registerId());

VPackSlice input = val.slice(); // will throw when wrong type
if (input.isNone()) {
return input;
}

if (!input.isObject()) {
THROW_ARANGO_EXCEPTION_MESSAGE(
TRI_ERROR_INTERNAL, "DistributeExecutor requires an object as input");
}
return input;
}

auto DistributeExecutor::getClient(VPackSlice input) const -> std::string {
auto res = _infos.getResponsibleClient(input);
if (res.fail()) {
Expand Down
9 changes: 0 additions & 9 deletions arangod/Aql/DistributeExecutor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,17 +97,8 @@ class DistributeExecutor {
std::unordered_map<std::string, ClientBlockData>& blockMap) -> void;

private:
/**
* @brief Compute which client needs to get this row
* @param block The input block
* @param rowIndex
* @return std::string Identifier used by the client
*/
auto extractInput(SharedAqlItemBlockPtr const& block, size_t rowIndex) const
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment needs to be removed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

-> velocypack::Slice;
auto getClient(velocypack::Slice input) const -> std::string;

private:
DistributeExecutorInfos const& _infos;

// a reusable Builder object for building _key values
Expand Down
0