Skip to content

Commit d8b50de

Browse files
authored
Fix off-by-one error for strings of certain sizes (#47)
Resizing a std::string to the number of chars to be copied into it usually worked because of how std::string usually allocates a little more memory than what is requested. But sometimes there would be no extra capacity, so the capacity() value passed to napi_get_value_string() was too short by one (due to the null-terminator). The result was strings of certain lengths could have one char trimmed off the end.
1 parent b353114 commit d8b50de

File tree

3 files changed

+25
-0
lines changed

3 files changed

+25
-0
lines changed

napi-inl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,7 @@ inline std::string String::Utf8Value() const {
502502
if (status != napi_ok) throw Error::New(_env);
503503

504504
std::string value;
505+
value.reserve(length + 1);
505506
value.resize(length);
506507
status = napi_get_value_string_utf8(_env, _value, &value[0], value.capacity(), nullptr);
507508
if (status != napi_ok) throw Error::New(_env);
@@ -514,6 +515,7 @@ inline std::u16string String::Utf16Value() const {
514515
if (status != napi_ok) throw Error::New(_env);
515516

516517
std::u16string value;
518+
value.reserve(length + 1);
517519
value.resize(length);
518520
status = napi_get_value_string_utf16(_env, _value, &value[0], value.capacity(), nullptr);
519521
if (status != napi_ok) throw Error::New(_env);

test/name.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,19 @@ using namespace Napi;
55
const char* testValueUtf8 = "123456789";
66
const char16_t* testValueUtf16 = u"123456789";
77

8+
Value EchoString(const CallbackInfo& info) {
9+
String value = info[0].As<String>();
10+
String encoding = info[1].As<String>();
11+
12+
if (encoding.Utf8Value() == "utf8") {
13+
return String::New(info.Env(), value.Utf8Value().c_str());
14+
} else if (encoding.Utf8Value() == "utf16") {
15+
return String::New(info.Env(), value.Utf16Value().c_str());
16+
} else {
17+
throw Error::New(info.Env(), "Invalid encoding.");
18+
}
19+
}
20+
821
Value CreateString(const CallbackInfo& info) {
922
String encoding = info[0].As<String>();
1023
Number length = info[1].As<Number>();
@@ -69,6 +82,7 @@ Value CheckSymbol(const CallbackInfo& info) {
6982
Object InitName(Env env) {
7083
Object exports = Object::New(env);
7184

85+
exports["echoString"] = Function::New(env, EchoString);
7286
exports["createString"] = Function::New(env, CreateString);
7387
exports["checkString"] = Function::New(env, CheckString);
7488
exports["createSymbol"] = Function::New(env, CreateSymbol);

test/name.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,12 @@ assert.ok(binding.name.checkSymbol(sym1));
4040
const sym2 = binding.name.createSymbol('test');
4141
assert.strictEqual(typeof sym2, 'symbol');
4242
assert.ok(binding.name.checkSymbol(sym1));
43+
44+
// Check for off-by-one errors which might only appear for strings of certain sizes,
45+
// due to how std::string increments its capacity in chunks.
46+
const longString = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz';
47+
for (let i = 10; i <= longString.length; i++) {
48+
const str = longString.substr(0, i);
49+
assert.strictEqual(binding.name.echoString(str, 'utf8'), str);
50+
assert.strictEqual(binding.name.echoString(str, 'utf16'), str);
51+
}

0 commit comments

Comments
 (0)