FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: unify implementations of Utf8Value etc. · nodejs/node@dda81b4 · GitHub

/ node Public

Commit dda81b4

Browse files
addaleax
authored and
Myles Borins
committed
src: unify implementations of Utf8Value etc.
Unify the common code of `Utf8Value`, `TwoByteValue`, `BufferValue` and `StringBytes::InlineDecoder` into one class. Always make the result zero-terminated for the first three. This fixes two problems in passing: * When the conversion of the input value to String fails, make the buffer zero-terminated anyway. Previously, this would have resulted in possibly reading uninitialized data in multiple places in the code. An instance of that problem can be reproduced by running e.g. `valgrind node -e 'net.isIP({ toString() { throw Error() } })'`. * Previously, `BufferValue` copied one byte too much from the source, possibly resulting in an out-of-bounds memory access. This can be reproduced by running e.g. `valgrind node -e \ 'fs.openSync(Buffer.from("node".repeat(8192)), "r")'`. Further minor changes: * This lifts the `out()` method of `StringBytes::InlineDecoder` to the common class so that it can be used when using the overloaded `operator*` does not seem appropiate. * Hopefully clearer variable names. * Add checks to make sure the length of the data does not exceed the allocated storage size, including the possible null terminator. PR-URL: #6357 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1 parent db2b23f commit dda81b4

3 files changed

Lines changed: 159 additions & 56 deletions

File tree

‎src/string_bytes.h‎

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,14 @@
77
#include "node.h"
88
#include "env.h"
99
#include "env-inl.h"
10+
#include "util.h"
1011

