Skip to content

Commit

Permalink
src: revert removal of SecureContext _external getter
Browse files Browse the repository at this point in the history
This `_external` getter is essential for some libs to work:
uWebSockets as an example.

PR-URL: #21711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
dyatlov authored and addaleax committed Oct 12, 2018
1 parent e4dea40 commit 3f08c00
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 1 deletion.
21 changes: 21 additions & 0 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,19 @@ void SecureContext::Initialize(Environment* env, Local<Object> target) {
t->Set(FIXED_ONE_BYTE_STRING(env->isolate(), "kTicketKeyIVIndex"),
Integer::NewFromUnsigned(env->isolate(), kTicketKeyIVIndex));

Local<FunctionTemplate> 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<FunctionTemplate>(),
static_cast<PropertyAttribute>(ReadOnly | DontDelete));

target->Set(secureContextString,
t->GetFunction(env->context()).ToLocalChecked());
env->set_secure_context_constructor_template(t);
Expand Down Expand Up @@ -1331,6 +1344,14 @@ int SecureContext::TicketCompatibilityCallback(SSL* ssl,
}


void SecureContext::CtxGetter(const FunctionCallbackInfo<Value>& info) {
SecureContext* sc;
ASSIGN_OR_RETURN_UNWRAP(&sc, info.This());
Local<External> ext = External::New(info.GetIsolate(), sc->ctx_.get());
info.GetReturnValue().Set(ext);
}


template <bool primary>
void SecureContext::GetCertificate(const FunctionCallbackInfo<Value>& args) {
SecureContext* wrap;
Expand Down
1 change: 1 addition & 0 deletions src/node_crypto.h
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ class SecureContext : public BaseObject {
const v8::FunctionCallbackInfo<v8::Value>& args);
static void EnableTicketKeyCallback(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void CtxGetter(const v8::FunctionCallbackInfo<v8::Value>& info);

template <bool primary>
static void GetCertificate(const v8::FunctionCallbackInfo<v8::Value>& args);
Expand Down
17 changes: 16 additions & 1 deletion test/parallel/test-accessor-properties.js
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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'
);
}
}
22 changes: 22 additions & 0 deletions test/parallel/test-tls-external-accessor.js
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 3f08c00

Please sign in to comment.