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.

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

JSError: never fail with invalid getter target" #1621

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions include/hermes/VM/JSError.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,6 @@ class JSError final : public JSObject {
Handle<JSError> selfHandle,
size_t index,
llvh::SmallVectorImpl<char16_t> &str);

/// Given an object \p targetHandle:
/// 1. If its [[CapturedError]] slot has a non-null handle, return it as a
/// JSError.
/// 2. Otherwise, return \t targetHandle cast to JSError.
/// Throws if any cast or property access fails.
static CallResult<Handle<JSError>> getErrorFromStackTarget(
Runtime &runtime,
Handle<JSObject> targetHandle);
};

} // namespace vm
Expand Down
17 changes: 10 additions & 7 deletions lib/VM/JSError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ void JSErrorBuildMeta(const GCCell *cell, Metadata::Builder &mb) {
mb.addField("domains", &self->domains_);
}

CallResult<Handle<JSError>> JSError::getErrorFromStackTarget(
/// Given an object \p targetHandle which may be nullptr:
/// 1. Look for [[CapturedError]] in the object or its prototype chain and
/// return it a JSError.
/// 2. Otherwise, return llvh::None.
static llvh::Optional<Handle<JSError>> getErrorFromStackTarget(
Runtime &runtime,
Handle<JSObject> targetHandle) {
MutableHandle<JSObject> mutHnd =
Expand All @@ -78,20 +82,19 @@ CallResult<Handle<JSError>> JSError::getErrorFromStackTarget(

mutHnd.set(targetHandle->getParent(runtime));
}
return runtime.raiseTypeError(
"Error.stack getter called with an invalid receiver");
return llvh::None;
}

CallResult<HermesValue>
errorStackGetter(void *, Runtime &runtime, NativeArgs args) {
GCScope gcScope(runtime);

auto targetHandle = args.dyncastThis<JSObject>();
auto errorHandleRes = JSError::getErrorFromStackTarget(runtime, targetHandle);
if (errorHandleRes == ExecutionStatus::EXCEPTION) {
return ExecutionStatus::EXCEPTION;
auto errorHandleOpt = getErrorFromStackTarget(runtime, targetHandle);
if (!errorHandleOpt.hasValue()) {
return HermesValue::encodeUndefinedValue();
}
auto errorHandle = *errorHandleRes;
auto errorHandle = *errorHandleOpt;
if (!errorHandle->stacktrace_) {
// Stacktrace has not been set, we simply return empty string.
// This is different from other VMs where stacktrace is created when
Expand Down
8 changes: 4 additions & 4 deletions test/hermes/error-capture-stack-trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ print(target.stack);
var dummy = {};
Error.captureStackTrace(dummy);
const stackGetter = Object.getOwnPropertyDescriptor(dummy, 'stack').get;
try { stackGetter.apply({}); } catch (e) { print(e); }
//CHECK: TypeError: Error.stack getter called with an invalid receiver
try { stackGetter.apply(null); } catch (e) { print(e); }
//CHECK: TypeError: Error.stack getter called with an invalid receiver
print(stackGetter.apply({}));
//CHECK: undefined
print(stackGetter.apply(null));
//CHECK: undefined
13 changes: 13 additions & 0 deletions test/hermes/error-in-proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,16 @@ try {
}
// CHECK: Caught Error MyError: 1234
// CHECK-NEXT: at global{{.*}}


function MyError2() {
this.message = "MyError2";
}
MyError2.prototype = Object.create(Error.prototype);
try {
throw new MyError2();
} catch (e) {
print("Caught", e, e.stack);
}

// CHECK-NEXT: Caught Error: MyError2 undefined
8 changes: 2 additions & 6 deletions test/hermes/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,8 @@ print(e);
// CHECK-NEXT: 12345: 67890

// Check exception case of accessing Error.stack from a different 'this'
try {
new Error().__lookupGetter__("stack").call({});
} catch (e) {
print(e.name);
}
//CHECK-NEXT: TypeError
print(new Error().__lookupGetter__("stack").call({}));
// CHECK-NEXT: undefined


// Regression test: setting the stack while getting the message causes a null dereference.
Expand Down
26 changes: 9 additions & 17 deletions test/hermes/regress-proxy-parent.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,14 @@
// Install stack trace information on Object.prototype.
Error.captureStackTrace(Object.prototype);

// Check that lookup on a regular proxy fails.
try{
var p = new Proxy(new Error(), {});
p.stack;
} catch (e) {
print(e);
}
// CHECK: TypeError: Error.stack getter called with an invalid receiver
// Check that lookup on a regular proxy returns undefined.
var p = new Proxy(new Error(), {});
print(p.stack);
// CHECK: undefined

// Check that lookup on a callable proxy fails.
try {
function foo(){}
foo.__proto__ = new Error();
var cp = new Proxy(foo, {});
cp.stack;
} catch (e) {
print(e);
}
// CHECK-NEXT: TypeError: Error.stack getter called with an invalid receiver
function foo(){}
foo.__proto__ = new Error();
var cp = new Proxy(foo, {});
print(cp.stack);
// CHECK-NEXT: undefined
Loading