1112
namespace node {
1213

1314
class StringBytes {
1415
public:
15-
class InlineDecoder {
16+
class InlineDecoder : public MaybeStackBuffer<char> {
1617
public:
17-
InlineDecoder() : out_(nullptr) {
18-
}
19-
20-
~InlineDecoder() {
21-
if (out_ != out_st_)
22-
delete[] out_;
23-
out_ = nullptr;
24-
}
25-
2618
inline bool Decode(Environment* env,
2719
v8::Local<v8::String> string,
2820
v8::Local<v8::Value> encoding,
@@ -33,28 +25,22 @@ class StringBytes {
3325
return false;
3426
}
3527

36-
size_t buflen = StringBytes::StorageSize(env->isolate(), string, enc);
37-
if (buflen > sizeof(out_st_))
38-
out_ = new char[buflen];
39-
else
40-
out_ = out_st_;
41-
size_ = StringBytes::Write(env->isolate(),
42-
out_,
43-
buflen,
44-
string,
45-
enc);
28+
const size_t storage = StringBytes::StorageSize(env->isolate(),
29+
string,
30+
enc);
31+
AllocateSufficientStorage(storage);
32+
const size_t length = StringBytes::Write(env->isolate(),
33+
out(),
34+
storage,
35+
string,
36+
enc);
37+
38+
// No zero terminator is included when using this method.
39+
SetLength(length);
4640
return true;
4741
}
4842

49-
inline const char* out() const { return out_; }
50-
inline size_t size() const { return size_; }
51-
52-
private:
53-
static const int kStorageSize = 1024;
54-
55-
char out_st_[kStorageSize];
56-
char* out_;
57-
size_t size_;
43+
inline size_t size() const { return length(); }
5844
};
5945

6046
// Does the string match the encoding? Quick but non-exhaustive.

‎src/util.cc‎

Lines changed: 59 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,78 @@
11
#include "util.h"
2+
#include "node_buffer.h"
23
#include "string_bytes.h"
34

45
namespace node {
56

6-
Utf8Value::Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value)
7-
: length_(0), str_(str_st_) {
8-
// Make sure result is always zero-terminated, even if conversion to string
9-
// fails.
10-
str_st_[0] = '\0';
7+
using v8::Isolate;
8+
using v8::Local;
9+
using v8::String;
10+
using v8::Value;
1111

12+
template <typename T>
13+
static void MakeUtf8String(Isolate* isolate,
14+
Local<Value> value,
15+
T* target) {
16+
Local<String> string = value->ToString(isolate);
17+
if (string.IsEmpty())
18+
return;
19+
20+
const size_t storage = StringBytes::StorageSize(isolate, string, UTF8) + 1;
21+
target->AllocateSufficientStorage(storage);
22+
const int flags =
23+
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;
24+
const int length = string->WriteUtf8(target->out(), storage, 0, flags);
25+
target->SetLengthAndZeroTerminate(length);
26+
}
27+
28+
Utf8Value::Utf8Value(Isolate* isolate, Local<Value> value) {
1229
if (value.IsEmpty())
1330
return;
1431

32+
MakeUtf8String(isolate, value, this);
33+
}
34+
35+
36+
TwoByteValue::TwoByteValue(Isolate* isolate, Local<Value> value) {
37+
if (value.IsEmpty()) {
38+
return;
39+
}
40+
1541
v8::Local<v8::String> string = value->ToString(isolate);
1642
if (string.IsEmpty())
1743
return;
1844

1945
// Allocate enough space to include the null terminator
20-
size_t len = StringBytes::StorageSize(isolate, string, UTF8) + 1;
21-
if (len > sizeof(str_st_)) {
22-
str_ = static_cast<char*>(malloc(len));
23-
CHECK_NE(str_, nullptr);
24-
}
46+
const size_t storage = string->Length() + 1;
47+
AllocateSufficientStorage(storage);
2548

2649
const int flags =
27-
v8::String::NO_NULL_TERMINATION | v8::String::REPLACE_INVALID_UTF8;
28-
length_ = string->WriteUtf8(str_, len, 0, flags);
29-
str_[length_] = '\0';
50+
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;
51+
const int length = string->Write(out(), 0, storage, flags);
52+
SetLengthAndZeroTerminate(length);
53+
}
54+
55+
BufferValue::BufferValue(Isolate* isolate, Local<Value> value) {
56+
// Slightly different take on Utf8Value. If value is a String,
57+
// it will return a Utf8 encoded string. If value is a Buffer,
58+
// it will copy the data out of the Buffer as is.
59+
if (value.IsEmpty()) {
60+
// Dereferencing this object will return nullptr.
61+
Invalidate();
62+
return;
63+
}
64+
65+
if (value->IsString()) {
66+
MakeUtf8String(isolate, value, this);
67+
} else if (Buffer::HasInstance(value)) {
68+
const size_t len = Buffer::Length(value);
69+
// Leave place for the terminating '\0' byte.
70+
AllocateSufficientStorage(len + 1);
71+
memcpy(out(), Buffer::Data(value), len);
72+
SetLengthAndZeroTerminate(len);
73+
} else {
74+
Invalidate();
75+
}
3076
}
3177

3278
} // namespace node

‎src/util.h‎

Lines changed: 85 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -184,31 +184,102 @@ inline char ToLower(char c);
184184
// strcasecmp() is locale-sensitive. Use StringEqualNoCase() instead.
185185
inline bool StringEqualNoCase(const char* a, const char* b);
186186

187-
class Utf8Value {
187+
// Allocates an array of member type T. For up to kStackStorageSize items,
188+
// the stack is used, otherwise malloc().
189+
template <typename T, size_t kStackStorageSize = 1024>
190+
class MaybeStackBuffer {
188191
public:
189-
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
192+
const T* out() const {
193+
return buf_;
194+
}
190195

191-
~Utf8Value() {
192-
if (str_ != str_st_)
193-
free(str_);
196+
T* out() {
197+
return buf_;
194198
}
195199

196-
char* operator*() {
197-
return str_;
198-
};
200+
// operator* for compatibility with `v8::String::(Utf8)Value`
201+
T* operator*() {
202+
return buf_;
203+
}
199204

200-
const char* operator*() const {
201-
return str_;
202-
};
205+
const T* operator*() const {
206+
return buf_;
207+
}
203208

204209
size_t length() const {
205210
return length_;
206-
};
211+
}
212+
213+
// Call to make sure enough space for `storage` entries is available.
214+
// There can only be 1 call to AllocateSufficientStorage or Invalidate
215+
// per instance.
216+
void AllocateSufficientStorage(size_t storage) {
217+
if (storage <= kStackStorageSize) {
218+
buf_ = buf_st_;
219+
} else {
220+
// Guard against overflow.
221+
CHECK_LE(storage, sizeof(T) * storage);
222+
223+
buf_ = static_cast<T*>(malloc(sizeof(T) * storage));
224+
CHECK_NE(buf_, nullptr);
225+
}
226+
227+
// Remember how much was allocated to check against that in SetLength().
228+
length_ = storage;
229+
}
230+
231+
void SetLength(size_t length) {
232+
// length_ stores how much memory was allocated.
233+
CHECK_LE(length, length_);
234+
length_ = length;
235+
}
236+
237+
void SetLengthAndZeroTerminate(size_t length) {
238+
// length_ stores how much memory was allocated.
239+
CHECK_LE(length + 1, length_);
240+
SetLength(length);
241+
242+
// T() is 0 for integer types, nullptr for pointers, etc.
243+
buf_[length] = T();
244+
}
245+
246+
// Make derefencing this object return nullptr.
247+
// Calling this is mutually exclusive with calling
248+
// AllocateSufficientStorage.
249+
void Invalidate() {
250+
CHECK_EQ(buf_, buf_st_);
251+
length_ = 0;
252+
buf_ = nullptr;
253+
}
207254

255+
MaybeStackBuffer() : length_(0), buf_(buf_st_) {
256+
// Default to a zero-length, null-terminated buffer.
257+
buf_[0] = T();
258+
}
259+
260+
~MaybeStackBuffer() {
261+
if (buf_ != buf_st_)
262+
free(buf_);
263+
}
208264
private:
209265
size_t length_;
210-
char* str_;
211-
char str_st_[1024];
266+
T* buf_;
267+
T buf_st_[kStackStorageSize];
268+
};
269+
270+
class Utf8Value : public MaybeStackBuffer<char> {
271+
public:
272+
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
273+
};
274+
275+
class TwoByteValue : public MaybeStackBuffer<uint16_t> {
276+
public:
277+
explicit TwoByteValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
278+
};
279+
280+
class BufferValue : public MaybeStackBuffer<char> {
281+
public:
282+
explicit BufferValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
212283
};
213284

214285
} // namespace node

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL