Skip to content

[security/yaml] Improve memory safety profile of Node accessors #99

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

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[security/yaml] Improve memory safety profile of Node accessors #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 10 additions & 10 deletions saphyr/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,20 @@ size_t Node::size() const { return node_.len(); }
bool Node::IsEmpty() const { return node_.is_empty(); }
Node::operator bool() const { return IsDefined(); }

NodeView Node::operator[](rs_std::StrRef key) const {
NodeView Node::operator[](rs_std::StrRef key) const& {
return NodeView(node_.get_at_key(key).value_or(rust::NodeView()));
}
NodeView Node::operator[](size_t index) const {
NodeView Node::operator[](size_t index) const& {
return NodeView(node_.get_at_index(index).value_or(rust::NodeView()));
}
std::optional<NodeView> Node::Get(size_t index) const {
std::optional<NodeView> Node::Get(size_t index) const& {
if (auto val = node_.get_at_index(index)) {
return std::make_optional(NodeView(*val));
}
return std::nullopt;
}

std::optional<NodeView> Node::Get(rs_std::StrRef key) const {
std::optional<NodeView> Node::Get(rs_std::StrRef key) const& {
if (auto val = node_.get_at_key(key)) {
return std::make_optional(NodeView(*val));
}
Expand All @@ -150,7 +150,7 @@ void Node::SetAtIndex(size_t index, rust::YamlOwned value) {
}

template <>
std::optional<int> Node::as_optional<int>() const {
std::optional<int> Node::as_optional<int>() const& {
auto v = node_.as_i64();
if (!v) return std::nullopt;
if (*v < std::numeric_limits<int>::min() ||
Expand All @@ -161,28 +161,28 @@ std::optional<int> Node::as_optional<int>() const {
}

template <>
std::optional<size_t> Node::as_optional<size_t>() const {
std::optional<size_t> Node::as_optional<size_t>() const& {
auto v = node_.as_i64();
if (!v || *v < 0) return std::nullopt;
return static_cast<size_t>(*v);
}

template <>
std::optional<bool> Node::as_optional<bool>() const {
std::optional<bool> Node::as_optional<bool>() const& {
return node_.as_bool();
}

template <>
std::optional<absl::string_view> Node::as_optional<absl::string_view>() const {
std::optional<absl::string_view> Node::as_optional<absl::string_view>() const& {
return node_.as_str();
}

template <>
std::optional<double> Node::as_optional<double>() const {
std::optional<double> Node::as_optional<double>() const& {
return node_.as_f64();
}

NodeView Node::as_view() const { return NodeView(node_.as_view()); }
NodeView Node::as_view() const& { return NodeView(node_.as_view()); }

absl::StatusOr<std::string> Dump(const Node& node) {
return node.as_view().Dump();
Expand Down
34 changes: 23 additions & 11 deletions saphyr/node.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,27 +211,36 @@ class Node {
// Accesses an element in a map by key.
// Returns a `NodeView` for which `IsDefined()` is false if the node is not a
// map or the key is not found.
NodeView operator[](rs_std::StrRef key) const ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView operator[](const char* key) const ABSL_ATTRIBUTE_LIFETIME_BOUND {
NodeView operator[](rs_std::StrRef key) const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView operator[](const char* key) const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
return operator[](rs_std::StrRef::FromUtf8Unchecked(key));
}
NodeView operator[](absl::string_view key) const
ABSL_ATTRIBUTE_LIFETIME_BOUND {
NodeView operator[](
absl::string_view key) const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
return operator[](rs_std::StrRef::FromUtf8Unchecked(key));
}
NodeView operator[](rs_std::StrRef key) const&& = delete;
NodeView operator[](const char* key) const&& = delete;
NodeView operator[](absl::string_view key) const&& = delete;

// Accesses an element in a sequence by index.
// Returns a `NodeView` for which `IsDefined()` is false if the node is not a
// sequence or the index is out of bounds.
NodeView operator[](size_t index) const ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView operator[](int index) const ABSL_ATTRIBUTE_LIFETIME_BOUND {
NodeView operator[](size_t index) const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView operator[](int index) const& ABSL_ATTRIBUTE_LIFETIME_BOUND {
return index < 0 ? NodeView() : operator[](static_cast<size_t>(index));
}
NodeView operator[](size_t index) const&& = delete;
NodeView operator[](int index) const&& = delete;

// A safer version of `operator[]` for sequence.
std::optional<NodeView> Get(size_t index) const ABSL_ATTRIBUTE_LIFETIME_BOUND;
std::optional<NodeView> Get(
size_t index) const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
// A safer version of `operator[]` for map.
std::optional<NodeView> Get(rs_std::StrRef key) const
ABSL_ATTRIBUTE_LIFETIME_BOUND;
std::optional<NodeView> Get(
rs_std::StrRef key) const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
std::optional<NodeView> Get(size_t index) const&& = delete;
std::optional<NodeView> Get(rs_std::StrRef key) const&& = delete;

// Sets the value at a given index in a sequence.
// If `index` is within bounds, the existing value is replaced.
Expand All @@ -242,10 +251,13 @@ class Node {
// Tries to convert the node to the specified type `T`.
// Returns `std::nullopt` if the conversion fails or the node is undefined.
template <typename T>
std::optional<T> as_optional() const;
std::optional<T> as_optional() const&;
template <typename T>
std::optional<T> as_optional() const&& = delete;

// Returns a view into the node.
NodeView as_view() const ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView as_view() const& ABSL_ATTRIBUTE_LIFETIME_BOUND;
NodeView as_view() const&& = delete;

private:
rust::NodeOwned node_;
Expand Down
Loading