Skip to content
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

Jump to bottom

n-api: add string api for latin1 encoding and new test cases #12368

Closed
wants to merge 4 commits into from
Closed

n-api: add string api for latin1 encoding and new test cases #12368

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6d9eee
n-api: add string api for latin1 encoding and new test cases
Apr 12, 2017
f506579
n-api: update the comment for string api changes
Apr 12, 2017
4f110e8
n-api: add test cases for latin1 encoding character set
Apr 12, 2017
c3faab1
n-api: update comment for string api to indicate sufficient buffer size
Apr 12, 2017
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
60 changes: 58 additions & 2 deletions src/node_api.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,25 @@ napi_status napi_create_array_with_length(napi_env env,
return GET_RETURN_STATUS(env);
}

napi_status napi_create_string_latin1(napi_env env,
const char* str,
size_t length,
napi_value* result) {
NAPI_PREAMBLE(env);
CHECK_ARG(env, result);

auto isolate = env->isolate;
auto str_maybe =
v8::String::NewFromOneByte(isolate,
reinterpret_cast<const uint8_t*>(str),
v8::NewStringType::kInternalized,
length);
CHECK_MAYBE_EMPTY(env, str_maybe, napi_generic_failure);

*result = v8impl::JsValueFromV8LocalValue(str_maybe.ToLocalChecked());
return GET_RETURN_STATUS(env);
}

