From 3f08c004c5c95abc8a2faf7e2ea156c72806b54b Mon Sep 17 00:00:00 2001 From: Vitaly Dyatlov Date: Sun, 8 Jul 2018 20:12:58 +0000 Subject: [PATCH] src: revert removal of SecureContext `_external` getter This `_external` getter is essential for some libs to work: uWebSockets as an example. PR-URL: https://github.com/nodejs/node/pull/21711 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- src/node_crypto.cc | 21 ++++++++++++++++++++ src/node_crypto.h | 1 + test/parallel/test-accessor-properties.js | 17 +++++++++++++++- test/parallel/test-tls-external-accessor.js | 22 +++++++++++++++++++++ 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-external-accessor.js diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 9b22edfb9605e6..dd7e0c84681438 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -359,6 +359,19 @@ void SecureContext::Initialize(Environment* env, Local target) { t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kTicketKeyIVIndex"), Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex)); + Local ctx_getter_templ = + FunctionTemplate::New(env->isolate(), + CtxGetter, + env->as_external(), + Signature::New(env->isolate(), t)); + + + t->PrototypeTemplate()->SetAccessorProperty( + FIXED_ONE_BYTE_STRING(env->isolate(), "_external"), + ctx_getter_templ, + Local(), + static_cast(ReadOnly | DontDelete)); + target->Set(secureContextString, t->GetFunction(env->context()).ToLocalChecked()); env->set_secure_context_constructor_template(t); @@ -1331,6 +1344,14 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl, } +void SecureContext::CtxGetter(const FunctionCallbackInfo& info) { + SecureContext* sc; + ASSIGN_OR_RETURN_UNWRAP(&sc, info.This()); + Local ext = External::New(info.GetIsolate(), sc->ctx_.get()); + info.GetReturnValue().Set(ext); +} + + template void SecureContext::GetCertificate(const FunctionCallbackInfo& args) { SecureContext* wrap; diff --git a/src/node_crypto.h b/src/node_crypto.h index 63d85669fa6a92..2ca333f3c22d12 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -165,6 +165,7 @@ class SecureContext : public BaseObject { const v8::FunctionCallbackInfo& args); static void EnableTicketKeyCallback( const v8::FunctionCallbackInfo& args); + static void CtxGetter(const v8::FunctionCallbackInfo& info); template static void GetCertificate(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-accessor-properties.js b/test/parallel/test-accessor-properties.js index 453100d10886ff..95b960b202cd53 100644 --- a/test/parallel/test-accessor-properties.js +++ b/test/parallel/test-accessor-properties.js @@ -1,7 +1,7 @@ // Flags: --expose-internals 'use strict'; -require('../common'); +const common = require('../common'); // This tests that the accessor properties do not raise assertions // when called with incompatible receivers. @@ -54,4 +54,19 @@ const UDP = internalBinding('udp_wrap').UDP; typeof Object.getOwnPropertyDescriptor(StreamWrapProto, 'fd'), 'object' ); + + if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check + // There are accessor properties in crypto too + const crypto = process.binding('crypto'); + + assert.throws(() => { + crypto.SecureContext.prototype._external; + }, TypeError); + + assert.strictEqual( + typeof Object.getOwnPropertyDescriptor( + crypto.SecureContext.prototype, '_external'), + 'object' + ); + } } diff --git a/test/parallel/test-tls-external-accessor.js b/test/parallel/test-tls-external-accessor.js new file mode 100644 index 00000000000000..33d371923a600c --- /dev/null +++ b/test/parallel/test-tls-external-accessor.js @@ -0,0 +1,22 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); + +// Ensure accessing ._external doesn't hit an assert in the accessor method. +{ + const pctx = tls.createSecureContext().context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +} +{ + const pctx = tls.createSecurePair().credentials.context; + const cctx = Object.create(pctx); + assert.throws(() => cctx._external, TypeError); + pctx._external; +}