Skip to content

Commit

Permalink
report: get stack trace with cross origin contexts
Browse files Browse the repository at this point in the history
When a new context with a different security token is entered, or
when no context is entered, `StackTrace::CurrentStackTrace` need to
be explicitly set with flag `kExposeFramesAcrossSecurityOrigins` to
avoid crashing.

PR-URL: #44398
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
  • Loading branch information
legendecas authored and RafaelGSS committed Sep 7, 2022
1 parent b17cc87 commit 807b1e5
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 12 deletions.
5 changes: 5 additions & 0 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,11 @@ void OOMErrorHandler(const char* location, bool is_heap_oom) {
}

if (report_on_fatalerror) {
// Trigger report with the isolate. Environment::GetCurrent may return
// nullptr here:
// - If the OOM is reported by a young generation space allocation,
// Isolate::GetCurrentContext returns an empty handle.
// - Otherwise, Isolate::GetCurrentContext returns a non-empty handle.
TriggerNodeReport(isolate, message, "OOMError", "", Local<Object>());
}

Expand Down
6 changes: 5 additions & 1 deletion src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,12 @@ static void PrintJavaScriptStack(JSONWriter* writer,
void* samples[MAX_FRAME_COUNT];
isolate->GetStackSample(state, samples, MAX_FRAME_COUNT, &info);

constexpr StackTrace::StackTraceOptions stack_trace_options =
static_cast<StackTrace::StackTraceOptions>(
StackTrace::kDetailed |
StackTrace::kExposeFramesAcrossSecurityOrigins);
Local<StackTrace> stack = StackTrace::CurrentStackTrace(
isolate, MAX_FRAME_COUNT, StackTrace::kDetailed);
isolate, MAX_FRAME_COUNT, stack_trace_options);

if (stack->GetFrameCount() == 0) {
PrintEmptyJavaScriptStack(writer);
Expand Down
27 changes: 27 additions & 0 deletions test/addons/report-api/binding.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <node.h>
#include <v8.h>

using v8::Context;
using v8::FunctionCallbackInfo;
using v8::Isolate;
using v8::Local;
Expand Down Expand Up @@ -43,11 +44,37 @@ void TriggerReportNoEnv(const FunctionCallbackInfo<Value>& args) {
Local<Value>());
}

void TriggerReportNoContext(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Local<Context> context = isolate->GetCurrentContext();
context->Exit();

if (isolate->GetCurrentContext().IsEmpty()) {
node::TriggerNodeReport(
isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
}

// Restore current context to avoid crashing in Context::Scope in
// SpinEventLoop.
context->Enter();
}

void TriggerReportNewContext(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Local<Context> context = Context::New(isolate);
Context::Scope context_scope(context);

node::TriggerNodeReport(
isolate, "FooMessage", "BarTrigger", std::string(), Local<Value>());
}

void init(Local<Object> exports) {
NODE_SET_METHOD(exports, "triggerReport", TriggerReport);
NODE_SET_METHOD(exports, "triggerReportNoIsolate", TriggerReportNoIsolate);
NODE_SET_METHOD(exports, "triggerReportEnv", TriggerReportEnv);
NODE_SET_METHOD(exports, "triggerReportNoEnv", TriggerReportNoEnv);
NODE_SET_METHOD(exports, "triggerReportNoContext", TriggerReportNoContext);
NODE_SET_METHOD(exports, "triggerReportNewContext", TriggerReportNewContext);
}

NODE_MODULE(NODE_GYP_MODULE_NAME, init)
31 changes: 20 additions & 11 deletions test/addons/report-api/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const tmpdir = require('../../common/tmpdir');
const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
const addon = require(binding);

function myAddonMain(method, hasJavaScriptFrames) {
function myAddonMain(method, { hasIsolate, hasEnv }) {
tmpdir.refresh();
process.report.directory = tmpdir.path;

Expand All @@ -19,26 +19,35 @@ function myAddonMain(method, hasJavaScriptFrames) {
assert.strictEqual(reports.length, 1);

const report = reports[0];
helper.validate(report);
helper.validate(report, [
['header.event', 'FooMessage'],
['header.trigger', 'BarTrigger'],
]);

const content = require(report);
assert.strictEqual(content.header.event, 'FooMessage');
assert.strictEqual(content.header.trigger, 'BarTrigger');

// Check that the javascript stack is present.
if (hasJavaScriptFrames) {
if (hasIsolate) {
assert.strictEqual(content.javascriptStack.stack.findIndex((frame) => frame.match('myAddonMain')), 0);
} else {
assert.strictEqual(content.javascriptStack, undefined);
}

if (hasEnv) {
assert.strictEqual(content.header.threadId, 0);
} else {
assert.strictEqual(content.header.threadId, null);
}
}

const methods = [
['triggerReport', true],
['triggerReportNoIsolate', false],
['triggerReportEnv', true],
['triggerReportNoEnv', false],
['triggerReport', true, true],
['triggerReportNoIsolate', false, false],
['triggerReportEnv', true, true],
['triggerReportNoEnv', false, false],
['triggerReportNoContext', true, false],
['triggerReportNewContext', true, false],
];
for (const [method, hasJavaScriptFrames] of methods) {
myAddonMain(method, hasJavaScriptFrames);
for (const [method, hasIsolate, hasEnv] of methods) {
myAddonMain(method, { hasIsolate, hasEnv });
}

0 comments on commit 807b1e5

Please sign in to comment.