napi_status napi_create_string_utf8(napi_env env,
const char* str,
size_t length,
Expand Down Expand Up @@ -1714,6 +1733,39 @@ napi_status napi_get_value_string_length(napi_env env,
return GET_RETURN_STATUS(env);
}

// Copies a JavaScript string into a LATIN-1 string buffer. The result is the
// number of bytes copied into buf, including the null terminator. If bufsize
Copy link
Member

Choose a reason for hiding this comment

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

Now the null terminator is not counted in the result. I understand that is done for consistency with the usage that passes a null buffer just to get the string length. But then the comments need to be updated on all 3 functions.

// is insufficient, the string will be truncated, including a null terminator.
// If buf is NULL, this method returns the length of the string (in bytes)
// via the result parameter.
Copy link
Member

Choose a reason for hiding this comment

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

Can we clarify here that the length of the string does not include space for the terminator and that you should pass in a buffer of length+1 to avoid truncation ?

// The result argument is optional unless buf is NULL.
napi_status napi_get_value_string_latin1(napi_env env,
napi_value value,
char* buf,
size_t bufsize,
size_t* result) {
NAPI_PREAMBLE(env);

v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
RETURN_STATUS_IF_FALSE(env, val->IsString(), napi_string_expected);

if (!buf) {
CHECK_ARG(env, result);
*result = val.As<v8::String>()->Length();
} else {
int copied = val.As<v8::String>()->WriteOneByte(
reinterpret_cast<uint8_t*>(buf), 0, bufsize - 1,
v8::String::NO_NULL_TERMINATION);

buf[copied] = '\0';
if (result != nullptr) {
*result = copied;
}
}

return GET_RETURN_STATUS(env);
}

// Copies a JavaScript string into a UTF-8 string buffer. The result is the
// number of bytes copied into buf, including the null terminator. If bufsize
// is insufficient, the string will be truncated, including a null terminator.
Expand All @@ -1735,8 +1787,10 @@ napi_status napi_get_value_string_utf8(napi_env env,
*result = val.As<v8::String>()->Utf8Length();
} else {
int copied = val.As<v8::String>()->WriteUtf8(
buf, bufsize, nullptr, v8::String::REPLACE_INVALID_UTF8);
buf, bufsize - 1, nullptr, v8::String::REPLACE_INVALID_UTF8 |
v8::String::NO_NULL_TERMINATION);

buf[copied] = '\0';
if (result != nullptr) {
*result = copied;
}
Expand Down Expand Up @@ -1767,8 +1821,10 @@ napi_status napi_get_value_string_utf16(napi_env env,
*result = val.As<v8::String>()->Length();
} else {
int copied = val.As<v8::String>()->Write(
reinterpret_cast<uint16_t*>(buf), 0, bufsize, v8::String::NO_OPTIONS);
reinterpret_cast<uint16_t*>(buf), 0, bufsize - 1,
v8::String::NO_NULL_TERMINATION);

buf[copied] = '\0';
if (result != nullptr) {
*result = copied;
}
Expand Down
11 changes: 11 additions & 0 deletions src/node_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ NAPI_EXTERN napi_status napi_create_array_with_length(napi_env env,
NAPI_EXTERN napi_status napi_create_number(napi_env env,
double value,
napi_value* result);
NAPI_EXTERN napi_status napi_create_string_latin1(napi_env env,
const char* str,
size_t length,
napi_value* result);
NAPI_EXTERN napi_status napi_create_string_utf8(napi_env env,
const char* str,
size_t length,
Expand Down Expand Up @@ -172,6 +176,13 @@ NAPI_EXTERN napi_status napi_get_value_string_length(napi_env env,
napi_value value,
size_t* result);

// Copies LATIN-1 encoded bytes from a string into a buffer.
NAPI_EXTERN napi_status napi_get_value_string_latin1(napi_env env,
napi_value value,
char* buf,
size_t bufsize,
size_t* result);

// Copies UTF-8 encoded bytes from a string into a buffer.
NAPI_EXTERN napi_status napi_get_value_string_utf8(napi_env env,
napi_value value,
Expand Down
39 changes: 32 additions & 7 deletions test/addons-napi/test_string/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,47 @@ const assert = require('assert');
// testing api calls for string
const test_string = require(`./build/${common.buildType}/test_string`);

const empty = '';
assert.strictEqual(test_string.TestLatin1(empty), empty);
assert.strictEqual(test_string.TestUtf8(empty), empty);
assert.strictEqual(test_string.TestUtf16(empty), empty);
assert.strictEqual(test_string.Length(empty), 0);
assert.strictEqual(test_string.Utf8Length(empty), 0);

const str1 = 'hello world';
assert.strictEqual(test_string.Copy(str1), str1);
assert.strictEqual(test_string.TestLatin1(str1), str1);
assert.strictEqual(test_string.TestUtf8(str1), str1);
assert.strictEqual(test_string.TestUtf16(str1), str1);
assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.Length(str1), 11);
assert.strictEqual(test_string.Utf8Length(str1), 11);

const str2 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
assert.strictEqual(test_string.Copy(str2), str2);
assert.strictEqual(test_string.TestLatin1(str2), str2);
assert.strictEqual(test_string.TestUtf8(str2), str2);
assert.strictEqual(test_string.TestUtf16(str2), str2);
assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.Length(str2), 62);
assert.strictEqual(test_string.Utf8Length(str2), 62);

const str3 = '?!@#$%^&*()_+-=[]{}/.,<>\'"\\';
assert.strictEqual(test_string.Copy(str3), str3);
assert.strictEqual(test_string.TestLatin1(str3), str3);
assert.strictEqual(test_string.TestUtf8(str3), str3);
assert.strictEqual(test_string.TestUtf16(str3), str3);
assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.Length(str3), 27);
assert.strictEqual(test_string.Utf8Length(str3), 27);

const str4 = '\u{2003}\u{2101}\u{2001}';
assert.strictEqual(test_string.Copy(str4), str4);
assert.strictEqual(test_string.Length(str4), 3);
assert.strictEqual(test_string.Utf8Length(str4), 9);
const str4 = '\u{2003}\u{2101}\u{2001}\u{202}\u{2011}';
Copy link
Member

Choose a reason for hiding this comment

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

Latin1 is not tested here, I assume because these characters are not supported by Latin1 encoding. So then can there be another test case that tests a string with non-ASCII Latin1 characters? (That would also work with UTF8 and UTF16 encodings.)

assert.strictEqual(test_string.TestUtf8(str4), str4);
assert.strictEqual(test_string.TestUtf16(str4), str4);
assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1));
assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3));
assert.strictEqual(test_string.Length(str4), 5);
assert.strictEqual(test_string.Utf8Length(str4), 14);
141 changes: 138 additions & 3 deletions test/addons-napi/test_string/test_string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,33 @@
#include <node_api.h>
#include "../common.h"

