Bug 1107953 part 5. Add tests for promise rejections with content-side DOMExceptions, and fix the promise code so those tests actually pass. r=bholley

This commit is contained in:
Boris Zbarsky 2015-01-15 17:39:02 -05:00
parent ec4f0b62d0
commit 20da84c169
11 changed files with 178 additions and 25 deletions

View File

@ -722,5 +722,32 @@ DOMException::Create(nsresult aRv)
return inst.forget();
}
bool
DOMException::Sanitize(JSContext* aCx,
JS::MutableHandle<JS::Value> aSanitizedValue)
{
nsRefPtr<DOMException> retval = this;
if (mLocation && !mLocation->CallerSubsumes(aCx)) {
nsString message;
GetMessageMoz(message);
nsString name;
GetName(name);
retval = new dom::DOMException(nsresult(Result()),
NS_ConvertUTF16toUTF8(message),
NS_ConvertUTF16toUTF8(name),
Code());
// Now it's possible that the stack on retval still starts with
// stuff aCx is not supposed to touch; it depends on what's on the
// stack right this second. Walk past all of that.
while (retval->mLocation && !retval->mLocation->CallerSubsumes(aCx)) {
nsCOMPtr<nsIStackFrame> caller;
retval->mLocation->GetCaller(getter_AddRefs(caller));
retval->mLocation.swap(caller);
}
}
return ToJSValue(aCx, retval, aSanitizedValue);
}
} // namespace dom
} // namespace mozilla

View File

@ -156,6 +156,15 @@ public:
static already_AddRefed<DOMException>
Create(nsresult aRv);
// Sanitize() is a workaround for the fact that DOMExceptions can leak stack
// information for the first stackframe to callers that should not have access
// to it. To prevent this, we check whether aCx subsumes our first stackframe
// and if not hand out a JS::Value for a clone of ourselves. Otherwise we
// hand out a JS::Value for ourselves.
//
// If the return value is false, an exception was thrown on aCx.
bool Sanitize(JSContext* aCx, JS::MutableHandle<JS::Value> aSanitizedValue);
protected:
virtual ~DOMException() {}

View File

