Skip to content

Commit 56ac9a2

Browse files
legendecastargos
authored andcommitted
src: migrate WriteOneByte to WriteOneByteV2
PR-URL: #59634 Fixes: #59555 Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Darshan Sen <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
1 parent 3d88aa9 commit 56ac9a2

File tree

5 files changed

+137
-36
lines changed

5 files changed

+137
-36
lines changed

src/node_buffer.cc

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,8 +1037,11 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
10371037
if (needle_data == nullptr) {
10381038
return args.GetReturnValue().Set(-1);
10391039
}
1040-
needle->WriteOneByte(
1041-
isolate, needle_data, 0, needle_length, String::NO_NULL_TERMINATION);
1040+
StringBytes::Write(isolate,
1041+
reinterpret_cast<char*>(needle_data),
1042+
needle_length,
1043+
needle,
1044+
enc);
10421045

10431046
result = nbytes::SearchString(reinterpret_cast<const uint8_t*>(haystack),
10441047
haystack_length,
@@ -1302,11 +1305,7 @@ static void Btoa(const FunctionCallbackInfo<Value>& args) {
13021305
simdutf::binary_to_base64(ext->data(), ext->length(), buffer.out());
13031306
} else if (input->IsOneByte()) {
13041307
MaybeStackBuffer<uint8_t> stack_buf(input->Length());
1305-
input->WriteOneByte(env->isolate(),
1306-
stack_buf.out(),
1307-
0,
1308-
input->Length(),
1309-
String::NO_NULL_TERMINATION);
1308+
input->WriteOneByteV2(env->isolate(), 0, input->Length(), stack_buf.out());
13101309

13111310
size_t expected_length =
13121311
simdutf::base64_length_from_binary(input->Length());
@@ -1362,11 +1361,8 @@ static void Atob(const FunctionCallbackInfo<Value>& args) {
13621361
ext->data(), ext->length(), buffer.out(), simdutf::base64_default);
13631362
} else if (input->IsOneByte()) {
13641363
MaybeStackBuffer<uint8_t> stack_buf(input->Length());
1365-
input->WriteOneByte(args.GetIsolate(),
1366-
stack_buf.out(),
1367-
0,
1368-
input->Length(),
1369-
String::NO_NULL_TERMINATION);
1364+
input->WriteOneByteV2(
1365+
args.GetIsolate(), 0, input->Length(), stack_buf.out());
13701366
const char* data = reinterpret_cast<const char*>(*stack_buf);
13711367
size_t expected_length =
13721368
simdutf::maximal_binary_length_from_base64(data, input->Length());

src/node_http2.cc

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -483,13 +483,10 @@ Origins::Origins(
483483

484484
CHECK_LE(origin_contents + origin_string_len,
485485
static_cast<char*>(bs_->Data()) + bs_->ByteLength());
486-
CHECK_EQ(origin_string->WriteOneByte(
487-
env->isolate(),
488-
reinterpret_cast<uint8_t*>(origin_contents),
489-
0,
490-
origin_string_len,
491-
String::NO_NULL_TERMINATION),
492-
origin_string_len);
486+
origin_string->WriteOneByteV2(env->isolate(),
487+
0,
488+
origin_string_len,
489+
reinterpret_cast<uint8_t*>(origin_contents));
493490

494491
size_t n = 0;
495492
char* p;
@@ -3186,8 +3183,8 @@ void Http2Session::AltSvc(const FunctionCallbackInfo<Value>& args) {
31863183
return;
31873184
}
31883185

3189-
size_t origin_len = origin_str->Length();
3190-
size_t value_len = value_str->Length();
3186+
int origin_len = origin_str->Length();
3187+
int value_len = value_str->Length();
31913188

31923189
CHECK_LE(origin_len + value_len, 16382); // Max permitted for ALTSVC
31933190
// Verify that origin len != 0 if stream id == 0, or
@@ -3196,8 +3193,13 @@ void Http2Session::AltSvc(const FunctionCallbackInfo<Value>& args) {
31963193

31973194
MaybeStackBuffer<uint8_t> origin(origin_len);
31983195
MaybeStackBuffer<uint8_t> value(value_len);
3199-
origin_str->WriteOneByte(env->isolate(), *origin);
3200-
value_str->WriteOneByte(env->isolate(), *value);
3196+
origin_str->WriteOneByteV2(env->isolate(),
3197+
0,
3198+
origin_len,
3199+
*origin,
3200+
String::WriteFlags::kNullTerminate);
3201+
value_str->WriteOneByteV2(
3202+
env->isolate(), 0, value_len, *value, String::WriteFlags::kNullTerminate);
32013203

32023204
session->AltSvc(id, *origin, origin_len, *value, value_len);
32033205
}

src/node_http_common-inl.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
#define SRC_NODE_HTTP_COMMON_INL_H_
33

44
#include "node_http_common.h"
5+
6+
#include "env-inl.h"
57
#include "node.h"
68
#include "node_mem-inl.h"
7-
#include "env-inl.h"
9+
#include "string_bytes.h"
810
#include "v8.h"
911

1012
#include <algorithm>
@@ -37,13 +39,12 @@ NgHeaders<T>::NgHeaders(Environment* env, v8::Local<v8::Array> headers) {
3739
nv_t* const nva = reinterpret_cast<nv_t*>(start);
3840

3941
CHECK_LE(header_contents + header_string_len, *buf_ + buf_.length());
40-
CHECK_EQ(header_string.As<v8::String>()->WriteOneByte(
41-
env->isolate(),
42-
reinterpret_cast<uint8_t*>(header_contents),
43-
0,
44-
header_string_len,
45-
v8::String::NO_NULL_TERMINATION),
46-
header_string_len);
42+
CHECK_EQ(StringBytes::Write(env->isolate(),
43+
header_contents,
44+
header_string_len,
45+
header_string.As<v8::String>(),
46+
LATIN1),
47+
static_cast<size_t>(header_string_len));
4748

4849
size_t n = 0;
4950
char* p;

src/string_bytes.cc

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,11 +254,13 @@ size_t StringBytes::Write(Isolate* isolate,
254254
nbytes = std::min(buflen, static_cast<size_t>(input_view.length()));
255255
memcpy(buf, input_view.data8(), nbytes);
256256
} else {
257-
uint8_t* const dst = reinterpret_cast<uint8_t*>(buf);
258-
const int flags = String::HINT_MANY_WRITES_EXPECTED |
259-
String::NO_NULL_TERMINATION |
260-
String::REPLACE_INVALID_UTF8;
261-
nbytes = str->WriteOneByte(isolate, dst, 0, buflen, flags);
257+
nbytes = std::min(buflen, static_cast<size_t>(input_view.length()));
258+
// Do not use v8::String::WriteOneByteV2 as it asserts the string to be
259+
// a one byte string. For compatibility, convert the uint16_t to uint8_t
260+
// even though this may loose accuracy.
261+
for (size_t i = 0; i < nbytes; i++) {
262+
buf[i] = static_cast<uint8_t>(input_view.data16()[i]);
263+
}
262264
}
263265
break;
264266

test/cctest/test_string_bytes.cc

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
#include "gtest/gtest.h"
2+
#include "node.h"
3+
#include "node_test_fixture.h"
4+
#include "string_bytes.h"
5+
#include "util-inl.h"
6+
7+
using node::MaybeStackBuffer;
8+
using node::StringBytes;
9+
using v8::HandleScope;
10+
using v8::Local;
11+
using v8::Maybe;
12+
using v8::String;
13+
14+
class StringBytesTest : public EnvironmentTestFixture {};
15+
16+
// Data "Hello, ÆÊÎÖÿ"
17+
static const char latin1_data[] = "Hello, \xC6\xCA\xCE\xD6\xFF";
18+
static const char utf8_data[] = "Hello, ÆÊÎÖÿ";
19+
20+
TEST_F(StringBytesTest, WriteLatin1WithOneByteString) {
21+
const HandleScope handle_scope(isolate_);
22+
const Argv argv;
23+
Env env_{handle_scope, argv};
24+
25+
Local<String> one_byte_str =
26+
String::NewFromOneByte(isolate_,
27+
reinterpret_cast<const uint8_t*>(latin1_data))
28+
.ToLocalChecked();
29+
30+
Maybe<size_t> size_maybe =
31+
StringBytes::StorageSize(isolate_, one_byte_str, node::LATIN1);
32+
33+
ASSERT_TRUE(size_maybe.IsJust());
34+
size_t size = size_maybe.FromJust();
35+
ASSERT_EQ(size, 12u);
36+
37+
MaybeStackBuffer<char> buf;
38+
size_t written = StringBytes::Write(
39+
isolate_, buf.out(), buf.capacity(), one_byte_str, node::LATIN1);
40+
ASSERT_EQ(written, 12u);
41+
42+
// Null-terminate the buffer and compare the contents.
43+
buf.SetLength(13);
44+
buf[12] = '\0';
45+
ASSERT_STREQ(latin1_data, buf.out());
46+
}
47+
48+
TEST_F(StringBytesTest, WriteLatin1WithUtf8String) {
49+
const HandleScope handle_scope(isolate_);
50+
const Argv argv;
51+
Env env_{handle_scope, argv};
52+
53+
Local<String> utf8_str =
54+
String::NewFromUtf8(isolate_, utf8_data).ToLocalChecked();
55+
56+
Maybe<size_t> size_maybe =
57+
StringBytes::StorageSize(isolate_, utf8_str, node::LATIN1);
58+
59+
ASSERT_TRUE(size_maybe.IsJust());
60+
size_t size = size_maybe.FromJust();
61+
ASSERT_EQ(size, 12u);
62+
63+
MaybeStackBuffer<char> buf;
64+
size_t written = StringBytes::Write(
65+
isolate_, buf.out(), buf.capacity(), utf8_str, node::LATIN1);
66+
ASSERT_EQ(written, 12u);
67+
68+
// Null-terminate the buffer and compare the contents.
69+
buf.SetLength(13);
70+
buf[12] = '\0';
71+
ASSERT_STREQ(latin1_data, buf.out());
72+
}
73+
74+
// Verify that StringBytes::Write converts two-byte characters to one-byte
75+
// characters, even if there is no valid one-byte representation.
76+
TEST_F(StringBytesTest, WriteLatin1WithInvalidChar) {
77+
const HandleScope handle_scope(isolate_);
78+
const Argv argv;
79+
Env env_{handle_scope, argv};
80+
81+
Local<String> utf8_str =
82+
String::NewFromUtf8(isolate_, "Hello, 世界").ToLocalChecked();
83+
84+
Maybe<size_t> size_maybe =
85+
StringBytes::StorageSize(isolate_, utf8_str, node::LATIN1);
86+
87+
ASSERT_TRUE(size_maybe.IsJust());
88+
size_t size = size_maybe.FromJust();
89+
ASSERT_EQ(size, 9u);
90+
91+
MaybeStackBuffer<char> buf;
92+
size_t written = StringBytes::Write(
93+
isolate_, buf.out(), buf.capacity(), utf8_str, node::LATIN1);
94+
ASSERT_EQ(written, 9u);
95+
96+
// Null-terminate the buffer and compare the contents.
97+
buf.SetLength(10);
98+
buf[9] = '\0';
99+
ASSERT_STREQ("Hello, \x16\x4C", buf.out());
100+
}

0 commit comments

Comments
 (0)