napi_value Copy(napi_env env, napi_callback_info info) {
napi_value TestLatin1(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");

napi_valuetype valuetype;
NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));

NAPI_ASSERT(env, valuetype == napi_string,
"Wrong type of argment. Expects a string.");

char buffer[128];
size_t buffer_size = 128;
size_t copied;

NAPI_CALL(env,
napi_get_value_string_latin1(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_latin1(env, buffer, copied, &output));

return output;
}

napi_value TestUtf8(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
Expand All @@ -22,7 +48,111 @@ napi_value Copy(napi_env env, napi_callback_info info) {
napi_get_value_string_utf8(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_utf8(env, buffer, copied-1, &output));
NAPI_CALL(env, napi_create_string_utf8(env, buffer, copied, &output));

return output;
}

napi_value TestUtf16(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");

napi_valuetype valuetype;
NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));

NAPI_ASSERT(env, valuetype == napi_string,
"Wrong type of argment. Expects a string.");

char16_t buffer[128];
size_t buffer_size = 128;
size_t copied;

NAPI_CALL(env,
napi_get_value_string_utf16(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_utf16(env, buffer, copied, &output));

return output;
}

napi_value TestLatin1Insufficient(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");

napi_valuetype valuetype;
NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));

NAPI_ASSERT(env, valuetype == napi_string,
"Wrong type of argment. Expects a string.");

char buffer[4];
size_t buffer_size = 4;
size_t copied;

NAPI_CALL(env,
napi_get_value_string_latin1(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_latin1(env, buffer, copied, &output));

return output;
}

napi_value TestUtf8Insufficient(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");

napi_valuetype valuetype;
NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));

NAPI_ASSERT(env, valuetype == napi_string,
"Wrong type of argment. Expects a string.");

char buffer[4];
size_t buffer_size = 4;
size_t copied;

NAPI_CALL(env,
napi_get_value_string_utf8(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_utf8(env, buffer, copied, &output));

return output;
}

napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));

NAPI_ASSERT(env, argc >= 1, "Wrong number of arguments");

napi_valuetype valuetype;
NAPI_CALL(env, napi_typeof(env, args[0], &valuetype));

NAPI_ASSERT(env, valuetype == napi_string,
"Wrong type of argment. Expects a string.");

char16_t buffer[4];
size_t buffer_size = 4;
size_t copied;

NAPI_CALL(env,
napi_get_value_string_utf16(env, args[0], buffer, buffer_size, &copied));

napi_value output;
NAPI_CALL(env, napi_create_string_utf16(env, buffer, copied, &output));

return output;
}
Expand Down Expand Up @@ -73,7 +203,12 @@ napi_value Utf8Length(napi_env env, napi_callback_info info) {

void Init(napi_env env, napi_value exports, napi_value module, void* priv) {
napi_property_descriptor properties[] = {
DECLARE_NAPI_PROPERTY("Copy", Copy),
DECLARE_NAPI_PROPERTY("TestLatin1", TestLatin1),
DECLARE_NAPI_PROPERTY("TestLatin1Insufficient", TestLatin1Insufficient),
DECLARE_NAPI_PROPERTY("TestUtf8", TestUtf8),
DECLARE_NAPI_PROPERTY("TestUtf8Insufficient", TestUtf8Insufficient),
DECLARE_NAPI_PROPERTY("TestUtf16", TestUtf16),
DECLARE_NAPI_PROPERTY("TestUtf16Insufficient", TestUtf16Insufficient),
DECLARE_NAPI_PROPERTY("Length", Length),
DECLARE_NAPI_PROPERTY("Utf8Length", Utf8Length),
};
Expand Down