From 329e162162366b4309e467619cff3f843fe79a79 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Wed, 13 Dec 2017 22:35:32 +0000 Subject: [PATCH 01/16] Change to using SetAccessorProperty instead of SetAccessor in StreamBase Fixes: https://github.com/nodejs/node/issues/17636 Refs: https://github.com/nodejs/node/pull/16482 Refs: https://github.com/nodejs/node/pull/16860 --- src/stream_base-inl.h | 74 ++++++++++--------- src/stream_base.h | 9 +-- .../test-stream-base-prototype-accessors.js | 20 ++++- 3 files changed, 61 insertions(+), 42 deletions(-) diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 29739011c6a439..f53079541f3708 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -11,7 +11,7 @@ namespace node { -using v8::AccessorSignature; +using v8::Signature; using v8::External; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -34,31 +34,42 @@ void StreamBase::AddMethods(Environment* env, enum PropertyAttribute attributes = static_cast( v8::ReadOnly | v8::DontDelete | v8::DontEnum); - Local signature = - AccessorSignature::New(env->isolate(), t); - t->PrototypeTemplate()->SetAccessor(env->fd_string(), - GetFD, - nullptr, - env->as_external(), - v8::DEFAULT, - attributes, - signature); - - t->PrototypeTemplate()->SetAccessor(env->external_stream_string(), - GetExternal, - nullptr, - env->as_external(), - v8::DEFAULT, - attributes, - signature); - - t->PrototypeTemplate()->SetAccessor(env->bytes_read_string(), - GetBytesRead, - nullptr, - env->as_external(), - v8::DEFAULT, - attributes, - signature); + + Local signature = + Signature::New(env->isolate(), t); + + Local get_fd_templ = FunctionTemplate::New( + env->isolate(), + GetFD, + Local(), + signature); + + Local get_external_templ = FunctionTemplate::New( + env->isolate(), + GetExternal, + Local(), + signature); + + Local get_bytes_read = FunctionTemplate::New( + env->isolate(), + GetBytesRead, + Local(), + signature); + + t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), + get_fd_templ, + Local(), + attributes); + + t->PrototypeTemplate()->SetAccessorProperty(env->external_stream_string(), + get_external_templ, + Local(), + attributes); + + t->PrototypeTemplate()->SetAccessorProperty(env->bytes_read_string(), + get_bytes_read, + Local(), + attributes); env->SetProtoMethod(t, "readStart", JSMethod); env->SetProtoMethod(t, "readStop", JSMethod); @@ -85,8 +96,7 @@ void StreamBase::AddMethods(Environment* env, template -void StreamBase::GetFD(Local key, - const PropertyCallbackInfo& args) { +void StreamBase::GetFD(const FunctionCallbackInfo& args) { // Mimic implementation of StreamBase::GetFD() and UDPWrap::GetFD(). Base* handle; ASSIGN_OR_RETURN_UNWRAP(&handle, @@ -100,10 +110,8 @@ void StreamBase::GetFD(Local key, args.GetReturnValue().Set(wrap->GetFD()); } - template -void StreamBase::GetBytesRead(Local key, - const PropertyCallbackInfo& args) { +void StreamBase::GetBytesRead(const FunctionCallbackInfo& args) { // The handle instance hasn't been set. So no bytes could have been read. Base* handle; ASSIGN_OR_RETURN_UNWRAP(&handle, @@ -115,10 +123,8 @@ void StreamBase::GetBytesRead(Local key, args.GetReturnValue().Set(static_cast(wrap->bytes_read_)); } - template -void StreamBase::GetExternal(Local key, - const PropertyCallbackInfo& args) { +void StreamBase::GetExternal(const FunctionCallbackInfo& args) { Base* handle; ASSIGN_OR_RETURN_UNWRAP(&handle, args.This()); diff --git a/src/stream_base.h b/src/stream_base.h index 5bf5f013947347..2269aa199b9dbc 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -265,16 +265,13 @@ class StreamBase : public StreamResource { int WriteString(const v8::FunctionCallbackInfo& args); template - static void GetFD(v8::Local key, - const v8::PropertyCallbackInfo& args); + static void GetFD(const v8::FunctionCallbackInfo& args); template - static void GetExternal(v8::Local key, - const v8::PropertyCallbackInfo& args); + static void GetExternal(const v8::FunctionCallbackInfo& args); template - static void GetBytesRead(v8::Local key, - const v8::PropertyCallbackInfo& args); + static void GetBytesRead(const v8::FunctionCallbackInfo& args); template { TTY.prototype.bytesRead; }, msg); @@ -24,4 +24,20 @@ const TTY = process.binding('tty_wrap').TTY; assert.throws(() => { TTY.prototype._externalStream; }, msg); + + // Should not throw for Object.getOwnPropertyDescriptor + assert.strictEqual( + typeof (Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead')), + 'object' + ); + + assert.strictEqual( + typeof (Object.getOwnPropertyDescriptor(TTY.prototype, 'fd')), + 'object' + ); + + assert.strictEqual( + typeof (Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream')), + 'object' + ); } From dcca3a5b168246710be28d06d765821ee7b96655 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Wed, 13 Dec 2017 23:55:10 +0000 Subject: [PATCH 02/16] Remove extra parentheses --- test/parallel/test-stream-base-prototype-accessors.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-stream-base-prototype-accessors.js b/test/parallel/test-stream-base-prototype-accessors.js index 832b5aa2119cb9..be933410465a95 100644 --- a/test/parallel/test-stream-base-prototype-accessors.js +++ b/test/parallel/test-stream-base-prototype-accessors.js @@ -27,17 +27,17 @@ const TTY = process.binding('tty_wrap').TTY; // Should not throw for Object.getOwnPropertyDescriptor assert.strictEqual( - typeof (Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead')), + typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead'), 'object' ); assert.strictEqual( - typeof (Object.getOwnPropertyDescriptor(TTY.prototype, 'fd')), + typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'fd'), 'object' ); assert.strictEqual( - typeof (Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream')), + typeof Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream'), 'object' ); } From ebbe147f8b787c7d75d8c4513fc8f3665a970470 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 00:52:14 +0000 Subject: [PATCH 03/16] Fix indentation --- src/stream_base-inl.h | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index f53079541f3708..3a5c85be116a61 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -35,26 +35,25 @@ void StreamBase::AddMethods(Environment* env, static_cast( v8::ReadOnly | v8::DontDelete | v8::DontEnum); - Local signature = - Signature::New(env->isolate(), t); - - Local get_fd_templ = FunctionTemplate::New( - env->isolate(), - GetFD, - Local(), - signature); - - Local get_external_templ = FunctionTemplate::New( - env->isolate(), - GetExternal, - Local(), - signature); - - Local get_bytes_read = FunctionTemplate::New( - env->isolate(), - GetBytesRead, - Local(), - signature); + Local signature = Signature::New(env->isolate(), t); + + Local get_fd_templ = + FunctionTemplate::New(env->isolate(), + GetFD, + Local(), + signature); + + Local get_external_templ = + FunctionTemplate::New(env->isolate(), + GetExternal, + Local(), + signature); + + Local get_bytes_read_templ = + FunctionTemplate::New(env->isolate(), + GetBytesRead, + Local(), + signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), get_fd_templ, @@ -67,7 +66,7 @@ void StreamBase::AddMethods(Environment* env, attributes); t->PrototypeTemplate()->SetAccessorProperty(env->bytes_read_string(), - get_bytes_read, + get_bytes_read_templ, Local(), attributes); From a03028f6e8aad29c23108c012dab939184a1b53c Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 00:59:06 +0000 Subject: [PATCH 04/16] Check for TypeError instead of matching error msg --- test/parallel/test-stream-base-prototype-accessors.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-stream-base-prototype-accessors.js b/test/parallel/test-stream-base-prototype-accessors.js index be933410465a95..c579c55bd1752c 100644 --- a/test/parallel/test-stream-base-prototype-accessors.js +++ b/test/parallel/test-stream-base-prototype-accessors.js @@ -12,18 +12,17 @@ const TTY = process.binding('tty_wrap').TTY; { // Should throw instead of raise assertions - const msg = /TypeError: Illegal invocation/; assert.throws(() => { TTY.prototype.bytesRead; - }, msg); + }, TypeError); assert.throws(() => { TTY.prototype.fd; - }, msg); + }, TypeError); assert.throws(() => { TTY.prototype._externalStream; - }, msg); + }, TypeError); // Should not throw for Object.getOwnPropertyDescriptor assert.strictEqual( From 06d8157c25a769ff6e846da2ae577b53abfd7cde Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 01:11:28 +0000 Subject: [PATCH 05/16] Change PrototypeTemplate SetAccessor to SetAccessorProperty --- src/node_crypto.cc | 42 ++++++++++++++++++++++++------------------ src/node_crypto.h | 6 ++---- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 614c1067c6cc1b..b1f675027293ca 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -81,6 +81,7 @@ namespace node { namespace crypto { using v8::AccessorSignature; +using v8::Signature; using v8::Array; using v8::Boolean; using v8::Context; @@ -544,14 +545,18 @@ void SecureContext::Initialize(Environment* env, Local target) { t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kTicketKeyIVIndex"), Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex)); - t->PrototypeTemplate()->SetAccessor( + Local ctx_getter_templ = + FunctionTemplate::New(env->isolate(), + CtxGetter, + Local(), + Signature::New(env->isolate(), t)); + + + t->PrototypeTemplate()->SetAccessorProperty( FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), - CtxGetter, - nullptr, - env->as_external(), - DEFAULT, - static_cast(ReadOnly | DontDelete), - AccessorSignature::New(env->isolate(), t)); + ctx_getter_templ, + Local(), + static_cast(ReadOnly | DontDelete)); target->Set(secureContextString, t->GetFunction()); env->set_secure_context_constructor_template(t); @@ -1565,8 +1570,7 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl, #endif -void SecureContext::CtxGetter(Local property, - const PropertyCallbackInfo& info) { +void SecureContext::CtxGetter(const FunctionCallbackInfo& info) { SecureContext* sc; ASSIGN_OR_RETURN_UNWRAP(&sc, info.This()); Local ext = External::New(info.GetIsolate(), sc->ctx_); @@ -1636,14 +1640,17 @@ void SSLWrap::AddMethods(Environment* env, Local t) { env->SetProtoMethod(t, "getALPNNegotiatedProtocol", GetALPNNegotiatedProto); env->SetProtoMethod(t, "setALPNProtocols", SetALPNProtocols); - t->PrototypeTemplate()->SetAccessor( + Local ssl_getter_templ = + FunctionTemplate::New(env->isolate(), + SSLGetter, + Local(), + Signature::New(env->isolate(), t)); + + t->PrototypeTemplate()->SetAccessorProperty( FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), - SSLGetter, - nullptr, - env->as_external(), - DEFAULT, - static_cast(ReadOnly | DontDelete), - AccessorSignature::New(env->isolate(), t)); + ssl_getter_templ, + Local(), + static_cast(ReadOnly | DontDelete)); } @@ -2804,8 +2811,7 @@ void SSLWrap::CertCbDone(const FunctionCallbackInfo& args) { template -void SSLWrap::SSLGetter(Local property, - const PropertyCallbackInfo& info) { +void SSLWrap::SSLGetter(const FunctionCallbackInfo& info) { Base* base; ASSIGN_OR_RETURN_UNWRAP(&base, info.This()); SSL* ssl = base->ssl_; diff --git a/src/node_crypto.h b/src/node_crypto.h index 636cbb99d4dde6..efc99c7c142ad0 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -148,8 +148,7 @@ class SecureContext : public BaseObject { const v8::FunctionCallbackInfo& args); static void EnableTicketKeyCallback( const v8::FunctionCallbackInfo& args); - static void CtxGetter(v8::Local property, - const v8::PropertyCallbackInfo& info); + static void CtxGetter(const v8::FunctionCallbackInfo& info); template static void GetCertificate(const v8::FunctionCallbackInfo& args); @@ -329,8 +328,7 @@ class SSLWrap { void* arg); static int TLSExtStatusCallback(SSL* s, void* arg); static int SSLCertCallback(SSL* s, void* arg); - static void SSLGetter(v8::Local property, - const v8::PropertyCallbackInfo& info); + static void SSLGetter(const v8::FunctionCallbackInfo& info); void DestroySSL(); void WaitForCertCb(CertCb cb, void* arg); From 0b96b123cfc12c5a3d1579570bb8e9993681f477 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 01:18:32 +0000 Subject: [PATCH 06/16] Changing SetAccessor to SetAccessorProperty in udp_wrap --- src/node_crypto.cc | 2 +- src/udp_wrap.cc | 22 +++++++++++++++------- src/udp_wrap.h | 3 +-- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index b1f675027293ca..1417701716cef6 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -81,7 +81,6 @@ namespace node { namespace crypto { using v8::AccessorSignature; -using v8::Signature; using v8::Array; using v8::Boolean; using v8::Context; @@ -106,6 +105,7 @@ using v8::Persistent; using v8::PropertyAttribute; using v8::PropertyCallbackInfo; using v8::ReadOnly; +using v8::Signature; using v8::String; using v8::Value; diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index d8665d67892c50..09f54823b9aeb4 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -42,6 +42,7 @@ using v8::Local; using v8::Object; using v8::PropertyAttribute; using v8::PropertyCallbackInfo; +using v8::Signature; using v8::String; using v8::Uint32; using v8::Undefined; @@ -110,12 +111,19 @@ void UDPWrap::Initialize(Local target, enum PropertyAttribute attributes = static_cast(v8::ReadOnly | v8::DontDelete); - t->PrototypeTemplate()->SetAccessor(env->fd_string(), - UDPWrap::GetFD, - nullptr, - env->as_external(), - v8::DEFAULT, - attributes); + + Local signature = Signature::New(env->isolate(), t); + + Local get_fd_templ = + FunctionTemplate::New(env->isolate(), + UDPWrap::GetFD, + Local(), + signature); + + t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), + get_fd_templ, + Local(), + attributes); env->SetProtoMethod(t, "bind", Bind); env->SetProtoMethod(t, "send", Send); @@ -163,7 +171,7 @@ void UDPWrap::New(const FunctionCallbackInfo& args) { } -void UDPWrap::GetFD(Local, const PropertyCallbackInfo& args) { +void UDPWrap::GetFD(const FunctionCallbackInfo& args) { int fd = UV_EBADF; #if !defined(_WIN32) UDPWrap* wrap = Unwrap(args.This()); diff --git a/src/udp_wrap.h b/src/udp_wrap.h index 7f0cc96d34d9bd..b0f90420519520 100644 --- a/src/udp_wrap.h +++ b/src/udp_wrap.h @@ -41,8 +41,7 @@ class UDPWrap: public HandleWrap { static void Initialize(v8::Local target, v8::Local unused, v8::Local context); - static void GetFD(v8::Local, - const v8::PropertyCallbackInfo&); + static void GetFD(const v8::FunctionCallbackInfo& args); static void New(const v8::FunctionCallbackInfo& args); static void Bind(const v8::FunctionCallbackInfo& args); static void Send(const v8::FunctionCallbackInfo& args); From 26ffff19c0f785a5caf93cba3a000d7c59cae888 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 01:31:39 +0000 Subject: [PATCH 07/16] Change InstanceTemplate SetAccessor to SetAccessorProperty --- src/node_crypto.cc | 37 +++++++++++++++++++++---------------- src/node_crypto.h | 4 +--- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 1417701716cef6..dc5833d4f60c98 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4785,14 +4785,17 @@ void DiffieHellman::Initialize(Environment* env, Local target) { env->SetProtoMethod(t, "setPublicKey", SetPublicKey); env->SetProtoMethod(t, "setPrivateKey", SetPrivateKey); - t->InstanceTemplate()->SetAccessor( + Local verify_error_getter_templ = + FunctionTemplate::New(env->isolate(), + DiffieHellman::VerifyErrorGetter, + Local(), + Signature::New(env->isolate(), t)); + + t->InstanceTemplate()->SetAccessorProperty( env->verify_error_string(), - DiffieHellman::VerifyErrorGetter, - nullptr, - env->as_external(), - DEFAULT, - attributes, - AccessorSignature::New(env->isolate(), t)); + verify_error_getter_templ, + Local(), + attributes); target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellman"), t->GetFunction()); @@ -4807,14 +4810,17 @@ void DiffieHellman::Initialize(Environment* env, Local target) { env->SetProtoMethod(t2, "getPublicKey", GetPublicKey); env->SetProtoMethod(t2, "getPrivateKey", GetPrivateKey); - t2->InstanceTemplate()->SetAccessor( + Local verify_error_getter_templ2 = + FunctionTemplate::New(env->isolate(), + DiffieHellman::VerifyErrorGetter, + Local(), + Signature::New(env->isolate(), t2)); + + t2->InstanceTemplate()->SetAccessorProperty( env->verify_error_string(), - DiffieHellman::VerifyErrorGetter, - nullptr, - env->as_external(), - DEFAULT, - attributes, - AccessorSignature::New(env->isolate(), t2)); + verify_error_getter_templ2, + Local(), + attributes); target->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "DiffieHellmanGroup"), t2->GetFunction()); @@ -5130,8 +5136,7 @@ void DiffieHellman::SetPrivateKey(const FunctionCallbackInfo& args) { } -void DiffieHellman::VerifyErrorGetter(Local property, - const PropertyCallbackInfo& args) { +void DiffieHellman::VerifyErrorGetter(const FunctionCallbackInfo& args) { HandleScope scope(args.GetIsolate()); DiffieHellman* diffieHellman; diff --git a/src/node_crypto.h b/src/node_crypto.h index efc99c7c142ad0..0adf0dc894bd18 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -693,9 +693,7 @@ class DiffieHellman : public BaseObject { static void ComputeSecret(const v8::FunctionCallbackInfo& args); static void SetPublicKey(const v8::FunctionCallbackInfo& args); static void SetPrivateKey(const v8::FunctionCallbackInfo& args); - static void VerifyErrorGetter( - v8::Local property, - const v8::PropertyCallbackInfo& args); + static void VerifyErrorGetter(const v8::FunctionCallbackInfo& args); DiffieHellman(Environment* env, v8::Local wrap) : BaseObject(env, wrap), From 9292fa03fb9b3edf14142bbc2d8e8af2b74b665d Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 01:44:29 +0000 Subject: [PATCH 08/16] Change InstanceTemplate SetAccessor to SetAccessorProperty --- src/node_perf.cc | 63 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index 94c3a0f8e0c047..b30db0c52a7baa 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -20,6 +20,7 @@ using v8::Name; using v8::Object; using v8::ObjectTemplate; using v8::PropertyCallbackInfo; +using v8::Signature; using v8::String; using v8::Value; @@ -120,8 +121,7 @@ void Measure(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(obj); } -void GetPerformanceEntryName(const Local prop, - const PropertyCallbackInfo& info) { +void GetPerformanceEntryName(const FunctionCallbackInfo& info) { Isolate* isolate = info.GetIsolate(); PerformanceEntry* entry; ASSIGN_OR_RETURN_UNWRAP(&entry, info.Holder()); @@ -129,8 +129,7 @@ void GetPerformanceEntryName(const Local prop, String::NewFromUtf8(isolate, entry->name().c_str(), String::kNormalString)); } -void GetPerformanceEntryType(const Local prop, - const PropertyCallbackInfo& info) { +void GetPerformanceEntryType(const FunctionCallbackInfo& info) { Isolate* isolate = info.GetIsolate(); PerformanceEntry* entry; ASSIGN_OR_RETURN_UNWRAP(&entry, info.Holder()); @@ -138,15 +137,13 @@ void GetPerformanceEntryType(const Local prop, String::NewFromUtf8(isolate, entry->type().c_str(), String::kNormalString)); } -void GetPerformanceEntryStartTime(const Local prop, - const PropertyCallbackInfo& info) { +void GetPerformanceEntryStartTime(const FunctionCallbackInfo& info) { PerformanceEntry* entry; ASSIGN_OR_RETURN_UNWRAP(&entry, info.Holder()); info.GetReturnValue().Set(entry->startTime()); } -void GetPerformanceEntryDuration(const Local prop, - const PropertyCallbackInfo& info) { +void GetPerformanceEntryDuration(const FunctionCallbackInfo& info) { PerformanceEntry* entry; ASSIGN_OR_RETURN_UNWRAP(&entry, info.Holder()); info.GetReturnValue().Set(entry->duration()); @@ -336,14 +333,50 @@ void Init(Local target, Local pe = env->NewFunctionTemplate(PerformanceEntry::New); pe->InstanceTemplate()->SetInternalFieldCount(1); pe->SetClassName(performanceEntryString); + + Local signature = Signature::New(env->isolate(), pe); + + Local get_performance_entry_name_templ = + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryName, + Local(), + signature); + + Local get_performance_entry_type_templ = + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryType, + Local(), + signature); + + Local get_performance_entry_start_time_templ = + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryStartTime, + Local(), + signature); + + Local get_performance_entry_duration_templ = + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryDuration, + Local(), + signature); + Local ot = pe->InstanceTemplate(); - ot->SetAccessor(env->name_string(), GetPerformanceEntryName); - ot->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "entryType"), - GetPerformanceEntryType); - ot->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "startTime"), - GetPerformanceEntryStartTime); - ot->SetAccessor(FIXED_ONE_BYTE_STRING(isolate, "duration"), - GetPerformanceEntryDuration); + ot->SetAccessorProperty(env->name_string(), + get_performance_entry_name_templ, + Local()); + + ot->SetAccessorProperty(FIXED_ONE_BYTE_STRING(isolate, "entryType"), + get_performance_entry_type_templ, + Local()); + + ot->SetAccessorProperty(FIXED_ONE_BYTE_STRING(isolate, "startTime"), + get_performance_entry_start_time_templ, + Local()); + + ot->SetAccessorProperty(FIXED_ONE_BYTE_STRING(isolate, "duration"), + get_performance_entry_duration_templ, + Local()); + Local fn = pe->GetFunction(); target->Set(performanceEntryString, fn); env->set_performance_entry_template(fn); From 354008a82e2bd3a1c6735d9805be853cf7402eb6 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 01:56:57 +0000 Subject: [PATCH 09/16] Fix indentation and linting errors in node_crypto --- src/node_crypto.cc | 34 ++++++++++++++++------------------ src/node_crypto.h | 3 ++- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index dc5833d4f60c98..a550e64036ba7e 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -80,7 +80,6 @@ static const int X509_NAME_FLAGS = ASN1_STRFLGS_ESC_CTRL namespace node { namespace crypto { -using v8::AccessorSignature; using v8::Array; using v8::Boolean; using v8::Context; @@ -103,7 +102,6 @@ using v8::Object; using v8::ObjectTemplate; using v8::Persistent; using v8::PropertyAttribute; -using v8::PropertyCallbackInfo; using v8::ReadOnly; using v8::Signature; using v8::String; @@ -546,10 +544,10 @@ void SecureContext::Initialize(Environment* env, Local target) { Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex)); Local ctx_getter_templ = - FunctionTemplate::New(env->isolate(), - CtxGetter, - Local(), - Signature::New(env->isolate(), t)); + FunctionTemplate::New(env->isolate(), + CtxGetter, + env->as_external(), + Signature::New(env->isolate(), t)); t->PrototypeTemplate()->SetAccessorProperty( @@ -1641,10 +1639,10 @@ void SSLWrap::AddMethods(Environment* env, Local t) { env->SetProtoMethod(t, "setALPNProtocols", SetALPNProtocols); Local ssl_getter_templ = - FunctionTemplate::New(env->isolate(), - SSLGetter, - Local(), - Signature::New(env->isolate(), t)); + FunctionTemplate::New(env->isolate(), + SSLGetter, + env->as_external(), + Signature::New(env->isolate(), t)); t->PrototypeTemplate()->SetAccessorProperty( FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), @@ -4786,10 +4784,10 @@ void DiffieHellman::Initialize(Environment* env, Local target) { env->SetProtoMethod(t, "setPrivateKey", SetPrivateKey); Local verify_error_getter_templ = - FunctionTemplate::New(env->isolate(), - DiffieHellman::VerifyErrorGetter, - Local(), - Signature::New(env->isolate(), t)); + FunctionTemplate::New(env->isolate(), + DiffieHellman::VerifyErrorGetter, + Local(), + Signature::New(env->isolate(), t)); t->InstanceTemplate()->SetAccessorProperty( env->verify_error_string(), @@ -4811,10 +4809,10 @@ void DiffieHellman::Initialize(Environment* env, Local target) { env->SetProtoMethod(t2, "getPrivateKey", GetPrivateKey); Local verify_error_getter_templ2 = - FunctionTemplate::New(env->isolate(), - DiffieHellman::VerifyErrorGetter, - Local(), - Signature::New(env->isolate(), t2)); + FunctionTemplate::New(env->isolate(), + DiffieHellman::VerifyErrorGetter, + Local(), + Signature::New(env->isolate(), t2)); t2->InstanceTemplate()->SetAccessorProperty( env->verify_error_string(), diff --git a/src/node_crypto.h b/src/node_crypto.h index 0adf0dc894bd18..6567896590810c 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -693,7 +693,8 @@ class DiffieHellman : public BaseObject { static void ComputeSecret(const v8::FunctionCallbackInfo& args); static void SetPublicKey(const v8::FunctionCallbackInfo& args); static void SetPrivateKey(const v8::FunctionCallbackInfo& args); - static void VerifyErrorGetter(const v8::FunctionCallbackInfo& args); + static void VerifyErrorGetter( + const v8::FunctionCallbackInfo& args); DiffieHellman(Environment* env, v8::Local wrap) : BaseObject(env, wrap), From a199e07e68620a17100f23434da3bf96ca2cde6c Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 02:08:10 +0000 Subject: [PATCH 10/16] Fixes for indentation --- src/node_perf.cc | 32 ++++++++++++++++---------------- src/stream_base-inl.h | 24 ++++++++++++------------ src/udp_wrap.cc | 8 ++++---- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index b30db0c52a7baa..7b41278fec8d85 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -337,28 +337,28 @@ void Init(Local target, Local signature = Signature::New(env->isolate(), pe); Local get_performance_entry_name_templ = - FunctionTemplate::New(env->isolate(), - GetPerformanceEntryName, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryName, + Local(), + signature); Local get_performance_entry_type_templ = - FunctionTemplate::New(env->isolate(), - GetPerformanceEntryType, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryType, + Local(), + signature); Local get_performance_entry_start_time_templ = - FunctionTemplate::New(env->isolate(), - GetPerformanceEntryStartTime, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryStartTime, + Local(), + signature); Local get_performance_entry_duration_templ = - FunctionTemplate::New(env->isolate(), - GetPerformanceEntryDuration, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetPerformanceEntryDuration, + Local(), + signature); Local ot = pe->InstanceTemplate(); ot->SetAccessorProperty(env->name_string(), diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index 3a5c85be116a61..bbe03f73ecdfe1 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -38,22 +38,22 @@ void StreamBase::AddMethods(Environment* env, Local signature = Signature::New(env->isolate(), t); Local get_fd_templ = - FunctionTemplate::New(env->isolate(), - GetFD, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetFD, + Local(), + signature); Local get_external_templ = - FunctionTemplate::New(env->isolate(), - GetExternal, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetExternal, + Local(), + signature); Local get_bytes_read_templ = - FunctionTemplate::New(env->isolate(), - GetBytesRead, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + GetBytesRead, + Local(), + signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), get_fd_templ, diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index 09f54823b9aeb4..b4562ad25bdead 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -115,10 +115,10 @@ void UDPWrap::Initialize(Local target, Local signature = Signature::New(env->isolate(), t); Local get_fd_templ = - FunctionTemplate::New(env->isolate(), - UDPWrap::GetFD, - Local(), - signature); + FunctionTemplate::New(env->isolate(), + UDPWrap::GetFD, + Local(), + signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), get_fd_templ, From 5eb65070ca889d0e14d8d6aeced704745d14aa82 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 02:19:12 +0000 Subject: [PATCH 11/16] Add more tests for UDP and crypto --- test/parallel/test-accessor-properties.js | 77 +++++++++++++++++++ .../test-stream-base-prototype-accessors.js | 42 ---------- 2 files changed, 77 insertions(+), 42 deletions(-) create mode 100644 test/parallel/test-accessor-properties.js delete mode 100644 test/parallel/test-stream-base-prototype-accessors.js diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js new file mode 100644 index 00000000000000..3db2703f9aaffd --- /dev/null +++ b/test/parallel/test-accessor-properties.js @@ -0,0 +1,77 @@ +'use strict'; + +require('../common'); + +// This tests that the accessor properties do not raise assersions +// when called with incompatible receivers. + +const assert = require('assert'); + +// Examples are things that calls StreamBase::AddMethods when setting up +// their prototype +const TTY = process.binding('tty_wrap').TTY; +const UDP = process.binding('udp_wrap').UDP; + +// Or the accessor properties in crypto +const crypto = process.binding('crypto'); + +{ + // Should throw instead of raise assertions + assert.throws(() => { + TTY.prototype.bytesRead; + }, TypeError); + + assert.throws(() => { + TTY.prototype.fd; + }, TypeError); + + assert.throws(() => { + TTY.prototype._externalStream; + }, TypeError); + + assert.throws(() => { + UDP.prototype.fd; + }, TypeError); + + assert.throws(() => { + crypto.SecureContext.prototype._external; + }, TypeError); + + assert.throws(() => { + crypto.Connection.prototype._external; + }, TypeError); + + + // Should not throw for Object.getOwnPropertyDescriptor + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead'), + 'object' + ); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'fd'), + 'object' + ); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream'), + 'object' + ); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor(UDP.prototype, 'fd'), + 'object' + ); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor( + crypto.SecureContext.prototype, '_external'), + 'object' + ); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor( + crypto.Connection.prototype, '_external'), + 'object' + ); +} diff --git a/test/parallel/test-stream-base-prototype-accessors.js b/test/parallel/test-stream-base-prototype-accessors.js deleted file mode 100644 index c579c55bd1752c..00000000000000 --- a/test/parallel/test-stream-base-prototype-accessors.js +++ /dev/null @@ -1,42 +0,0 @@ -'use strict'; - -require('../common'); - -// This tests that the prototype accessors added by StreamBase::AddMethods -// do not raise assersions when called with incompatible receivers. - -const assert = require('assert'); - -// Or anything that calls StreamBase::AddMethods when setting up its prototype -const TTY = process.binding('tty_wrap').TTY; - -{ - // Should throw instead of raise assertions - assert.throws(() => { - TTY.prototype.bytesRead; - }, TypeError); - - assert.throws(() => { - TTY.prototype.fd; - }, TypeError); - - assert.throws(() => { - TTY.prototype._externalStream; - }, TypeError); - - // Should not throw for Object.getOwnPropertyDescriptor - assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'bytesRead'), - 'object' - ); - - assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, 'fd'), - 'object' - ); - - assert.strictEqual( - typeof Object.getOwnPropertyDescriptor(TTY.prototype, '_externalStream'), - 'object' - ); -} From 232d2be7ac836ead24c0eacc43abc91933e4407d Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 02:19:35 +0000 Subject: [PATCH 12/16] Fixes for indentation --- src/node_perf.cc | 1 - src/udp_wrap.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index 7b41278fec8d85..c8043f0eb89963 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -19,7 +19,6 @@ using v8::Local; using v8::Name; using v8::Object; using v8::ObjectTemplate; -using v8::PropertyCallbackInfo; using v8::Signature; using v8::String; using v8::Value; diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index b4562ad25bdead..a289abd69ac316 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -41,7 +41,6 @@ using v8::Integer; using v8::Local; using v8::Object; using v8::PropertyAttribute; -using v8::PropertyCallbackInfo; using v8::Signature; using v8::String; using v8::Uint32; From d89578adc26e4570ca2f499a467d06193ac382b9 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 02:40:18 +0000 Subject: [PATCH 13/16] Use env->as_external to get access to Environment in callback --- src/stream_base-inl.h | 6 +++--- src/udp_wrap.cc | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/stream_base-inl.h b/src/stream_base-inl.h index bbe03f73ecdfe1..cc89a11bac249c 100644 --- a/src/stream_base-inl.h +++ b/src/stream_base-inl.h @@ -40,19 +40,19 @@ void StreamBase::AddMethods(Environment* env, Local get_fd_templ = FunctionTemplate::New(env->isolate(), GetFD, - Local(), + env->as_external(), signature); Local get_external_templ = FunctionTemplate::New(env->isolate(), GetExternal, - Local(), + env->as_external(), signature); Local get_bytes_read_templ = FunctionTemplate::New(env->isolate(), GetBytesRead, - Local(), + env->as_external(), signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), diff --git a/src/udp_wrap.cc b/src/udp_wrap.cc index a289abd69ac316..9d4e7f7ab41678 100644 --- a/src/udp_wrap.cc +++ b/src/udp_wrap.cc @@ -116,7 +116,7 @@ void UDPWrap::Initialize(Local target, Local get_fd_templ = FunctionTemplate::New(env->isolate(), UDPWrap::GetFD, - Local(), + env->as_external(), signature); t->PrototypeTemplate()->SetAccessorProperty(env->fd_string(), From 5a60daaa945ee901f7d2f6a8bcfdc25a3cd1b80b Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 02:41:16 +0000 Subject: [PATCH 14/16] More env->as_external to get access to Environment in callback --- src/node_crypto.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index a550e64036ba7e..6ba3a760953b14 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4786,7 +4786,7 @@ void DiffieHellman::Initialize(Environment* env, Local target) { Local verify_error_getter_templ = FunctionTemplate::New(env->isolate(), DiffieHellman::VerifyErrorGetter, - Local(), + env->as_external(), Signature::New(env->isolate(), t)); t->InstanceTemplate()->SetAccessorProperty( @@ -4811,7 +4811,7 @@ void DiffieHellman::Initialize(Environment* env, Local target) { Local verify_error_getter_templ2 = FunctionTemplate::New(env->isolate(), DiffieHellman::VerifyErrorGetter, - Local(), + env->as_external(), Signature::New(env->isolate(), t2)); t2->InstanceTemplate()->SetAccessorProperty( From 84905d63630f09c8f4ae0e6af11097a3a4ef9b44 Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 03:16:19 +0000 Subject: [PATCH 15/16] Use env->as_external in node_perf too --- src/node_perf.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/node_perf.cc b/src/node_perf.cc index c8043f0eb89963..37256a943cb014 100644 --- a/src/node_perf.cc +++ b/src/node_perf.cc @@ -338,25 +338,25 @@ void Init(Local target, Local get_performance_entry_name_templ = FunctionTemplate::New(env->isolate(), GetPerformanceEntryName, - Local(), + env->as_external(), signature); Local get_performance_entry_type_templ = FunctionTemplate::New(env->isolate(), GetPerformanceEntryType, - Local(), + env->as_external(), signature); Local get_performance_entry_start_time_templ = FunctionTemplate::New(env->isolate(), GetPerformanceEntryStartTime, - Local(), + env->as_external(), signature); Local get_performance_entry_duration_templ = FunctionTemplate::New(env->isolate(), GetPerformanceEntryDuration, - Local(), + env->as_external(), signature); Local ot = pe->InstanceTemplate(); From 73aaf0ef5574774c858ff1e7ab11becd37a6b1aa Mon Sep 17 00:00:00 2001 From: Jure Triglav Date: Thu, 14 Dec 2017 09:54:30 +0000 Subject: [PATCH 16/16] Update comments --- test/parallel/test-accessor-properties.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js index 3db2703f9aaffd..478b1c55e93fdb 100644 --- a/test/parallel/test-accessor-properties.js +++ b/test/parallel/test-accessor-properties.js @@ -2,17 +2,17 @@ require('../common'); -// This tests that the accessor properties do not raise assersions +// This tests that the accessor properties do not raise assertions // when called with incompatible receivers. const assert = require('assert'); -// Examples are things that calls StreamBase::AddMethods when setting up +// Objects that call StreamBase::AddMethods, when setting up // their prototype const TTY = process.binding('tty_wrap').TTY; const UDP = process.binding('udp_wrap').UDP; -// Or the accessor properties in crypto +// There are accessor properties in crypto too const crypto = process.binding('crypto'); {