diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index a7b9132a71e6aa..a6a5149126f24e 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -123,7 +123,7 @@ static void NewQueryReqWrap(const FunctionCallbackInfo& args) { } -static int cmp_ares_tasks(const ares_task_t* a, const ares_task_t* b) { +static int cmp_ares_tasks(const node_ares_task* a, const node_ares_task* b) { if (a->sock < b->sock) return -1; if (a->sock > b->sock) @@ -132,7 +132,7 @@ static int cmp_ares_tasks(const ares_task_t* a, const ares_task_t* b) { } -RB_GENERATE_STATIC(ares_task_list, ares_task_t, node, cmp_ares_tasks) +RB_GENERATE_STATIC(node_ares_task_list, node_ares_task, node, cmp_ares_tasks) @@ -146,7 +146,7 @@ static void ares_timeout(uv_timer_t* handle) { static void ares_poll_cb(uv_poll_t* watcher, int status, int events) { - ares_task_t* task = ContainerOf(&ares_task_t::poll_watcher, watcher); + node_ares_task* task = ContainerOf(&node_ares_task::poll_watcher, watcher); Environment* env = task->env; /* Reset the idle timer */ @@ -167,15 +167,16 @@ static void ares_poll_cb(uv_poll_t* watcher, int status, int events) { static void ares_poll_close_cb(uv_handle_t* watcher) { - ares_task_t* task = ContainerOf(&ares_task_t::poll_watcher, + node_ares_task* task = ContainerOf(&node_ares_task::poll_watcher, reinterpret_cast(watcher)); free(task); } -/* Allocates and returns a new ares_task_t */ -static ares_task_t* ares_task_create(Environment* env, ares_socket_t sock) { - ares_task_t* task = static_cast(malloc(sizeof(*task))); +/* Allocates and returns a new node_ares_task */ +static node_ares_task* ares_task_create(Environment* env, ares_socket_t sock) { + node_ares_task* task = + static_cast(node::Malloc(sizeof(*task))); if (task == nullptr) { /* Out of memory. */ @@ -201,11 +202,11 @@ static void ares_sockstate_cb(void* data, int read, int write) { Environment* env = static_cast(data); - ares_task_t* task; + node_ares_task* task; - ares_task_t lookup_task; + node_ares_task lookup_task; lookup_task.sock = sock; - task = RB_FIND(ares_task_list, env->cares_task_list(), &lookup_task); + task = RB_FIND(node_ares_task_list, env->cares_task_list(), &lookup_task); if (read || write) { if (!task) { @@ -226,7 +227,7 @@ static void ares_sockstate_cb(void* data, return; } - RB_INSERT(ares_task_list, env->cares_task_list(), task); + RB_INSERT(node_ares_task_list, env->cares_task_list(), task); } /* This should never fail. If it fails anyway, the query will eventually */ @@ -242,7 +243,7 @@ static void ares_sockstate_cb(void* data, CHECK(task && "When an ares socket is closed we should have a handle for it"); - RB_REMOVE(ares_task_list, env->cares_task_list(), task); + RB_REMOVE(node_ares_task_list, env->cares_task_list(), task); uv_close(reinterpret_cast(&task->poll_watcher), ares_poll_close_cb); diff --git a/src/env-inl.h b/src/env-inl.h index 6f19ff50cb536f..0a4ce55b3bc4ae 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -398,7 +398,7 @@ inline ares_channel* Environment::cares_channel_ptr() { return &cares_channel_; } -inline ares_task_list* Environment::cares_task_list() { +inline node_ares_task_list* Environment::cares_task_list() { return &cares_task_list_; } diff --git a/src/env.h b/src/env.h index 7b6ffc8b87c8ee..67d10b20e41673 100644 --- a/src/env.h +++ b/src/env.h @@ -259,16 +259,14 @@ namespace node { class Environment; -// TODO(bnoordhuis) Rename struct, the ares_ prefix implies it's part -// of the c-ares API while the _t suffix implies it's a typedef. -struct ares_task_t { +struct node_ares_task { Environment* env; ares_socket_t sock; uv_poll_t poll_watcher; - RB_ENTRY(ares_task_t) node; + RB_ENTRY(node_ares_task) node; }; -RB_HEAD(ares_task_list, ares_task_t); +RB_HEAD(node_ares_task_list, node_ares_task); class Environment { public: @@ -440,7 +438,7 @@ class Environment { inline uv_timer_t* cares_timer_handle(); inline ares_channel cares_channel(); inline ares_channel* cares_channel_ptr(); - inline ares_task_list* cares_task_list(); + inline node_ares_task_list* cares_task_list(); inline bool using_domains() const; inline void set_using_domains(bool value); @@ -542,7 +540,7 @@ class Environment { const uint64_t timer_base_; uv_timer_t cares_timer_handle_; ares_channel cares_channel_; - ares_task_list cares_task_list_; + node_ares_task_list cares_task_list_; bool using_domains_; bool printed_error_; bool trace_sync_io_; diff --git a/src/node.cc b/src/node.cc index c5d4391bce9d96..d34225e2132dca 100644 --- a/src/node.cc +++ b/src/node.cc @@ -978,9 +978,9 @@ void* ArrayBufferAllocator::Allocate(size_t size) { if (env_ == nullptr || !env_->array_buffer_allocator_info()->no_zero_fill() || zero_fill_all_buffers) - return calloc(size, 1); + return node::Calloc(size, 1); env_->array_buffer_allocator_info()->reset_fill_flag(); - return malloc(size); + return node::Malloc(size); } static bool DomainHasErrorHandler(const Environment* env, diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 877fdc0a551579..11317328a6b549 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -49,7 +49,7 @@ size_t length = end - start; #define BUFFER_MALLOC(length) \ - zero_fill_all_buffers ? calloc(length, 1) : malloc(length) + zero_fill_all_buffers ? node::Calloc(length, 1) : node::Malloc(length) namespace node { @@ -247,10 +247,6 @@ MaybeLocal New(Isolate* isolate, size_t actual = 0; char* data = nullptr; - // malloc(0) and realloc(ptr, 0) have implementation-defined behavior in - // that the standard allows them to either return a unique pointer or a - // nullptr for zero-sized allocation requests. Normalize by always using - // a nullptr. if (length > 0) { data = static_cast(BUFFER_MALLOC(length)); @@ -264,7 +260,7 @@ MaybeLocal New(Isolate* isolate, free(data); data = nullptr; } else if (actual < length) { - data = static_cast(realloc(data, actual)); + data = static_cast(node::Realloc(data, actual)); CHECK_NE(data, nullptr); } } @@ -343,7 +339,7 @@ MaybeLocal Copy(Environment* env, const char* data, size_t length) { void* new_data; if (length > 0) { CHECK_NE(data, nullptr); - new_data = malloc(length); + new_data = node::Malloc(length); if (new_data == nullptr) return Local(); memcpy(new_data, data, length); @@ -931,7 +927,7 @@ void IndexOfString(const FunctionCallbackInfo& args) { needle_length, offset); } else if (enc == BINARY) { - uint8_t* needle_data = static_cast(malloc(needle_length)); + uint8_t* needle_data = static_cast(node::Malloc(needle_length)); if (needle_data == nullptr) { return args.GetReturnValue().Set(-1); } diff --git a/src/node_crypto.cc b/src/node_crypto.cc index b3169caa2b884d..c3d61026d38a8a 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2090,7 +2090,7 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { size_t len = Buffer::Length(obj); // OpenSSL takes control of the pointer after accepting it - char* data = reinterpret_cast(malloc(len)); + char* data = reinterpret_cast(node::Malloc(len)); CHECK_NE(data, nullptr); memcpy(data, resp, len); @@ -3139,7 +3139,7 @@ bool CipherBase::GetAuthTag(char** out, unsigned int* out_len) const { if (initialised_ || kind_ != kCipher || !auth_tag_) return false; *out_len = auth_tag_len_; - *out = static_cast(malloc(auth_tag_len_)); + *out = static_cast(node::Malloc(auth_tag_len_)); CHECK_NE(*out, nullptr); memcpy(*out, auth_tag_, auth_tag_len_); return true; @@ -4694,7 +4694,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { // NOTE: field_size is in bits int field_size = EC_GROUP_get_degree(ecdh->group_); size_t out_len = (field_size + 7) / 8; - char* out = static_cast(malloc(out_len)); + char* out = static_cast(node::Malloc(out_len)); CHECK_NE(out, nullptr); int r = ECDH_compute_key(out, out_len, pub, ecdh->key_, nullptr); @@ -4733,7 +4733,7 @@ void ECDH::GetPublicKey(const FunctionCallbackInfo& args) { if (size == 0) return env->ThrowError("Failed to get public key length"); - unsigned char* out = static_cast(malloc(size)); + unsigned char* out = static_cast(node::Malloc(size)); CHECK_NE(out, nullptr); int r = EC_POINT_point2oct(ecdh->group_, pub, form, out, size, nullptr); @@ -4762,7 +4762,7 @@ void ECDH::GetPrivateKey(const FunctionCallbackInfo& args) { return env->ThrowError("Failed to get ECDH private key"); int size = BN_num_bytes(b); - unsigned char* out = static_cast(malloc(size)); + unsigned char* out = static_cast(node::Malloc(size)); CHECK_NE(out, nullptr); if (size != BN_bn2bin(b, out)) { @@ -4839,7 +4839,7 @@ class PBKDF2Request : public AsyncWrap { saltlen_(saltlen), salt_(salt), keylen_(keylen), - key_(static_cast(malloc(keylen))), + key_(static_cast(node::Malloc(keylen))), iter_(iter) { if (key() == nullptr) FatalError("node::PBKDF2Request()", "Out of Memory"); @@ -5002,7 +5002,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(args[1]); - pass = static_cast(malloc(passlen)); + pass = static_cast(node::Malloc(passlen)); if (pass == nullptr) { FatalError("node::PBKDF2()", "Out of Memory"); } @@ -5014,7 +5014,7 @@ void PBKDF2(const FunctionCallbackInfo& args) { goto err; } - salt = static_cast(malloc(saltlen)); + salt = static_cast(node::Malloc(saltlen)); if (salt == nullptr) { FatalError("node::PBKDF2()", "Out of Memory"); } @@ -5107,7 +5107,7 @@ class RandomBytesRequest : public AsyncWrap { : AsyncWrap(env, object, AsyncWrap::PROVIDER_CRYPTO), error_(0), size_(size), - data_(static_cast(malloc(size))) { + data_(static_cast(node::Malloc(size))) { if (data() == nullptr) FatalError("node::RandomBytesRequest()", "Out of Memory"); Wrap(object, this); @@ -5336,7 +5336,7 @@ void GetCurves(const FunctionCallbackInfo& args) { if (num_curves) { alloc_size = sizeof(*curves) * num_curves; - curves = static_cast(malloc(alloc_size)); + curves = static_cast(node::Malloc(alloc_size)); CHECK_NE(curves, nullptr); diff --git a/src/node_internals.h b/src/node_internals.h index 62bf1c463831a7..f7ede94d88e021 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -216,7 +216,8 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator { inline void set_env(Environment* env) { env_ = env; } virtual void* Allocate(size_t size); // Defined in src/node.cc - virtual void* AllocateUninitialized(size_t size) { return malloc(size); } + virtual void* AllocateUninitialized(size_t size) + { return node::Malloc(size); } virtual void Free(void* data, size_t) { free(data); } private: diff --git a/src/stream_wrap.cc b/src/stream_wrap.cc index 56012e67a55144..602a3642cf7ddb 100644 --- a/src/stream_wrap.cc +++ b/src/stream_wrap.cc @@ -154,7 +154,7 @@ void StreamWrap::OnAlloc(uv_handle_t* handle, void StreamWrap::OnAllocImpl(size_t size, uv_buf_t* buf, void* ctx) { - buf->base = static_cast(malloc(size)); + buf->base = static_cast(node::Malloc(size)); buf->len = size; if (buf->base == nullptr && size > 0) { @@ -210,7 +210,7 @@ void StreamWrap::OnReadImpl(ssize_t nread, return; } - char* base = static_cast(realloc(buf->base, nread)); + char* base = static_cast(node::Realloc(buf->base, nread)); CHECK_LE(static_cast(nread), buf->len); if (pending == UV_TCP) { diff --git a/src/string_bytes.cc b/src/string_bytes.cc index 8b993f57466f78..a650ac0b00452e 100644 --- a/src/string_bytes.cc +++ b/src/string_bytes.cc @@ -54,7 +54,7 @@ class ExternString: public ResourceType { return scope.Escape(String::Empty(isolate)); TypeName* new_data = - static_cast(malloc(length * sizeof(*new_data))); + static_cast(node::Malloc(length * sizeof(*new_data))); if (new_data == nullptr) { return Local(); } @@ -784,7 +784,7 @@ Local StringBytes::Encode(Isolate* isolate, case ASCII: if (contains_non_ascii(buf, buflen)) { - char* out = static_cast(malloc(buflen)); + char* out = static_cast(node::Malloc(buflen)); if (out == nullptr) { return Local(); } @@ -819,7 +819,7 @@ Local StringBytes::Encode(Isolate* isolate, case BASE64: { size_t dlen = base64_encoded_size(buflen); - char* dst = static_cast(malloc(dlen)); + char* dst = static_cast(node::Malloc(dlen)); if (dst == nullptr) { return Local(); } @@ -838,7 +838,7 @@ Local StringBytes::Encode(Isolate* isolate, case HEX: { size_t dlen = buflen * 2; - char* dst = static_cast(malloc(dlen)); + char* dst = static_cast(node::Malloc(dlen)); if (dst == nullptr) { return Local(); } diff --git a/src/tls_wrap.cc b/src/tls_wrap.cc index 116a379337ef57..dd1b0e3b5f340f 100644 --- a/src/tls_wrap.cc +++ b/src/tls_wrap.cc @@ -662,7 +662,7 @@ void TLSWrap::OnReadImpl(ssize_t nread, void TLSWrap::OnAllocSelf(size_t suggested_size, uv_buf_t* buf, void* ctx) { - buf->base = static_cast(malloc(suggested_size)); + buf->base = static_cast(node::Malloc(suggested_size)); CHECK_NE(buf->base, nullptr); buf->len = suggested_size; } diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 57238534c4a30e..e469d77511a50c 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -359,7 +359,7 @@ void UDPWrap::OnSend(uv_udp_send_t* req, int status) { void UDPWrap::OnAlloc(uv_handle_t* handle, size_t suggested_size, uv_buf_t* buf) { - buf->base = static_cast(malloc(suggested_size)); + buf->base = static_cast(node::Malloc(suggested_size)); buf->len = suggested_size; if (buf->base == nullptr && suggested_size > 0) { @@ -401,7 +401,7 @@ void UDPWrap::OnRecv(uv_udp_t* handle, return; } - char* base = static_cast(realloc(buf->base, nread)); + char* base = static_cast(node::Realloc(buf->base, nread)); argv[2] = Buffer::New(env, base, nread).ToLocalChecked(); argv[3] = AddressToJS(env, addr); wrap->MakeCallback(env->onmessage_string(), arraysize(argv), argv); diff --git a/src/util-inl.h b/src/util-inl.h index 7051659a5e0e6a..834771f5ee996d 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -217,6 +217,34 @@ bool StringEqualNoCase(const char* a, const char* b) { return false; } +// These should be used in our code as opposed to the native +// versions as they abstract out some platform and or +// compiler version specific functionality. +// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in +// that the standard allows them to either return a unique pointer or a +// nullptr for zero-sized allocation requests. Normalize by always using +// a nullptr. +void* Realloc(void* pointer, size_t size) { + if (size == 0) { + free(pointer); + return nullptr; + } + return realloc(pointer, size); +} + +// As per spec realloc behaves like malloc if passed nullptr. +void* Malloc(size_t size) { + if (size == 0) size = 1; + return Realloc(nullptr, size); +} + +void* Calloc(size_t n, size_t size) { + if (n == 0) n = 1; + if (size == 0) size = 1; + CHECK_GE(n * size, n); // Overflow guard. + return calloc(n, size); +} + } // namespace node #endif // SRC_UTIL_INL_H_ diff --git a/src/util.h b/src/util.h index e5de6f2207e3b0..f96fb77cfafbea 100644 --- a/src/util.h +++ b/src/util.h @@ -16,6 +16,17 @@ namespace node { +// These should be used in our code as opposed to the native +// versions as they abstract out some platform and or +// compiler version specific functionality +// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in +// that the standard allows them to either return a unique pointer or a +// nullptr for zero-sized allocation requests. Normalize by always using +// a nullptr. +inline void* Realloc(void* pointer, size_t size); +inline void* Malloc(size_t size); +inline void* Calloc(size_t n, size_t size); + #ifdef __APPLE__ template using remove_reference = std::tr1::remove_reference; #else @@ -250,7 +261,7 @@ class MaybeStackBuffer { // Guard against overflow. CHECK_LE(storage, sizeof(T) * storage); - buf_ = static_cast(malloc(sizeof(T) * storage)); + buf_ = static_cast(Malloc(sizeof(T) * storage)); CHECK_NE(buf_, nullptr); } diff --git a/test/cctest/util.cc b/test/cctest/util.cc index 37133aca562b72..1f1584fa72124a 100644 --- a/test/cctest/util.cc +++ b/test/cctest/util.cc @@ -74,3 +74,17 @@ TEST(UtilTest, ToLower) { EXPECT_EQ('a', ToLower('a')); EXPECT_EQ('a', ToLower('A')); } + +TEST(UtilTest, Malloc) { + using node::Malloc; + EXPECT_NE(nullptr, Malloc(0)); + EXPECT_NE(nullptr, Malloc(1)); +} + +TEST(UtilTest, Calloc) { + using node::Calloc; + EXPECT_NE(nullptr, Calloc(0, 0)); + EXPECT_NE(nullptr, Calloc(1, 0)); + EXPECT_NE(nullptr, Calloc(0, 1)); + EXPECT_NE(nullptr, Calloc(1, 1)); +} \ No newline at end of file diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index 0b0ab13cbbfc5d..41940af4b91690 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -79,3 +79,11 @@ assert.throws(function() { assert.throws(function() { crypto.pbkdf2('password', 'salt', 1, -1, common.fail); }, /Bad key length/); + +// Should not get FATAL ERROR with empty password and salt +// https://github.com/nodejs/node/issues/8571 +assert.doesNotThrow(() => { + crypto.pbkdf2('', '', 1, 32, 'sha256', common.mustCall((e) => { + assert.ifError(e); + })); +});