Skip to content

Commit 4693ccd

Browse files
ActuallyaDeviloperActuallyaDeviloper
committed
Avoid assuming that "std::string_view" is nullterminated. Generally it is really not.
Also avoid recomputing the string length for APIs that allow that.
1 parent 49ac063 commit 4693ccd

File tree

11 files changed

+41
-36
lines changed

11 files changed

+41
-36
lines changed

‎include/rfl/avro/Writer.hpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ class RFL_API Writer {
147147
OutputVarType add_value_to_map(const std::string_view& _name, const T& _var,
148148
OutputMapType* _parent) const {
149149
avro_value_t new_value;
150-
int result = avro_value_add(&_parent->val_, _name.data(), &new_value,
150+
// Copy name to nullterminate.
151+
int result = avro_value_add(&_parent->val_, std::string(_name).c_str(), &new_value,
151152
nullptr, nullptr);
152153
if (result != 0) {
153154
throw std::runtime_error("Error adding value to map: error(" +
@@ -163,7 +164,8 @@ class RFL_API Writer {
163164
const T& _var,
164165
OutputObjectType* _parent) const {
165166
avro_value_t new_value;
166-
int result = avro_value_get_by_name(&_parent->val_, _name.data(),
167+
// Copy name to nullterminate.
168+
int result = avro_value_get_by_name(&_parent->val_, std::string(_name).c_str(),
167169
&new_value, nullptr);
168170
if (result != 0) {
169171
throw std::runtime_error("Error adding value to object: error(" +

‎include/rfl/capnproto/Reader.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,8 @@ class RFL_API Reader {
165165
const auto entries = _map.val_.get("entries").as<capnp::DynamicList>();
166166
for (auto entry : entries) {
167167
auto s = entry.template as<capnp::DynamicStruct>();
168-
const char* key = s.get("key").as<capnp::Text>().cStr();
169-
_map_reader.read(std::string_view(key), InputVarType{s.get("value")});
168+
const auto key = s.get("key").as<capnp::Text>();
169+
_map_reader.read(std::string_view(key.cStr(), key.size()), InputVarType{s.get("value")});
170170
}
171171
return std::nullopt;
172172
} catch (std::exception& e) {

‎include/rfl/capnproto/Writer.hpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ class RFL_API Writer {
193193
auto entries =
194194
OutputArrayType{_parent->val_.get("entries").as<capnp::DynamicList>()};
195195
auto new_entry = add_object_to_array(2, &entries);
196-
add_value_to_object("key", std::string(_name), &new_entry);
196+
new_entry->val_.set("key", { _name.data(), _name.size() });
197197
return add_value_to_object("value", _var, &new_entry);
198198
}
199199

@@ -202,20 +202,20 @@ class RFL_API Writer {
202202
const T& _var,
203203
OutputObjectType* _parent) const {
204204
if constexpr (std::is_same<std::remove_cvref_t<T>, std::string>()) {
205-
_parent->val_.set(_name.data(), _var.c_str());
205+
_parent->val_.set({ _name.data(), _name.size() }, { _var.c_str(), _var.size() });
206206

207207
} else if constexpr (std::is_same<std::remove_cvref_t<T>,
208208
rfl::Bytestring>()) {
209209
const auto array_ptr = kj::ArrayPtr<const kj::byte>(
210210
internal::ptr_cast<const unsigned char*>(_var.data()), _var.size());
211-
_parent->val_.set(_name.data(), capnp::Data::Reader(array_ptr));
211+
_parent->val_.set({ _name.data(), _name.size() }, capnp::Data::Reader(array_ptr));
212212

213213
} else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>() ||
214214
std::is_same<std::remove_cvref_t<T>, bool>()) {
215-
_parent->val_.set(_name.data(), _var);
215+
_parent->val_.set({ _name.data(), _name.size() }, _var);
216216

217217
} else if constexpr (std::is_integral<std::remove_cvref_t<T>>()) {
218-
_parent->val_.set(_name.data(), static_cast<std::int64_t>(_var));
218+
_parent->val_.set({ _name.data(), _name.size() }, static_cast<std::int64_t>(_var));
219219

220220
} else if constexpr (internal::is_literal_v<T>) {
221221
return add_value_to_object(_name, _var.value(), _parent);

‎include/rfl/json/Reader.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ struct Reader {
8888
yyjson_obj_iter_init(_obj.val_, &iter);
8989
yyjson_val* key;
9090
while ((key = yyjson_obj_iter_next(&iter))) {
91-
const auto name = std::string_view(yyjson_get_str(key));
91+
const auto name = std::string_view(yyjson_get_str(key), yyjson_get_len(key));
9292
_object_reader.read(name, InputVarType(yyjson_obj_iter_get_val(key)));
9393
}
9494
return std::nullopt;
@@ -101,7 +101,7 @@ struct Reader {
101101
if (r == NULL) {
102102
return error("Could not cast to string.");
103103
}
104-
return std::string(r);
104+
return std::string(r, yyjson_get_len(_var.val_));
105105

106106
} else if constexpr (std::is_same<std::remove_cvref_t<T>, bool>()) {
107107
if (!yyjson_is_bool(_var.val_)) {

‎include/rfl/json/Writer.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class RFL_API Writer {
9696
OutputObjectType* _parent) const {
9797
const auto val = from_basic_type(_var);
9898
const bool ok = yyjson_mut_obj_add(
99-
_parent->val_, yyjson_mut_strcpy(doc(), _name.data()), val.val_);
99+
_parent->val_, yyjson_mut_strncpy(doc(), _name.data(), _name.size()), val.val_);
100100
if (!ok) {
101101
throw std::runtime_error("Could not add field '" + std::string(_name) +
102102
"' to object.");
@@ -117,7 +117,7 @@ class RFL_API Writer {
117117
template <class T>
118118
OutputVarType from_basic_type(const T& _var) const noexcept {
119119
if constexpr (std::is_same<std::remove_cvref_t<T>, std::string>()) {
120-
return OutputVarType(yyjson_mut_strcpy(doc(), _var.c_str()));
120+
return OutputVarType(yyjson_mut_strncpy(doc(), _var.c_str(), _var.size()));
121121

122122
} else if constexpr (std::is_same<std::remove_cvref_t<T>, bool>()) {
123123
return OutputVarType(yyjson_mut_bool(doc(), _var));

‎include/rfl/xml/Writer.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ struct RFL_API Writer {
9393

9494
private:
9595
template <class T>
96-
std::string to_string(const T& _val) const {
96+
decltype(auto) to_string(const T& _val) const {
9797
if constexpr (std::is_same<std::remove_cvref_t<T>, std::string>()) {
98-
return _val;
98+
return _val; // Return reference to avoid expensive string copy.
9999
} else if constexpr (std::is_same<std::remove_cvref_t<T>, bool>()) {
100100
return _val ? "true" : "false";
101101
} else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>() ||

‎include/rfl/yaml/Writer.hpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ class RFL_API Writer {
8585
std::is_same<std::remove_cvref_t<T>, bool>() ||
8686
std::is_same<std::remove_cvref_t<T>,
8787
std::remove_cvref_t<decltype(YAML::Null)>>()) {
88-
(*out_) << YAML::Key << _name.data() << YAML::Value << _var;
88+
(*out_) << YAML::Key << _name << YAML::Value << _var;
8989
} else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>()) {
9090
// std::to_string is necessary to ensure that floating point values are
9191
// always written as floats.
92-
(*out_) << YAML::Key << _name.data() << YAML::Value
92+
(*out_) << YAML::Key << _name << YAML::Value
9393
<< std::to_string(_var);
9494
} else if constexpr (std::is_integral<std::remove_cvref_t<T>>()) {
95-
(*out_) << YAML::Key << _name.data() << YAML::Value
95+
(*out_) << YAML::Key << _name << YAML::Value
9696
<< static_cast<int64_t>(_var);
9797
} else {
9898
static_assert(rfl::always_false_v<T>, "Unsupported type.");

‎src/rfl/capnproto/Writer.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ Writer::OutputArrayType Writer::add_array_to_map(const std::string_view& _name,
4747
Writer::OutputArrayType Writer::add_array_to_object(
4848
const std::string_view& _name, const size_t _size,
4949
OutputObjectType* _parent) const {
50-
return OutputArrayType{_parent->val_.init(_name.data(), SafeSizeToUInt(_size))
50+
return OutputArrayType{_parent->val_.init({ _name.data(), _name.size() }, SafeSizeToUInt(_size))
5151
.as<capnp::DynamicList>()};
5252
}
5353

@@ -111,7 +111,7 @@ Writer::OutputObjectType Writer::add_object_to_object(
111111
const std::string_view& _name, const size_t /*_size*/,
112112
OutputObjectType* _parent) const {
113113
return OutputObjectType{
114-
_parent->val_.get(_name.data()).as<capnp::DynamicStruct>()};
114+
_parent->val_.get({ _name.data(), _name.size() }).as<capnp::DynamicStruct>()};
115115
}
116116

117117
Writer::OutputObjectType Writer::add_object_to_union(
@@ -142,7 +142,7 @@ Writer::OutputUnionType Writer::add_union_to_map(const std::string_view& _name,
142142
Writer::OutputUnionType Writer::add_union_to_object(
143143
const std::string_view& _name, OutputObjectType* _parent) const {
144144
return OutputUnionType{
145-
_parent->val_.get(_name.data()).as<capnp::DynamicStruct>()};
145+
_parent->val_.get({ _name.data(), _name.size() }).as<capnp::DynamicStruct>()};
146146
}
147147

148148
Writer::OutputUnionType Writer::add_union_to_union(
@@ -171,7 +171,7 @@ Writer::OutputVarType Writer::add_null_to_map(const std::string_view& _name,
171171

172172
Writer::OutputVarType Writer::add_null_to_object(
173173
const std::string_view& _name, OutputObjectType* _parent) const {
174-
_parent->val_.set(_name.data(), capnp::VOID);
174+
_parent->val_.set({ _name.data(), _name.size() }, capnp::VOID);
175175
return OutputVarType{};
176176
}
177177

‎src/rfl/flexbuf/Writer.cpp‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ Writer::OutputVarType Writer::add_null_to_array(
5151

5252
Writer::OutputVarType Writer::add_null_to_object(
5353
const std::string_view& _name, OutputObjectType* /*_parent*/) const {
54-
fbb_->Null(_name.data());
54+
// Copy the string, because "std::string_view" might not be nullterminated.
55+
fbb_->Null(std::string(_name).c_str());
5556
return OutputVarType{};
5657
}
5758

@@ -64,7 +65,8 @@ void Writer::end_object(OutputObjectType* _obj) const {
6465
}
6566

6667
Writer::OutputArrayType Writer::new_array(const std::string_view& _name) const {
67-
const auto start = fbb_->StartVector(_name.data());
68+
// Copy the string, because "std::string_view" might not be nullterminated.
69+
const auto start = fbb_->StartVector(std::string(_name).c_str());
6870
return OutputArrayType{start};
6971
}
7072

@@ -75,7 +77,8 @@ Writer::OutputArrayType Writer::new_array() const {
7577

7678
Writer::OutputObjectType Writer::new_object(
7779
const std::string_view& _name) const {
78-
const auto start = fbb_->StartMap(_name.data());
80+
// Copy the string, because "std::string_view" might not be nullterminated.
81+
const auto start = fbb_->StartMap(std::string(_name).c_str());
7982
return OutputObjectType{start};
8083
}
8184

‎src/rfl/xml/Writer.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,27 @@ Writer::~Writer() = default;
3939

4040
Writer::OutputArrayType Writer::array_as_root(const size_t /*_size*/) const {
4141
auto node_child =
42-
Ref<pugi::xml_node>::make(root_->append_child(root_name_.c_str()));
42+
Ref<pugi::xml_node>::make(root_->append_child(root_name_));
4343
return OutputArrayType(root_name_, node_child);
4444
}
4545

4646
Writer::OutputObjectType Writer::object_as_root(const size_t /*_size*/) const {
4747
auto node_child =
48-
Ref<pugi::xml_node>::make(root_->append_child(root_name_.c_str()));
48+
Ref<pugi::xml_node>::make(root_->append_child(root_name_));
4949
return OutputObjectType(node_child);
5050
}
5151

5252
Writer::OutputVarType Writer::null_as_root() const {
5353
auto node_child =
54-
Ref<pugi::xml_node>::make(root_->append_child(root_name_.c_str()));
54+
Ref<pugi::xml_node>::make(root_->append_child(root_name_));
5555
return OutputVarType(node_child);
5656
}
5757

5858
Writer::OutputVarType Writer::value_as_root_impl(
5959
const std::string& _str) const {
6060
auto node_child =
61-
Ref<pugi::xml_node>::make(root_->append_child(root_name_.c_str()));
62-
node_child->append_child(pugi::node_pcdata).set_value(_str.c_str());
61+
Ref<pugi::xml_node>::make(root_->append_child(root_name_));
62+
node_child->append_child(pugi::node_pcdata).set_value(_str);
6363
return OutputVarType(node_child);
6464
}
6565

@@ -78,23 +78,23 @@ Writer::OutputVarType Writer::add_value_to_array_impl(
7878
const std::string& _str, OutputArrayType* _parent) const {
7979
auto node_child =
8080
Ref<pugi::xml_node>::make(_parent->node_->append_child(_parent->name_));
81-
node_child->append_child(pugi::node_pcdata).set_value(_str.c_str());
81+
node_child->append_child(pugi::node_pcdata).set_value(_str);
8282
return OutputVarType(node_child);
8383
}
8484

8585
Writer::OutputVarType Writer::add_value_to_object_impl(
8686
const std::string_view& _name, const std::string& _str,
8787
OutputObjectType* _parent, const bool _is_attribute) const {
8888
if (_is_attribute) {
89-
_parent->node_->append_attribute(_name) = _str.c_str();
89+
_parent->node_->append_attribute(_name) = _str;
9090
return OutputVarType(_parent->node_);
9191
} else if (_name == XML_CONTENT) {
92-
_parent->node_->append_child(pugi::node_pcdata).set_value(_str.c_str());
92+
_parent->node_->append_child(pugi::node_pcdata).set_value(_str);
9393
return OutputVarType(_parent->node_);
9494
} else {
9595
auto node_child =
9696
Ref<pugi::xml_node>::make(_parent->node_->append_child(_name));
97-
node_child->append_child(pugi::node_pcdata).set_value(_str.c_str());
97+
node_child->append_child(pugi::node_pcdata).set_value(_str);
9898
return OutputVarType(node_child);
9999
}
100100
}

0 commit comments

Comments
 (0)