Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

src: check IsConstructCall in TLSWrap constructor #13097

Closed
wants to merge 1 commit into from
Closed

src: check IsConstructCall in TLSWrap constructor #13097

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 18, 2017

Currently it is possible to call TLSWrap() without using new and the
following error message:

FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:

/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src, crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. tls Issues and PRs related to the tls subsystem. labels May 18, 2017
@danbev
Copy link
Contributor Author

danbev commented May 18, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/8146/

@bnoordhuis
Copy link
Member

I'd maybe reword the commit log as "src: check IsConstructCall in TLSWrap constructor" - that still fits in 50 columns and is a bit more descriptive, IMO.

@danbev
src: check IsConstructCall in TLSWrap constructor
1ff414e 
Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]
@danbev danbev force-pushed the tls_wrap-add-is-construct-call branch from e333f29 to 1ff414e Compare May 18, 2017 10:25
@danbev danbev changed the title src: add IsConstructCall TLSWrap constructor src: check IsConstructCall in TLSWrap constructor May 18, 2017
@danbev
Copy link
Contributor Author

danbev commented May 18, 2017 •
edited
Loading

I'd maybe reword the commit log as "src: check IsConstructCall in TLSWrap constructor" - that still fits in 50 columns and is a bit more descriptive, IMO.

I agree, I've updated this now.

danbev added a commit to danbev/node that referenced this pull request May 22, 2017
@danbev
src: check IsConstructCall in TLSWrap constructor
57f38c6 
Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

PR-URL: nodejs#13097
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@danbev
Copy link
Contributor Author

danbev commented May 22, 2017

Landed in 57f38c6

@danbev danbev closed this May 22, 2017
@danbev danbev deleted the tls_wrap-add-is-construct-call branch May 22, 2017 05:37
jasnell pushed a commit that referenced this pull request May 23, 2017
@danbev @jasnell
src: check IsConstructCall in TLSWrap constructor
b521c7d 
Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

PR-URL: #13097
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
jasnell pushed a commit that referenced this pull request May 23, 2017
@danbev @jasnell
src: check IsConstructCall in TLSWrap constructor
60132e8 
Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

PR-URL: #13097
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@jasnell jasnell mentioned this pull request May 28, 2017
Closed
@gibfahn gibfahn mentioned this pull request Jun 15, 2017
Closed
3 tasks
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
@danbev @MylesBorins
src: check IsConstructCall in TLSWrap constructor
37e1929 
Currently it is possible to call TLSWrap() without using new and the
following error message:
FATAL ERROR: v8::Object::SetAlignedPointerInInternalField() Internal
field out of bounds
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::OnFatalError(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: v8::Utils::ReportApiFailure(char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 4: v8::Utils::ApiCheck(bool, char const*, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 5: v8::InternalFieldOK(v8::internal::Handle<v8::internal::JSReceiver>,
int, char const*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 6: v8::Object::SetAlignedPointerInInternalField(int, void*)
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 7: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

This commit adds a IsConstructCall check which will produce the
following error message:
/Users/danielbevenius/work/nodejs/node/out/Debug/node[2212]:
../src/tls_wrap.cc:936:auto node::TLSWrap::Initialize(Local<v8::Object>,
Local<v8::Value>, Local<v8::Context>)::(anonymous
class)::operator()(const FunctionCallbackInfo<v8::Value> &) const:
Assertion `args.IsConstructCall()' failed.
 1: node::Abort()
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 2: node::Assert(char const* const (*) [4])
[/Users/danielbevenius/work/nodejs/node/out/Debug/node]
 3: node::TLSWrap::Initialize(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)::$_0::operator()(v8::FunctionCallbackInfo<v8::Value>
const&) const [/Users/danielbevenius/work/nodejs/node/out/Debug/node]

PR-URL: #13097
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@danbev @bnoordhuis @addaleax @cjihrig @MylesBorins @nodejs-github-bot