Skip to content

Commit 8b2833b

Browse files
ActuallyaDeviloperliuzicheng1987
ActuallyaDeviloper
and
liuzicheng1987
authored
Avoid assuming that "std::string_view" is nullterminated. Generally it is really not. (#538)
Co-authored-by: ActuallyaDeviloper <ActuallyaDeviloper@users.noreply.github.com> Co-authored-by: Dr. Patrick Urbanke <patrick@getml.com>
1 parent 36bc907 commit 8b2833b

11 files changed

Lines changed: 52 additions & 43 deletions

File tree

‎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: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,9 @@ 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()),
170+
InputVarType{s.get("value")});
170171
}
171172
return std::nullopt;
172173
} catch (std::exception& e) {

‎include/rfl/capnproto/Writer.hpp‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,11 @@
1010
#include <string_view>
1111
#include <type_traits>
1212

13-
// #include "../Box.hpp"
1413
#include "../Bytestring.hpp"
15-
// #include "../Ref.hpp"
16-
// #include "../Result.hpp"
1714
#include "../Vectorstring.hpp"
1815
#include "../always_false.hpp"
19-
#include "../internal/is_literal.hpp"
20-
// #include "../internal/ptr_cast.hpp"
2116
#include "../common.hpp"
17+
#include "../internal/is_literal.hpp"
2218

2319
namespace rfl::capnproto {
2420

@@ -193,7 +189,7 @@ class RFL_API Writer {
193189
auto entries =
194190
OutputArrayType{_parent->val_.get("entries").as<capnp::DynamicList>()};
195191
auto new_entry = add_object_to_array(2, &entries);
196-
add_value_to_object("key", std::string(_name), &new_entry);
192+
add_value_to_object("key", _name, &new_entry);
197193
return add_value_to_object("value", _var, &new_entry);
198194
}
199195

@@ -202,20 +198,22 @@ class RFL_API Writer {
202198
const T& _var,
203199
OutputObjectType* _parent) const {
204200
if constexpr (std::is_same<std::remove_cvref_t<T>, std::string>()) {
205-
_parent->val_.set(_name.data(), _var.c_str());
201+
_parent->val_.set(to_kj_string_ptr(_name), _var.c_str());
206202

207203
} else if constexpr (std::is_same<std::remove_cvref_t<T>,
208204
rfl::Bytestring>()) {
209205
const auto array_ptr = kj::ArrayPtr<const kj::byte>(
210206
internal::ptr_cast<const unsigned char*>(_var.data()), _var.size());
211-
_parent->val_.set(_name.data(), capnp::Data::Reader(array_ptr));
207+
_parent->val_.set(to_kj_string_ptr(_name),
208+
capnp::Data::Reader(array_ptr));
212209

213210
} else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>() ||
214211
std::is_same<std::remove_cvref_t<T>, bool>()) {
215-
_parent->val_.set(_name.data(), _var);
212+
_parent->val_.set(to_kj_string_ptr(_name), _var);
216213

217214
} else if constexpr (std::is_integral<std::remove_cvref_t<T>>()) {
218-
_parent->val_.set(_name.data(), static_cast<std::int64_t>(_var));
215+
_parent->val_.set(to_kj_string_ptr(_name),
216+
static_cast<std::int64_t>(_var));
219217

220218
} else if constexpr (internal::is_literal_v<T>) {
221219
return add_value_to_object(_name, _var.value(), _parent);
@@ -262,6 +260,11 @@ class RFL_API Writer {
262260

263261
void end_object(OutputObjectType* /*_obj*/) const {}
264262

263+
private:
264+
kj::StringPtr to_kj_string_ptr(const std::string_view& _str) const {
265+
return kj::StringPtr(_str.data(), _str.size());
266+
}
267+
265268
private:
266269
capnp::DynamicStruct::Builder* root_;
267270
};

‎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 & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,22 +77,21 @@ class RFL_API Writer {
7777

7878
void end_object(OutputObjectType* _obj) const;
7979

80-
private:
8180
template <class T>
8281
OutputVarType insert_value(const std::string_view& _name,
8382
const T& _var) const {
8483
if constexpr (std::is_same<std::remove_cvref_t<T>, std::string>() ||
8584
std::is_same<std::remove_cvref_t<T>, bool>() ||
8685
std::is_same<std::remove_cvref_t<T>,
8786
std::remove_cvref_t<decltype(YAML::Null)>>()) {
88-
(*out_) << YAML::Key << _name.data() << YAML::Value << _var;
87+
(*out_) << YAML::Key << std::string(_name) << YAML::Value << _var;
8988
} else if constexpr (std::is_floating_point<std::remove_cvref_t<T>>()) {
9089
// std::to_string is necessary to ensure that floating point values are
9190
// always written as floats.
92-
(*out_) << YAML::Key << _name.data() << YAML::Value
91+
(*out_) << YAML::Key << std::string(_name) << YAML::Value
9392
<< std::to_string(_var);
9493
} else if constexpr (std::is_integral<std::remove_cvref_t<T>>()) {
95-
(*out_) << YAML::Key << _name.data() << YAML::Value
94+
(*out_) << YAML::Key << std::string(_name) << YAML::Value
9695
<< static_cast<int64_t>(_var);
9796
} else {
9897
static_assert(rfl::always_false_v<T>, "Unsupported type.");

‎src/rfl/capnproto/Writer.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ 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))
51-
.as<capnp::DynamicList>()};
50+
return OutputArrayType{
51+
_parent->val_.init(to_kj_string_ptr(_name), SafeSizeToUInt(_size))
52+
.as<capnp::DynamicList>()};
5253
}
5354

5455
Writer::OutputArrayType Writer::add_array_to_union(
@@ -111,7 +112,7 @@ Writer::OutputObjectType Writer::add_object_to_object(
111112
const std::string_view& _name, const size_t /*_size*/,
112113
OutputObjectType* _parent) const {
113114
return OutputObjectType{
114-
_parent->val_.get(_name.data()).as<capnp::DynamicStruct>()};
115+
_parent->val_.get(to_kj_string_ptr(_name)).as<capnp::DynamicStruct>()};
115116
}
116117

117118
Writer::OutputObjectType Writer::add_object_to_union(
@@ -142,7 +143,7 @@ Writer::OutputUnionType Writer::add_union_to_map(const std::string_view& _name,
142143
Writer::OutputUnionType Writer::add_union_to_object(
143144
const std::string_view& _name, OutputObjectType* _parent) const {
144145
return OutputUnionType{
145-
_parent->val_.get(_name.data()).as<capnp::DynamicStruct>()};
146+
_parent->val_.get(to_kj_string_ptr(_name)).as<capnp::DynamicStruct>()};
146147
}
147148

148149
Writer::OutputUnionType Writer::add_union_to_union(
@@ -171,7 +172,7 @@ Writer::OutputVarType Writer::add_null_to_map(const std::string_view& _name,
171172

172173
Writer::OutputVarType Writer::add_null_to_object(
173174
const std::string_view& _name, OutputObjectType* _parent) const {
174-
_parent->val_.set(_name.data(), capnp::VOID);
175+
_parent->val_.set(to_kj_string_ptr(_name), capnp::VOID);
175176
return OutputVarType{};
176177
}
177178

‎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)