From 31975bbc88d353cf2eecc93c9d2cde62dba67b2c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 25 Feb 2019 04:12:19 +0100 Subject: src: allow not materializing ArrayBuffers from C++ Where appropriate, use a helper that wraps around `ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()` rather than `Buffer::Data()`, as that may help to avoid materializing the underlying `ArrayBuffer` when reading small typed arrays from C++. This allows keeping the performance benefits of the faster creation of heap-allocated small typed arrays in many cases. PR-URL: https://github.com/nodejs/node/pull/26301 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- src/node_crypto.cc | 227 ++++++++++++++++++++++++-------------------------- src/node_crypto.h | 7 +- src/node_http2.cc | 26 +++--- src/node_http2.h | 5 +- src/string_decoder.cc | 9 +- src/util-inl.h | 26 ++++++ src/util.cc | 7 +- src/util.h | 21 +++++ 8 files changed, 183 insertions(+), 145 deletions(-) (limited to 'src') diff --git a/src/node_crypto.cc b/src/node_crypto.cc index dcdecb5064..aca5df298c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -57,6 +57,7 @@ namespace crypto { using node::THROW_ERR_TLS_INVALID_PROTOCOL_METHOD; using v8::Array; +using v8::ArrayBufferView; using v8::Boolean; using v8::ConstructorBehavior; using v8::Context; @@ -539,8 +540,9 @@ static BIOPointer LoadBIO(Environment* env, Local v) { return NodeBIO::NewFixed(*s, s.length()); } - if (Buffer::HasInstance(v)) { - return NodeBIO::NewFixed(Buffer::Data(v), Buffer::Length(v)); + if (v->IsArrayBufferView()) { + ArrayBufferViewContents buf(v.As()); + return NodeBIO::NewFixed(buf.data(), buf.length()); } return nullptr; @@ -1135,9 +1137,10 @@ void SecureContext::LoadPKCS12(const FunctionCallbackInfo& args) { if (args.Length() >= 2) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[1], "Pass phrase"); - size_t passlen = Buffer::Length(args[1]); + Local abv = args[1].As(); + size_t passlen = abv->ByteLength(); pass.resize(passlen + 1); - memcpy(pass.data(), Buffer::Data(args[1]), passlen); + abv->CopyContents(pass.data(), passlen); pass[passlen] = '\0'; } @@ -1261,15 +1264,16 @@ void SecureContext::SetTicketKeys(const FunctionCallbackInfo& args) { } THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Ticket keys"); + ArrayBufferViewContents buf(args[0].As()); - if (Buffer::Length(args[0]) != 48) { + if (buf.length() != 48) { return THROW_ERR_INVALID_ARG_VALUE( env, "Ticket keys length must be 48 bytes"); } - memcpy(wrap->ticket_key_name_, Buffer::Data(args[0]), 16); - memcpy(wrap->ticket_key_hmac_, Buffer::Data(args[0]) + 16, 16); - memcpy(wrap->ticket_key_aes_, Buffer::Data(args[0]) + 32, 16); + memcpy(wrap->ticket_key_name_, buf.data(), 16); + memcpy(wrap->ticket_key_hmac_, buf.data() + 16, 16); + memcpy(wrap->ticket_key_aes_, buf.data() + 32, 16); args.GetReturnValue().Set(true); #endif // !def(OPENSSL_NO_TLSEXT) && def(SSL_CTX_get_tlsext_ticket_keys) @@ -1349,29 +1353,29 @@ int SecureContext::TicketKeyCallback(SSL* ssl, return -1; } - memcpy(name, Buffer::Data(name_val), kTicketPartSize); - memcpy(iv, Buffer::Data(iv_val), kTicketPartSize); + name_val.As()->CopyContents(name, kTicketPartSize); + iv_val.As()->CopyContents(iv, kTicketPartSize); } + ArrayBufferViewContents hmac_buf(hmac); HMAC_Init_ex(hctx, - Buffer::Data(hmac), - Buffer::Length(hmac), + hmac_buf.data(), + hmac_buf.length(), EVP_sha256(), nullptr); - const unsigned char* aes_key = - reinterpret_cast(Buffer::Data(aes)); + ArrayBufferViewContents aes_key(aes.As()); if (enc) { EVP_EncryptInit_ex(ectx, EVP_aes_128_cbc(), nullptr, - aes_key, + aes_key.data(), iv); } else { EVP_DecryptInit_ex(ectx, EVP_aes_128_cbc(), nullptr, - aes_key, + aes_key.data(), iv); } @@ -2094,13 +2098,10 @@ void SSLWrap::SetSession(const FunctionCallbackInfo& args) { } THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Session"); - size_t slen = Buffer::Length(args[0]); - std::vector sbuf(slen); - if (char* p = Buffer::Data(args[0])) - sbuf.assign(p, p + slen); + ArrayBufferViewContents sbuf(args[0].As()); - const unsigned char* p = reinterpret_cast(sbuf.data()); - SSLSessionPointer sess(d2i_SSL_SESSION(nullptr, &p, slen)); + const unsigned char* p = sbuf.data(); + SSLSessionPointer sess(d2i_SSL_SESSION(nullptr, &p, sbuf.length())); if (sess == nullptr) return; @@ -2118,11 +2119,10 @@ void SSLWrap::LoadSession(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&w, args.Holder()); if (args.Length() >= 1 && Buffer::HasInstance(args[0])) { - ssize_t slen = Buffer::Length(args[0]); - char* sbuf = Buffer::Data(args[0]); + ArrayBufferViewContents sbuf(args[0]); - const unsigned char* p = reinterpret_cast(sbuf); - SSL_SESSION* sess = d2i_SSL_SESSION(nullptr, &p, slen); + const unsigned char* p = sbuf.data(); + SSL_SESSION* sess = d2i_SSL_SESSION(nullptr, &p, sbuf.length()); // Setup next session and move hello to the BIO buffer w->next_sess_.reset(sess); @@ -2204,7 +2204,7 @@ void SSLWrap::SetOCSPResponse(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "OCSP response"); - w->ocsp_response_.Reset(args.GetIsolate(), args[0].As()); + w->ocsp_response_.Reset(args.GetIsolate(), args[0].As()); } @@ -2403,12 +2403,10 @@ int SSLWrap::SelectALPNCallback(SSL* s, w->object()->GetPrivate( env->context(), env->alpn_buffer_private_symbol()).ToLocalChecked(); - CHECK(Buffer::HasInstance(alpn_buffer)); - const unsigned char* alpn_protos = - reinterpret_cast(Buffer::Data(alpn_buffer)); - unsigned alpn_protos_len = Buffer::Length(alpn_buffer); + ArrayBufferViewContents alpn_protos(alpn_buffer); int status = SSL_select_next_proto(const_cast(out), outlen, - alpn_protos, alpn_protos_len, in, inlen); + alpn_protos.data(), alpn_protos.length(), + in, inlen); // According to 3.2. Protocol Selection of RFC7301, fatal // no_application_protocol alert shall be sent but OpenSSL 1.0.2 does not // support it yet. See @@ -2446,10 +2444,9 @@ void SSLWrap::SetALPNProtocols(const FunctionCallbackInfo& args) { return env->ThrowTypeError("Must give a Buffer as first argument"); if (w->is_client()) { - const unsigned char* alpn_protos = - reinterpret_cast(Buffer::Data(args[0])); - unsigned alpn_protos_len = Buffer::Length(args[0]); - int r = SSL_set_alpn_protos(w->ssl_.get(), alpn_protos, alpn_protos_len); + ArrayBufferViewContents alpn_protos(args[0]); + int r = SSL_set_alpn_protos( + w->ssl_.get(), alpn_protos.data(), alpn_protos.length()); CHECK_EQ(r, 0); } else { CHECK( @@ -2493,14 +2490,13 @@ int SSLWrap::TLSExtStatusCallback(SSL* s, void* arg) { if (w->ocsp_response_.IsEmpty()) return SSL_TLSEXT_ERR_NOACK; - Local obj = PersistentToLocal::Default(env->isolate(), - w->ocsp_response_); - char* resp = Buffer::Data(obj); - size_t len = Buffer::Length(obj); + Local obj = PersistentToLocal::Default(env->isolate(), + w->ocsp_response_); + size_t len = obj->ByteLength(); // OpenSSL takes control of the pointer after accepting it unsigned char* data = MallocOpenSSL(len); - memcpy(data, resp, len); + obj->CopyContents(data, len); if (!SSL_set_tlsext_status_ocsp_resp(s, data, len)) OPENSSL_free(data); @@ -2998,10 +2994,12 @@ ByteSource ByteSource::FromString(Environment* env, Local str, } ByteSource ByteSource::FromBuffer(Local buffer, bool ntc) { - size_t size = Buffer::Length(buffer); + CHECK(buffer->IsArrayBufferView()); + Local abv = buffer.As(); + size_t size = abv->ByteLength(); if (ntc) { char* data = MallocOpenSSL(size + 1); - memcpy(data, Buffer::Data(buffer), size); + abv->CopyContents(data, size); data[size] = 0; return Allocated(data, size); } @@ -3404,7 +3402,8 @@ void KeyObject::Init(const FunctionCallbackInfo& args) { switch (key->key_type_) { case kKeyTypeSecret: CHECK_EQ(args.Length(), 1); - key->InitSecret(Buffer::Data(args[0]), Buffer::Length(args[0])); + CHECK(args[0]->IsArrayBufferView()); + key->InitSecret(args[0].As()); break; case kKeyTypePublic: CHECK_EQ(args.Length(), 3); @@ -3429,11 +3428,12 @@ void KeyObject::Init(const FunctionCallbackInfo& args) { } } -void KeyObject::InitSecret(const char* key, size_t key_len) { +void KeyObject::InitSecret(v8::Local abv) { CHECK_EQ(this->key_type_, kKeyTypeSecret); + size_t key_len = abv->ByteLength(); char* mem = MallocOpenSSL(key_len); - memcpy(mem, key, key_len); + abv->CopyContents(mem, key_len); this->symmetric_key_ = std::unique_ptr>(mem, [key_len](char* p) { OPENSSL_clear_free(p, key_len); @@ -3643,8 +3643,7 @@ void CipherBase::Init(const FunctionCallbackInfo& args) { CHECK_GE(args.Length(), 3); const node::Utf8Value cipher_type(args.GetIsolate(), args[0]); - const char* key_buf = Buffer::Data(args[1]); - ssize_t key_buf_len = Buffer::Length(args[1]); + ArrayBufferViewContents key_buf(args[1]); // Don't assign to cipher->auth_tag_len_ directly; the value might not // represent a valid length at this point. @@ -3656,7 +3655,7 @@ void CipherBase::Init(const FunctionCallbackInfo& args) { auth_tag_len = kNoAuthTagLength; } - cipher->Init(*cipher_type, key_buf, key_buf_len, auth_tag_len); + cipher->Init(*cipher_type, key_buf.data(), key_buf.length(), auth_tag_len); } void CipherBase::InitIv(const char* cipher_type, @@ -3712,14 +3711,12 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { const node::Utf8Value cipher_type(env->isolate(), args[0]); const ByteSource key = GetSecretKeyBytes(env, args[1]); - ssize_t iv_len; - const unsigned char* iv_buf; - if (args[2]->IsNull()) { - iv_buf = nullptr; - iv_len = -1; - } else { - iv_buf = reinterpret_cast(Buffer::Data(args[2])); - iv_len = Buffer::Length(args[2]); + ArrayBufferViewContents iv_buf; + ssize_t iv_len = -1; + if (!args[2]->IsNull()) { + CHECK(args[2]->IsArrayBufferView()); + iv_buf.Read(args[2].As()); + iv_len = iv_buf.length(); } // Don't assign to cipher->auth_tag_len_ directly; the value might not @@ -3735,7 +3732,7 @@ void CipherBase::InitIv(const FunctionCallbackInfo& args) { cipher->InitIv(*cipher_type, reinterpret_cast(key.get()), key.size(), - iv_buf, + iv_buf.data(), iv_len, auth_tag_len); } @@ -3889,7 +3886,8 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { CHECK_LE(cipher->auth_tag_len_, sizeof(cipher->auth_tag_)); memset(cipher->auth_tag_, 0, sizeof(cipher->auth_tag_)); - memcpy(cipher->auth_tag_, Buffer::Data(args[0]), cipher->auth_tag_len_); + args[0].As()->CopyContents( + cipher->auth_tag_, cipher->auth_tag_len_); args.GetReturnValue().Set(true); } @@ -3953,9 +3951,9 @@ void CipherBase::SetAAD(const FunctionCallbackInfo& args) { CHECK_EQ(args.Length(), 2); CHECK(args[1]->IsInt32()); int plaintext_len = args[1].As()->Value(); + ArrayBufferViewContents buf(args[0]); - bool b = cipher->SetAAD(Buffer::Data(args[0]), Buffer::Length(args[0]), - plaintext_len); + bool b = cipher->SetAAD(buf.data(), buf.length(), plaintext_len); args.GetReturnValue().Set(b); // Possibly report invalid state failure } @@ -4028,9 +4026,8 @@ void CipherBase::Update(const FunctionCallbackInfo& args) { return; r = cipher->Update(decoder.out(), decoder.size(), &out); } else { - char* buf = Buffer::Data(args[0]); - size_t buflen = Buffer::Length(args[0]); - r = cipher->Update(buf, buflen, &out); + ArrayBufferViewContents buf(args[0]); + r = cipher->Update(buf.data(), buf.length(), &out); } if (r != kSuccess) { @@ -4213,10 +4210,8 @@ void Hmac::HmacUpdate(const FunctionCallbackInfo& args) { r = hmac->HmacUpdate(decoder.out(), decoder.size()); } } else { - CHECK(args[0]->IsArrayBufferView()); - char* buf = Buffer::Data(args[0]); - size_t buflen = Buffer::Length(args[0]); - r = hmac->HmacUpdate(buf, buflen); + ArrayBufferViewContents buf(args[0]); + r = hmac->HmacUpdate(buf.data(), buf.length()); } args.GetReturnValue().Set(r); @@ -4324,9 +4319,8 @@ void Hash::HashUpdate(const FunctionCallbackInfo& args) { } r = hash->HashUpdate(decoder.out(), decoder.size()); } else if (args[0]->IsArrayBufferView()) { - char* buf = Buffer::Data(args[0]); - size_t buflen = Buffer::Length(args[0]); - r = hash->HashUpdate(buf, buflen); + ArrayBufferViewContents buf(args[0].As()); + r = hash->HashUpdate(buf.data(), buf.length()); } args.GetReturnValue().Set(r); @@ -4487,9 +4481,8 @@ void Sign::SignUpdate(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&sign, args.Holder()); Error err; - char* buf = Buffer::Data(args[0]); - size_t buflen = Buffer::Length(args[0]); - err = sign->Update(buf, buflen); + ArrayBufferViewContents buf(args[0]); + err = sign->Update(buf.data(), buf.length()); sign->CheckThrow(err); } @@ -4634,9 +4627,8 @@ void Verify::VerifyUpdate(const FunctionCallbackInfo& args) { ASSIGN_OR_RETURN_UNWRAP(&verify, args.Holder()); Error err; - char* buf = Buffer::Data(args[0]); - size_t buflen = Buffer::Length(args[0]); - err = verify->Update(buf, buflen); + ArrayBufferViewContents buf(args[0]); + err = verify->Update(buf.data(), buf.length()); verify->CheckThrow(err); } @@ -4688,8 +4680,7 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { if (!pkey) return; - char* hbuf = Buffer::Data(args[offset]); - ssize_t hlen = Buffer::Length(args[offset]); + ArrayBufferViewContents hbuf(args[offset]); CHECK(args[offset + 1]->IsInt32()); int padding = args[offset + 1].As()->Value(); @@ -4698,8 +4689,8 @@ void Verify::VerifyFinal(const FunctionCallbackInfo& args) { int salt_len = args[offset + 2].As()->Value(); bool verify_result; - Error err = verify->VerifyFinal(pkey, hbuf, hlen, padding, salt_len, - &verify_result); + Error err = verify->VerifyFinal(pkey, hbuf.data(), hbuf.length(), padding, + salt_len, &verify_result); if (err != kSignOk) return verify->CheckThrow(err); args.GetReturnValue().Set(verify_result); @@ -4753,8 +4744,7 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { return; THROW_AND_RETURN_IF_NOT_BUFFER(env, args[offset], "Data"); - char* buf = Buffer::Data(args[offset]); - ssize_t len = Buffer::Length(args[offset]); + ArrayBufferViewContents buf(args[offset]); uint32_t padding; if (!args[offset + 1]->Uint32Value(env->context()).To(&padding)) return; @@ -4767,8 +4757,8 @@ void PublicKeyCipher::Cipher(const FunctionCallbackInfo& args) { env, pkey, padding, - reinterpret_cast(buf), - len, + buf.data(), + buf.length(), &out); if (!r) @@ -4906,15 +4896,15 @@ void DiffieHellman::New(const FunctionCallbackInfo& args) { args[1].As()->Value()); } } else { + ArrayBufferViewContents arg0(args[0]); if (args[1]->IsInt32()) { - initialized = diffieHellman->Init(Buffer::Data(args[0]), - Buffer::Length(args[0]), + initialized = diffieHellman->Init(arg0.data(), + arg0.length(), args[1].As()->Value()); } else { - initialized = diffieHellman->Init(Buffer::Data(args[0]), - Buffer::Length(args[0]), - Buffer::Data(args[1]), - Buffer::Length(args[1])); + ArrayBufferViewContents arg1(args[1]); + initialized = diffieHellman->Init(arg0.data(), arg0.length(), + arg1.data(), arg1.length()); } } } @@ -5017,10 +5007,8 @@ void DiffieHellman::ComputeSecret(const FunctionCallbackInfo& args) { } THROW_AND_RETURN_IF_NOT_BUFFER(env, args[0], "Other party's public key"); - BignumPointer key(BN_bin2bn( - reinterpret_cast(Buffer::Data(args[0])), - Buffer::Length(args[0]), - nullptr)); + ArrayBufferViewContents key_buf(args[0].As()); + BignumPointer key(BN_bin2bn(key_buf.data(), key_buf.length(), nullptr)); AllocatedBuffer ret = env->AllocateManaged(DH_size(diffieHellman->dh_.get())); @@ -5087,9 +5075,9 @@ void DiffieHellman::SetKey(const FunctionCallbackInfo& args, return THROW_ERR_INVALID_ARG_TYPE(env, errmsg); } + ArrayBufferViewContents buf(args[0].As()); BIGNUM* num = - BN_bin2bn(reinterpret_cast(Buffer::Data(args[0])), - Buffer::Length(args[0]), nullptr); + BN_bin2bn(buf.data(), buf.length(), nullptr); CHECK_NOT_NULL(num); CHECK_EQ(1, set_field(dh->dh_.get(), num)); } @@ -5188,8 +5176,7 @@ void ECDH::GenerateKeys(const FunctionCallbackInfo& args) { ECPointPointer ECDH::BufferToPoint(Environment* env, const EC_GROUP* group, - char* data, - size_t len) { + Local buf) { int r; ECPointPointer pub(EC_POINT_new(group)); @@ -5198,11 +5185,12 @@ ECPointPointer ECDH::BufferToPoint(Environment* env, return pub; } + ArrayBufferViewContents input(buf); r = EC_POINT_oct2point( group, pub.get(), - reinterpret_cast(data), - len, + input.data(), + input.length(), nullptr); if (!r) return ECPointPointer(); @@ -5227,8 +5215,7 @@ void ECDH::ComputeSecret(const FunctionCallbackInfo& args) { ECPointPointer pub( ECDH::BufferToPoint(env, ecdh->group_, - Buffer::Data(args[0]), - Buffer::Length(args[0]))); + args[0])); if (!pub) { args.GetReturnValue().Set( FIXED_ONE_BYTE_STRING(env->isolate(), @@ -5360,8 +5347,7 @@ void ECDH::SetPublicKey(const FunctionCallbackInfo& args) { ECPointPointer pub( ECDH::BufferToPoint(env, ecdh->group_, - Buffer::Data(args[0].As()), - Buffer::Length(args[0].As()))); + args[0])); if (!pub) return env->ThrowError("Failed to convert Buffer to EC_POINT"); @@ -5427,8 +5413,10 @@ void CryptoJob::Run(std::unique_ptr job, Local wrap) { inline void CopyBuffer(Local buf, std::vector* vec) { + CHECK(buf->IsArrayBufferView()); vec->clear(); - if (auto p = Buffer::Data(buf)) vec->assign(p, p + Buffer::Length(buf)); + vec->resize(buf.As()->ByteLength()); + buf.As()->CopyContents(vec->data(), vec->size()); } @@ -6033,14 +6021,13 @@ bool VerifySpkac(const char* data, unsigned int len) { void VerifySpkac(const FunctionCallbackInfo& args) { bool verify_result = false; - size_t length = Buffer::Length(args[0]); - if (length == 0) - return args.GetReturnValue().Set(verify_result); + ArrayBufferViewContents input(args[0]); + if (input.length() == 0) + return args.GetReturnValue().SetEmptyString(); - char* data = Buffer::Data(args[0]); - CHECK_NOT_NULL(data); + CHECK_NOT_NULL(input.data()); - verify_result = VerifySpkac(data, length); + verify_result = VerifySpkac(input.data(), input.length()); args.GetReturnValue().Set(verify_result); } @@ -6075,15 +6062,15 @@ AllocatedBuffer ExportPublicKey(Environment* env, void ExportPublicKey(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - size_t length = Buffer::Length(args[0]); - if (length == 0) + ArrayBufferViewContents input(args[0]); + if (input.length() == 0) return args.GetReturnValue().SetEmptyString(); - char* data = Buffer::Data(args[0]); - CHECK_NOT_NULL(data); + CHECK_NOT_NULL(input.data()); size_t pkey_size; - AllocatedBuffer pkey = ExportPublicKey(env, data, length, &pkey_size); + AllocatedBuffer pkey = + ExportPublicKey(env, input.data(), input.length(), &pkey_size); if (pkey.data() == nullptr) return args.GetReturnValue().SetEmptyString(); @@ -6130,8 +6117,9 @@ void ConvertKey(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 3); + CHECK(args[0]->IsArrayBufferView()); - size_t len = Buffer::Length(args[0]); + size_t len = args[0].As()->ByteLength(); if (len == 0) return args.GetReturnValue().SetEmptyString(); @@ -6149,8 +6137,7 @@ void ConvertKey(const FunctionCallbackInfo& args) { ECPointPointer pub( ECDH::BufferToPoint(env, group.get(), - Buffer::Data(args[0]), - len)); + args[0])); if (pub == nullptr) return env->ThrowError("Failed to convert Buffer to EC_POINT"); diff --git a/src/node_crypto.h b/src/node_crypto.h index ce93412830..982fc70543 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -333,7 +333,7 @@ class SSLWrap { ClientHelloParser hello_parser_; - Persistent ocsp_response_; + Persistent ocsp_response_; Persistent sni_context_; friend class SecureContext; @@ -462,7 +462,7 @@ class KeyObject : public BaseObject { static void New(const v8::FunctionCallbackInfo& args); static void Init(const v8::FunctionCallbackInfo& args); - void InitSecret(const char* key, size_t key_len); + void InitSecret(v8::Local abv); void InitPublic(const ManagedEVPPKey& pkey); void InitPrivate(const ManagedEVPPKey& pkey); @@ -803,8 +803,7 @@ class ECDH : public BaseObject { static void Initialize(Environment* env, v8::Local target); static ECPointPointer BufferToPoint(Environment* env, const EC_GROUP* group, - char* data, - size_t len); + v8::Local buf); // TODO(joyeecheung): track the memory used by OpenSSL types SET_NO_MEMORY_INFO() diff --git a/src/node_http2.cc b/src/node_http2.cc index 7fc21c9cae..8946c71fc3 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -12,6 +12,7 @@ namespace node { using v8::ArrayBuffer; +using v8::ArrayBufferView; using v8::Boolean; using v8::Context; using v8::Float64Array; @@ -2483,7 +2484,7 @@ void Http2Session::Request(const FunctionCallbackInfo& args) { // state of the Http2Session, it's simply a notification. void Http2Session::Goaway(uint32_t code, int32_t lastStreamID, - uint8_t* data, + const uint8_t* data, size_t len) { if (IsDestroyed()) return; @@ -2508,16 +2509,13 @@ void Http2Session::Goaway(const FunctionCallbackInfo& args) { uint32_t code = args[0]->Uint32Value(context).ToChecked(); int32_t lastStreamID = args[1]->Int32Value(context).ToChecked(); - Local opaqueData = args[2]; - uint8_t* data = nullptr; - size_t length = 0; + ArrayBufferViewContents opaque_data; - if (Buffer::HasInstance(opaqueData)) { - data = reinterpret_cast(Buffer::Data(opaqueData)); - length = Buffer::Length(opaqueData); + if (args[2]->IsArrayBufferView()) { + opaque_data.Read(args[2].As()); } - session->Goaway(code, lastStreamID, data, length); + session->Goaway(code, lastStreamID, opaque_data.data(), opaque_data.length()); } // Update accounting of data chunks. This is used primarily to manage timeout @@ -2771,10 +2769,10 @@ void Http2Session::Ping(const FunctionCallbackInfo& args) { // A PING frame may have exactly 8 bytes of payload data. If not provided, // then the current hrtime will be used as the payload. - uint8_t* payload = nullptr; - if (Buffer::HasInstance(args[0])) { - payload = reinterpret_cast(Buffer::Data(args[0])); - CHECK_EQ(Buffer::Length(args[0]), 8); + ArrayBufferViewContents payload; + if (args[0]->IsArrayBufferView()) { + payload.Read(args[0].As()); + CHECK_EQ(payload.length(), 8); } Local obj; @@ -2799,7 +2797,7 @@ void Http2Session::Ping(const FunctionCallbackInfo& args) { // the callback will be invoked and a notification sent out to JS land. The // notification will include the duration of the ping, allowing the round // trip to be measured. - ping->Send(payload); + ping->Send(payload.data()); args.GetReturnValue().Set(true); } @@ -2871,7 +2869,7 @@ Http2Session::Http2Ping::Http2Ping(Http2Session* session, Local obj) session_(session), startTime_(uv_hrtime()) {} -void Http2Session::Http2Ping::Send(uint8_t* payload) { +void Http2Session::Http2Ping::Send(const uint8_t* payload) { uint8_t data[8]; if (payload == nullptr) { memcpy(&data, &startTime_, arraysize(data)); diff --git a/src/node_http2.h b/src/node_http2.h index 9065718c60..8a8ef74af9 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -699,7 +699,8 @@ class Http2Session : public AsyncWrap, public StreamListener { void Close(uint32_t code = NGHTTP2_NO_ERROR, bool socket_closed = false); void Consume(Local external); - void Goaway(uint32_t code, int32_t lastStreamID, uint8_t* data, size_t len); + void Goaway(uint32_t code, int32_t lastStreamID, + const uint8_t* data, size_t len); void AltSvc(int32_t id, uint8_t* origin, size_t origin_len, @@ -1089,7 +1090,7 @@ class Http2Session::Http2Ping : public AsyncWrap { SET_MEMORY_INFO_NAME(Http2Ping) SET_SELF_SIZE(Http2Ping) - void Send(uint8_t* payload); + void Send(const uint8_t* payload); void Done(bool ack, const uint8_t* payload = nullptr); private: diff --git a/src/string_decoder.cc b/src/string_decoder.cc index 9cf1bd671b..1441ca8693 100644 --- a/src/string_decoder.cc +++ b/src/string_decoder.cc @@ -4,6 +4,7 @@ #include "string_decoder-inl.h" using v8::Array; +using v8::ArrayBufferView; using v8::Context; using v8::FunctionCallbackInfo; using v8::Integer; @@ -252,9 +253,13 @@ void DecodeData(const FunctionCallbackInfo& args) { StringDecoder* decoder = reinterpret_cast(Buffer::Data(args[0])); CHECK_NOT_NULL(decoder); - size_t nread = Buffer::Length(args[1]); + + CHECK(args[1]->IsArrayBufferView()); + ArrayBufferViewContents content(args[1].As()); + size_t length = content.length(); + MaybeLocal ret = - decoder->DecodeData(args.GetIsolate(), Buffer::Data(args[1]), &nread); + decoder->DecodeData(args.GetIsolate(), content.data(), &length); if (!ret.IsEmpty()) args.GetReturnValue().Set(ret.ToLocalChecked()); } diff --git a/src/util-inl.h b/src/util-inl.h index 9f57b635db..0b174170ca 100644 --- a/src/util-inl.h +++ b/src/util-inl.h @@ -466,6 +466,32 @@ SlicedArguments::SlicedArguments( (*this)[i] = args[i + start]; } +template +ArrayBufferViewContents::ArrayBufferViewContents( + v8::Local value) { + CHECK(value->IsArrayBufferView()); + Read(value.As()); +} + +template +ArrayBufferViewContents::ArrayBufferViewContents( + v8::Local abv) { + Read(abv); +} + +template +void ArrayBufferViewContents::Read(v8::Local abv) { + static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment"); + length_ = abv->ByteLength(); + if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) { + data_ = static_cast(abv->Buffer()->GetContents().Data()) + + abv->ByteOffset(); + } else { + abv->CopyContents(stack_storage_, sizeof(stack_storage_)); + data_ = stack_storage_; + } +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/util.cc b/src/util.cc index 3cdc222a96..223e0e8772 100644 --- a/src/util.cc +++ b/src/util.cc @@ -29,6 +29,7 @@ namespace node { +using v8::ArrayBufferView; using v8::Isolate; using v8::Local; using v8::String; @@ -89,11 +90,11 @@ BufferValue::BufferValue(Isolate* isolate, Local value) { if (value->IsString()) { MakeUtf8String(isolate, value, this); - } else if (Buffer::HasInstance(value)) { - const size_t len = Buffer::Length(value); + } else if (value->IsArrayBufferView()) { + const size_t len = value.As()->ByteLength(); // Leave place for the terminating '\0' byte. AllocateSufficientStorage(len + 1); - memcpy(out(), Buffer::Data(value), len); + value.As()->CopyContents(out(), len); SetLengthAndZeroTerminate(len); } else { Invalidate(); diff --git a/src/util.h b/src/util.h index 4cd04dc6af..4c930e637c 100644 --- a/src/util.h +++ b/src/util.h @@ -417,6 +417,27 @@ class MaybeStackBuffer { T buf_st_[kStackStorageSize]; }; +// Provides access to an ArrayBufferView's storage, either the original, +// or for small data, a copy of it. This object's lifetime is bound to the +// original ArrayBufferView's lifetime. +template +class ArrayBufferViewContents { + public: + ArrayBufferViewContents() {} + + explicit inline ArrayBufferViewContents(v8::Local value); + explicit inline ArrayBufferViewContents(v8::Local abv); + inline void Read(v8::Local abv); + + inline const T* data() const { return data_; } + inline size_t length() const { return length_; } + + private: + T stack_storage_[kStackStorageSize]; + T* data_ = nullptr; + size_t length_ = 0; +}; + class Utf8Value : public MaybeStackBuffer { public: explicit Utf8Value(v8::Isolate* isolate, v8::Local value); -- cgit v1.2.3