@ -234,20 +234,11 @@ ErrorResult::ReportJSExceptionFromJSImplementation(JSContext* aCx)
nsresult rv =
UNWRAP_OBJECT(DOMException, &mJSException.toObject(), domException);
if (NS_SUCCEEDED(rv)) {
// We actually have to create a new DOMException object, because the one we
// We may have to create a new DOMException object, because the one we
// have has a stack that includes the chrome code that threw it, and in
// particular has the wrong file/line/column information.
nsString message;
domException->GetMessageMoz(message);
nsString name;
domException->GetName(name);
nsRefPtr<dom::DOMException> newException =
new dom::DOMException(nsresult(domException->Result()),
NS_ConvertUTF16toUTF8(message),
NS_ConvertUTF16toUTF8(name),
domException->Code());
JS::Rooted<JS::Value> reflector(aCx);
if (!GetOrCreateDOMReflector(aCx, newException, &reflector)) {
if (!domException->Sanitize(aCx, &reflector)) {
// Well, that threw _an_ exception. Let's forget ours. We can just
// unroot and not change the value, since mJSException is completely
// ignored if mResult is not NS_ERROR_DOM_JS_EXCEPTION and we plan to
@ -301,6 +292,17 @@ ErrorResult::StealJSException(JSContext* cx,
value.set(mJSException);
js::RemoveRawValueRoot(cx, &mJSException);
mResult = NS_OK;
if (value.isObject()) {
// If it's a DOMException we may need to sanitize it.
dom::DOMException* domException;
nsresult rv =
UNWRAP_OBJECT(DOMException, &value.toObject(), domException);
if (NS_SUCCEEDED(rv) && !domException->Sanitize(cx, value)) {
JS_GetPendingException(cx, value);
JS_ClearPendingException(cx);
}
}
}
void

View File

@ -97,7 +97,8 @@ public:
// StealJSException steals the JS Exception from the object. This method must
// be called only if IsJSException() returns true. This method also resets the
// ErrorCode() to NS_OK.
// ErrorCode() to NS_OK. The value will be ensured to be sanitized wrt to the
// current compartment of cx if it happens to be a DOMException.
void StealJSException(JSContext* cx, JS::MutableHandle<JS::Value> value);
void MOZ_ALWAYS_INLINE MightThrowJSException()

View File

@ -286,6 +286,7 @@ public:
NS_IMETHOD GetName(nsAString& aFunction) MOZ_OVERRIDE;
NS_IMETHOD GetCaller(nsIStackFrame** aCaller) MOZ_OVERRIDE;
NS_IMETHOD GetFormattedStack(nsAString& aStack) MOZ_OVERRIDE;
virtual bool CallerSubsumes(JSContext* aCx) MOZ_OVERRIDE;
protected:
virtual bool IsJSFrame() const MOZ_OVERRIDE {
@ -613,6 +614,37 @@ NS_IMETHODIMP StackFrame::ToString(nsACString& _retval)
return NS_OK;
}
/* virtual */ bool
StackFrame::CallerSubsumes(JSContext* aCx)
{
return true;
}
/* virtual */ bool
JSStackFrame::CallerSubsumes(JSContext* aCx)
{
if (!NS_IsMainThread()) {
return true;
}
if (!mStack) {
// No problem here, there's no data to leak.
return true;
}
nsIPrincipal* callerPrincipal = nsContentUtils::SubjectPrincipal();
JS::Rooted<JSObject*> unwrappedStack(aCx, js::CheckedUnwrap(mStack));
if (!unwrappedStack) {
// We can't leak data here either.
return true;
}
nsIPrincipal* stackPrincipal =
nsJSPrincipals::get(js::GetSavedFramePrincipals(unwrappedStack));
return callerPrincipal->SubsumesConsideringDomain(stackPrincipal);
}
/* static */ already_AddRefed<nsIStackFrame>
JSStackFrame::CreateStack(JSContext* aCx, int32_t aMaxDepth)
{

View File

@ -83,6 +83,13 @@ TestInterfaceJS.prototype = {
return new this._win.Promise(func);
},
testPromiseWithDOMExceptionThrowingPromiseInit: function() {
return new this._win.Promise(() => {
throw new this._win.DOMException("We are a second DOMException",
"NotFoundError");
})
},
testPromiseWithThrowingChromeThenFunction: function() {
return this._win.Promise.resolve(5).then(function() {
noSuchMethodExistsYo2();
@ -93,6 +100,13 @@ TestInterfaceJS.prototype = {
return this._win.Promise.resolve(10).then(func);
},
testPromiseWithDOMExceptionThrowingThenFunction: function() {
return this._win.Promise.resolve(5).then(() => {
throw new this._win.DOMException("We are a third DOMException",
"NetworkError");
});
},
testPromiseWithThrowingChromeThenable: function() {
// We need to produce a thing that has a "then" property in the page
// compartment, since we plan to call the page-provided resolve function.
@ -112,6 +126,20 @@ TestInterfaceJS.prototype = {
// property won't return the function.
return this._win.Promise.resolve(Cu.waiveXrays(thenable));
},
testPromiseWithDOMExceptionThrowingThenable: function() {
// We need to produce a thing that has a "then" property in the page
// compartment, since we plan to call the page-provided resolve function.
var thenable = new this._win.Object();
Cu.waiveXrays(thenable).then = () => {
throw new this._win.DOMException("We are a fourth DOMException",
"TypeMismatchError");
}
return new this._win.Promise(function(resolve) {
resolve(thenable)
});
},
};
this.NSGetFactory = XPCOMUtils.generateNSGetFactory([TestInterfaceJS])

View File

@ -14,17 +14,21 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
SimpleTest.waitForExplicitFinish();
function checkExn(lineNumber, message, filename, testNumber, exn) {
function checkExn(lineNumber, name, message, code, filename, testNumber, stack, exn) {
ise(exn.lineNumber, lineNumber,
"Should have the right line number in test " + testNumber);
ise(exn.name, name,
"Should have the right exception name in test " + testNumber);
ise("filename" in exn ? exn.filename : exn.fileName, filename,
"Should have the right file name in test " + testNumber);
ise(exn.message, message,
"Should have the right message in test " + testNumber);
ise(exn.code, code, "Should have the right .code in test " + testNumber);
if (message === "") {
ise(exn.name, "NS_ERROR_UNEXPECTED",
"Should have one of our synthetic exceptions in test " + testNumber);
}
ise(exn.stack, stack, "Should have the right stack in test " + testNumber);
}
function ensurePromiseFail(testNumber, value) {
@ -39,34 +43,55 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=1107592
Promise.all([
t.testPromiseWithThrowingChromePromiseInit().then(
ensurePromiseFail.bind(null, 1),
checkExn.bind(null, 40, "", ourFile, 1)),
checkExn.bind(null, 44, "NS_ERROR_UNEXPECTED", "", undefined,
ourFile, 1,
"doTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:44:6\n")),
t.testPromiseWithThrowingContentPromiseInit(function() {
thereIsNoSuchContentFunction1();
}).then(
ensurePromiseFail.bind(null, 2),
checkExn.bind(null, 44,
checkExn.bind(null, 50, "ReferenceError",
"thereIsNoSuchContentFunction1 is not defined",
ourFile, 2)),
undefined, ourFile, 2,
"doTest/<@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:50:11\ndoTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:49:7\n")),
t.testPromiseWithThrowingChromeThenFunction().then(
ensurePromiseFail.bind(null, 3),
checkExn.bind(null, 0, "", "", 3)),
checkExn.bind(null, 0, "NS_ERROR_UNEXPECTED", "", undefined, "", 3, "")),
t.testPromiseWithThrowingContentThenFunction(function() {
thereIsNoSuchContentFunction2();
}).then(
ensurePromiseFail.bind(null, 4),
checkExn.bind(null, 54,
checkExn.bind(null, 61, "ReferenceError",
"thereIsNoSuchContentFunction2 is not defined",
ourFile, 4)),
undefined, ourFile, 4,
"doTest/<@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:61:11\n")),
t.testPromiseWithThrowingChromeThenable().then(
ensurePromiseFail.bind(null, 5),
checkExn.bind(null, 0, "", "", 5)),
checkExn.bind(null, 0, "NS_ERROR_UNEXPECTED", "", undefined, "", 5, "")),
t.testPromiseWithThrowingContentThenable({
then: function() { thereIsNoSuchContentFunction3(); }
}).then(
ensurePromiseFail.bind(null, 6),
checkExn.bind(null, 64,
checkExn.bind(null, 72, "ReferenceError",
"thereIsNoSuchContentFunction3 is not defined",
ourFile, 6)),
undefined, ourFile, 6,
"doTest/<.then@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:72:32\n")),
t.testPromiseWithDOMExceptionThrowingPromiseInit().then(
ensurePromiseFail.bind(null, 7),
checkExn.bind(null, 79, "NotFoundError",
"We are a second DOMException",
DOMException.NOT_FOUND_ERR, ourFile, 7,
"doTest@http://mochi.test:8888/tests/dom/bindings/test/test_promise_rejections_from_jsimplemented.html:79:6\n")),
t.testPromiseWithDOMExceptionThrowingThenFunction().then(
ensurePromiseFail.bind(null, 8),
checkExn.bind(null, 0, "NetworkError",
"We are a third DOMException",
DOMException.NETWORK_ERR, "", 8, "")),
t.testPromiseWithDOMExceptionThrowingThenable().then(
ensurePromiseFail.bind(null, 9),
checkExn.bind(null, 0, "TypeMismatchError",
"We are a fourth DOMException",
DOMException.TYPE_MISMATCH_ERR, "", 9, "")),
]).then(SimpleTest.finish,
function() {
ok(false, "One of our catch statements totally failed");

View File

@ -212,6 +212,11 @@ protected:
if (rv.Failed()) {
JS::Rooted<JS::Value> exn(cx);
if (rv.IsJSException()) {
// Enter the compartment of mPromise before stealing the JS exception,
// since the StealJSException call will use the current compartment for
// a security check that determines how much of the stack we're allowed
// to see and we'll be exposing that stack to consumers of mPromise.
JSAutoCompartment ac(cx, mPromise->GlobalJSObject());
rv.StealJSException(cx, &exn);
} else {
// Convert the ErrorResult to a JS exception object that we can reject
@ -579,7 +584,14 @@ Promise::CallInitFunction(const GlobalObject& aGlobal,
if (aRv.IsJSException()) {
JS::Rooted<JS::Value> value(cx);
aRv.StealJSException(cx, &value);
{ // scope for ac
// Enter the compartment of our global before stealing the JS exception,
// since the StealJSException call will use the current compartment for
// a security check that determines how much of the stack we're allowed
// to see, and we'll be exposing that stack to consumers of this promise.
JSAutoCompartment ac(cx, GlobalJSObject());
aRv.StealJSException(cx, &value);
}
// we want the same behavior as this JS implementation:
// function Promise(arg) { try { arg(a, b); } catch (e) { this.reject(e); }}

View File

@ -214,7 +214,15 @@ WrapperPromiseCallback::Call(JSContext* aCx,
if (rv.Failed()) {
JS::Rooted<JS::Value> value(aCx);
if (rv.IsJSException()) {
rv.StealJSException(aCx, &value);
{ // scope for ac
// Enter the compartment of mNextPromise before stealing the JS
// exception, since the StealJSException call will use the current
// compartment for a security check that determines how much of the
// stack we're allowed to see and we'll be exposing that stack to
// consumers of mPromise.
JSAutoCompartment ac(aCx, mNextPromise->GlobalJSObject());
rv.StealJSException(aCx, &value);
}
if (!JS_WrapValue(aCx, &value)) {
NS_WARNING("Failed to wrap value into the right compartment.");

View File

@ -59,8 +59,11 @@ interface TestInterfaceJS {
// Tests for promise-rejection behavior
Promise<void> testPromiseWithThrowingChromePromiseInit();
Promise<void> testPromiseWithThrowingContentPromiseInit(PromiseInit func);
Promise<void> testPromiseWithDOMExceptionThrowingPromiseInit();
Promise<void> testPromiseWithThrowingChromeThenFunction();
Promise<void> testPromiseWithThrowingContentThenFunction(AnyCallback func);
Promise<void> testPromiseWithDOMExceptionThrowingThenFunction();
Promise<void> testPromiseWithThrowingChromeThenable();
Promise<void> testPromiseWithThrowingContentThenable(object thenable);
Promise<void> testPromiseWithDOMExceptionThrowingThenable();
};

View File

@ -10,7 +10,9 @@
#include "nsISupports.idl"
[scriptable, uuid(f80ac10b-68dd-4482-a8c2-3cbe13fa89af)]
[ptr] native JSContext(JSContext);
[scriptable, uuid(4f2927e5-ae85-4c6b-93d2-2f639af6a236)]
interface nsIStackFrame : nsISupports
{
// see nsIProgrammingLanguage for list of language consts
@ -30,6 +32,10 @@ interface nsIStackFrame : nsISupports
readonly attribute AString formattedStack;
AUTF8String toString();
// Return whether this stack frame can be accessed by the caller. This is
// safe to call on non-main threads, but will always report "yes" there.
[noscript, notxpcom, nostdcall] boolean callerSubsumes(in JSContext aCx);
};
[scriptable, uuid(1caf1461-be1d-4b79-a552-5292b6bf3c35)]