From fe819ad8d74ab67516061802420e60f14a2e4ebb Mon Sep 17 00:00:00 2001 From: Nikhil Marathe Date: Wed, 19 Aug 2015 16:21:25 -0700 Subject: [PATCH] Bug 1180861 - Various ServiceWorker registration fixes to get test passing. r=bkelly,jgraham. This commit implements the following changes to get registration.https.html working. 1) Fail with NS_ERROR_DOM_SECURITY_ERR where the spec requires it. 2) Propagate JSExnType to ServiceWorkerManager::HandleError() so that a JS exception object with the correct .name can be created. 3) Fail with security error on redirect failure. 4) Check fetched script's mimetype. 5) Add missing python server files for web-platform-tests. 6) Update web-platform-tests expected data. 7) Several tests have been changed to use TypeError or more appropriate JS errors based on my reading of the spec. --HG-- extra : commitid : IxWo2IVUweU extra : rebase_source : c3c1ead153027bf84e7f239fd7125224fe25c3c0 --- dom/workers/ServiceWorkerContainer.cpp | 4 +- dom/workers/ServiceWorkerManager.cpp | 22 +++++--- dom/workers/ServiceWorkerManager.h | 5 +- dom/workers/ServiceWorkerScriptCache.cpp | 36 +++++++++---- dom/workers/WorkerPrivate.cpp | 19 ++++--- .../test_installation_simple.html | 4 +- .../registration.https.html.ini | 50 ------------------- .../service-worker/registration.https.html | 20 ++++---- .../invalid-chunked-encoding-with-flush.py | 12 +++++ .../resources/invalid-chunked-encoding.py | 2 + .../resources/malformed-worker.py | 10 ++++ .../resources/mime-type-worker.py | 4 ++ .../service-worker/resources/redirect.py | 25 ++++++++++ 13 files changed, 123 insertions(+), 90 deletions(-) delete mode 100644 testing/web-platform/mozilla/meta/service-workers/service-worker/registration.https.html.ini create mode 100644 testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding-with-flush.py create mode 100644 testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding.py create mode 100644 testing/web-platform/mozilla/tests/service-workers/service-worker/resources/malformed-worker.py create mode 100644 testing/web-platform/mozilla/tests/service-workers/service-worker/resources/mime-type-worker.py create mode 100644 testing/web-platform/mozilla/tests/service-workers/service-worker/resources/redirect.py diff --git a/dom/workers/ServiceWorkerContainer.cpp b/dom/workers/ServiceWorkerContainer.cpp index 4a86bf1f8c71..01b449e2b24d 100644 --- a/dom/workers/ServiceWorkerContainer.cpp +++ b/dom/workers/ServiceWorkerContainer.cpp @@ -106,9 +106,11 @@ CheckForSlashEscapedCharsInPath(nsIURI* aURI) { MOZ_ASSERT(aURI); + // A URL that can't be downcast to a standard URL is an invalid URL and should + // be treated as such and fail with SecurityError. nsCOMPtr url(do_QueryInterface(aURI)); if (NS_WARN_IF(!url)) { - return NS_ERROR_FAILURE; + return NS_ERROR_DOM_SECURITY_ERR; } nsAutoCString path; diff --git a/dom/workers/ServiceWorkerManager.cpp b/dom/workers/ServiceWorkerManager.cpp index eecbb99648e0..275fae951839 100644 --- a/dom/workers/ServiceWorkerManager.cpp +++ b/dom/workers/ServiceWorkerManager.cpp @@ -574,7 +574,7 @@ public: } void - UpdateFailed(const ErrorEventInit& aErrorDesc) override + UpdateFailed(JSExnType aExnType, const ErrorEventInit& aErrorDesc) override { AutoJSAPI jsapi; jsapi.Init(mWindow); @@ -598,7 +598,8 @@ public: JS::Rooted msg(cx, msgval.toString()); JS::Rooted error(cx); - if (!JS::CreateError(cx, JSEXN_ERR, nullptr, fn, aErrorDesc.mLineno, + if ((aExnType < JSEXN_ERR) || + !JS::CreateError(cx, aExnType, nullptr, fn, aErrorDesc.mLineno, aErrorDesc.mColno, nullptr, msg, &error)) { JS_ClearPendingException(cx); mPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR); @@ -975,13 +976,17 @@ public: const nsACString& aMaxScope) override { nsRefPtr kungFuDeathGrip = this; - if (mCanceled) { + if (NS_WARN_IF(mCanceled)) { Fail(NS_ERROR_DOM_TYPE_ERR); return; } if (NS_WARN_IF(NS_FAILED(aStatus))) { - Fail(NS_ERROR_DOM_TYPE_ERR); + if (aStatus == NS_ERROR_DOM_SECURITY_ERR) { + Fail(aStatus); + } else { + Fail(NS_ERROR_DOM_TYPE_ERR); + } return; } @@ -1101,7 +1106,7 @@ public: // Public so our error handling code can use it. // Callers MUST hold a strong ref before calling this! void - Fail(const ErrorEventInit& aError) + Fail(JSExnType aExnType, const ErrorEventInit& aError) { MOZ_ASSERT(mCallback); nsRefPtr callback = mCallback.forget(); @@ -1111,7 +1116,7 @@ public: // FailCommon relies on it. // FailCommon does check for cancellation, but let's be safe here. nsRefPtr kungFuDeathGrip = this; - callback->UpdateFailed(aError); + callback->UpdateFailed(aExnType, aError); FailCommon(NS_ERROR_DOM_JS_EXCEPTION); } @@ -2927,7 +2932,8 @@ ServiceWorkerManager::HandleError(JSContext* aCx, nsString aLine, uint32_t aLineNumber, uint32_t aColumnNumber, - uint32_t aFlags) + uint32_t aFlags, + JSExnType aExnType) { AssertIsOnMainThread(); MOZ_ASSERT(aPrincipal); @@ -2967,7 +2973,7 @@ ServiceWorkerManager::HandleError(JSContext* aCx, "Script error caused ServiceWorker registration to fail: %s:%u '%s'", NS_ConvertUTF16toUTF8(aFilename).get(), aLineNumber, NS_ConvertUTF16toUTF8(aMessage).get()).get()); - regJob->Fail(init); + regJob->Fail(aExnType, init); } return true; diff --git a/dom/workers/ServiceWorkerManager.h b/dom/workers/ServiceWorkerManager.h index d9c6f74dad43..c2de794eaf63 100644 --- a/dom/workers/ServiceWorkerManager.h +++ b/dom/workers/ServiceWorkerManager.h @@ -148,7 +148,7 @@ public: { } virtual - void UpdateFailed(const ErrorEventInit& aDesc) + void UpdateFailed(JSExnType aExnType, const ErrorEventInit& aDesc) { } }; @@ -375,7 +375,8 @@ public: nsString aLine, uint32_t aLineNumber, uint32_t aColumnNumber, - uint32_t aFlags); + uint32_t aFlags, + JSExnType aExnType); void GetAllClients(nsIPrincipal* aPrincipal, diff --git a/dom/workers/ServiceWorkerScriptCache.cpp b/dom/workers/ServiceWorkerScriptCache.cpp index b19872b5ce34..9f2fb7494f49 100644 --- a/dom/workers/ServiceWorkerScriptCache.cpp +++ b/dom/workers/ServiceWorkerScriptCache.cpp @@ -366,7 +366,7 @@ public: mNetworkFinished = true; - if (NS_FAILED(aStatus)) { + if (NS_WARN_IF(NS_FAILED(aStatus))) { if (mCC) { mCC->Abort(); } @@ -386,7 +386,7 @@ public: mCacheFinished = true; mInCache = aInCache; - if (NS_FAILED(aStatus)) { + if (NS_WARN_IF(NS_FAILED(aStatus))) { if (mCN) { mCN->Abort(); } @@ -407,7 +407,7 @@ public: return; } - if (!mCC || !mInCache) { + if (NS_WARN_IF(!mCC || !mInCache)) { ComparisonFinished(NS_OK, false); return; } @@ -536,7 +536,7 @@ private: AssertIsOnMainThread(); MOZ_ASSERT(mCallback); - if (NS_FAILED(aStatus)) { + if (NS_WARN_IF(NS_FAILED(aStatus))) { Fail(aStatus); return; } @@ -695,7 +695,11 @@ CompareNetwork::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext } if (NS_WARN_IF(NS_FAILED(aStatus))) { - mManager->NetworkFinished(aStatus); + if (aStatus == NS_ERROR_REDIRECT_LOOP) { + mManager->NetworkFinished(NS_ERROR_DOM_SECURITY_ERR); + } else { + mManager->NetworkFinished(aStatus); + } return NS_OK; } @@ -715,18 +719,32 @@ CompareNetwork::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext return NS_OK; } - if (!requestSucceeded) { + if (NS_WARN_IF(!requestSucceeded)) { mManager->NetworkFinished(NS_ERROR_FAILURE); return NS_OK; } nsAutoCString maxScope; // Note: we explicitly don't check for the return value here, because the - // absense of the header is not an error condition. + // absence of the header is not an error condition. unused << httpChannel->GetResponseHeader(NS_LITERAL_CSTRING("Service-Worker-Allowed"), maxScope); mManager->SetMaxScope(maxScope); + + nsAutoCString mimeType; + rv = httpChannel->GetContentType(mimeType); + if (NS_WARN_IF(NS_FAILED(rv))) { + mManager->NetworkFinished(NS_ERROR_DOM_SECURITY_ERR); + return rv; + } + + if (!mimeType.LowerCaseEqualsLiteral("text/javascript") && + !mimeType.LowerCaseEqualsLiteral("application/x-javascript") && + !mimeType.LowerCaseEqualsLiteral("application/javascript")) { + mManager->NetworkFinished(NS_ERROR_DOM_SECURITY_ERR); + return rv; + } } else { // The only supported request schemes are http, https, and app. @@ -752,14 +770,12 @@ CompareNetwork::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext return NS_OK; } - if (!scheme.LowerCaseEqualsLiteral("app")) { + if (NS_WARN_IF(!scheme.LowerCaseEqualsLiteral("app"))) { mManager->NetworkFinished(NS_ERROR_FAILURE); return NS_OK; } } - // FIXME(nsm): "Extract mime type..." - char16_t* buffer = nullptr; size_t len = 0; diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index f5508e4f0916..72b96d8b13b0 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1366,6 +1366,7 @@ class ReportErrorRunnable final : public WorkerRunnable uint32_t mColumnNumber; uint32_t mFlags; uint32_t mErrorNumber; + JSExnType mExnType; public: // aWorkerPrivate is the worker thread we're on (or the main thread, if null) @@ -1377,7 +1378,7 @@ public: const nsString& aMessage, const nsString& aFilename, const nsString& aLine, uint32_t aLineNumber, uint32_t aColumnNumber, uint32_t aFlags, - uint32_t aErrorNumber, uint64_t aInnerWindowId) + uint32_t aErrorNumber, JSExnType aExnType, uint64_t aInnerWindowId) { if (aWorkerPrivate) { aWorkerPrivate->AssertIsOnWorkerThread(); @@ -1482,7 +1483,7 @@ public: nsRefPtr runnable = new ReportErrorRunnable(aWorkerPrivate, aMessage, aFilename, aLine, aLineNumber, aColumnNumber, aFlags, - aErrorNumber); + aErrorNumber, aExnType); return runnable->Dispatch(aCx); } @@ -1496,11 +1497,12 @@ private: ReportErrorRunnable(WorkerPrivate* aWorkerPrivate, const nsString& aMessage, const nsString& aFilename, const nsString& aLine, uint32_t aLineNumber, uint32_t aColumnNumber, - uint32_t aFlags, uint32_t aErrorNumber) + uint32_t aFlags, uint32_t aErrorNumber, + JSExnType aExnType) : WorkerRunnable(aWorkerPrivate, ParentThreadUnchangedBusyCount), mMessage(aMessage), mFilename(aFilename), mLine(aLine), mLineNumber(aLineNumber), mColumnNumber(aColumnNumber), mFlags(aFlags), - mErrorNumber(aErrorNumber) + mErrorNumber(aErrorNumber), mExnType(aExnType) { } virtual void @@ -1544,7 +1546,7 @@ private: aWorkerPrivate->ScriptURL(), mMessage, mFilename, mLine, mLineNumber, - mColumnNumber, mFlags); + mColumnNumber, mFlags, mExnType); if (handled) { return true; } @@ -1573,7 +1575,7 @@ private: return ReportError(aCx, parent, fireAtScope, aWorkerPrivate, mMessage, mFilename, mLine, mLineNumber, mColumnNumber, mFlags, - mErrorNumber, innerWindowId); + mErrorNumber, mExnType, innerWindowId); } }; @@ -6383,6 +6385,7 @@ WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage, nsString message, filename, line; uint32_t lineNumber, columnNumber, flags, errorNumber; + JSExnType exnType = JSEXN_ERR; if (aReport) { // ErrorEvent objects don't have a |name| field the way ES |Error| objects @@ -6404,6 +6407,8 @@ WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage, columnNumber = aReport->uctokenptr - aReport->uclinebuf; flags = aReport->flags; errorNumber = aReport->errorNumber; + MOZ_ASSERT(aReport->exnType >= JSEXN_NONE && aReport->exnType < JSEXN_LIMIT); + exnType = JSExnType(aReport->exnType); } else { lineNumber = columnNumber = errorNumber = 0; @@ -6425,7 +6430,7 @@ WorkerPrivate::ReportError(JSContext* aCx, const char* aMessage, if (!ReportErrorRunnable::ReportError(aCx, this, fireAtScope, nullptr, message, filename, line, lineNumber, - columnNumber, flags, errorNumber, 0)) { + columnNumber, flags, errorNumber, exnType, 0)) { JS_ReportPendingException(aCx); } diff --git a/dom/workers/test/serviceworkers/test_installation_simple.html b/dom/workers/test/serviceworkers/test_installation_simple.html index 40b107bb2311..1806b7a4083b 100644 --- a/dom/workers/test/serviceworkers/test_installation_simple.html +++ b/dom/workers/test/serviceworkers/test_installation_simple.html @@ -90,9 +90,9 @@ function redirectError() { return navigator.serviceWorker.register("redirect_serviceworker.sjs", { scope: "redirect_error/" }).then(function(swr) { - ok(false, "redirection should fail with TypeError"); + ok(false, "redirection should fail"); }, function (e) { - ok(e.name === "TypeError", "redirection should fail with TypeError"); + ok(e.name === "SecurityError", "redirection should fail with SecurityError"); }); } diff --git a/testing/web-platform/mozilla/meta/service-workers/service-worker/registration.https.html.ini b/testing/web-platform/mozilla/meta/service-workers/service-worker/registration.https.html.ini deleted file mode 100644 index 9a10751eb999..000000000000 --- a/testing/web-platform/mozilla/meta/service-workers/service-worker/registration.https.html.ini +++ /dev/null @@ -1,50 +0,0 @@ -[registration.https.html] - type: testharness - [Registering non-existent script] - expected: FAIL - - [Registering invalid chunked encoding script] - expected: FAIL - - [Registering invalid chunked encoding script with flush] - expected: FAIL - - [Registering script with no MIME type] - expected: FAIL - - [Registering script with bad MIME type] - expected: FAIL - - [Registering redirected script] - expected: FAIL - - [Registering script including parse error] - expected: FAIL - - [Registering script including undefined error] - expected: FAIL - - [Registering script including uncaught exception] - expected: FAIL - - [Registering script including caught exception] - expected: FAIL - - [Registering script importing malformed script] - expected: FAIL - - [Registering script importing non-existent script] - expected: FAIL - - [Script URL including URL-encoded slash] - expected: FAIL - - [Scope including URL-encoded slash] - expected: FAIL - - [Script URL including URL-encoded backslash] - expected: FAIL - - [Scope including URL-encoded backslash] - expected: FAIL - diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/registration.https.html b/testing/web-platform/mozilla/tests/service-workers/service-worker/registration.https.html index 101c71c5e46c..bde78d149b17 100644 --- a/testing/web-platform/mozilla/tests/service-workers/service-worker/registration.https.html +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/registration.https.html @@ -87,7 +87,7 @@ promise_test(function(t) { var scope = 'resources/scope/no-such-worker'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'NetworkError', + new TypeError(), 'Registration of non-existent script should fail.'); }, 'Registering non-existent script'); @@ -96,7 +96,7 @@ promise_test(function(t) { var scope = 'resources/scope/invalid-chunked-encoding/'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'NetworkError', + new TypeError(), 'Registration of invalid chunked encoding script should fail.'); }, 'Registering invalid chunked encoding script'); @@ -105,7 +105,7 @@ promise_test(function(t) { var scope = 'resources/scope/invalid-chunked-encoding-with-flush/'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'NetworkError', + new TypeError(), 'Registration of invalid chunked encoding script should fail.'); }, 'Registering invalid chunked encoding script with flush'); @@ -142,7 +142,7 @@ promise_test(function(t) { var scope = 'resources/scope/parse-error'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'AbortError', + new SyntaxError(), 'Registration of script including parse error should fail.'); }, 'Registering script including parse error'); @@ -151,7 +151,7 @@ promise_test(function(t) { var scope = 'resources/scope/undefined-error'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'AbortError', + new ReferenceError(), 'Registration of script including undefined error should fail.'); }, 'Registering script including undefined error'); @@ -180,7 +180,7 @@ promise_test(function(t) { var scope = 'resources/scope/import-malformed-script'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'AbortError', + new SyntaxError(), 'Registration of script importing malformed script should fail.'); }, 'Registering script importing malformed script'); @@ -228,7 +228,7 @@ promise_test(function(t) { var scope = 'resources/scope/encoded-slash-in-script-url'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'SecurityError', + new TypeError(), 'URL-encoded slash in the script URL should be rejected.'); }, 'Script URL including URL-encoded slash'); @@ -237,7 +237,7 @@ promise_test(function(t) { var scope = 'resources/scope%2fencoded-slash-in-scope'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'SecurityError', + new TypeError(), 'URL-encoded slash in the scope should be rejected.'); }, 'Scope including URL-encoded slash'); @@ -246,7 +246,7 @@ promise_test(function(t) { var scope = 'resources/scope/encoded-slash-in-script-url'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'SecurityError', + new TypeError(), 'URL-encoded backslash in the script URL should be rejected.'); }, 'Script URL including URL-encoded backslash'); @@ -255,7 +255,7 @@ promise_test(function(t) { var scope = 'resources/scope%5cencoded-slash-in-scope'; return assert_promise_rejects( navigator.serviceWorker.register(script, {scope: scope}), - 'SecurityError', + new TypeError(), 'URL-encoded backslash in the scope should be rejected.'); }, 'Scope including URL-encoded backslash'); diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding-with-flush.py b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding-with-flush.py new file mode 100644 index 000000000000..4b5194b8d48c --- /dev/null +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding-with-flush.py @@ -0,0 +1,12 @@ +import time +def main(request, response): + print dir(response) + response.headers.set("Content-Type", "application/javascript") + response.headers.set("Transfer-encoding", "chunked") + response.write_status_headers() + + time.sleep(1) + response.explicit_flush = True + + response.writer.write("XX\r\n\r\n") + response.writer.flush() diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding.py b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding.py new file mode 100644 index 000000000000..ae2c1f21b2ec --- /dev/null +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/invalid-chunked-encoding.py @@ -0,0 +1,2 @@ +def main(request, response): + return [("Content-Type", "application/javascript"), ("Transfer-encoding", "chunked")], "XX\r\n\r\n" diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/malformed-worker.py b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/malformed-worker.py new file mode 100644 index 000000000000..501521ff3e37 --- /dev/null +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/malformed-worker.py @@ -0,0 +1,10 @@ +def main(request, response): + headers = [("Content-Type", "application/javascript")] + + body = {'parse-error': 'var foo = function() {;', + 'undefined-error': 'foo.bar = 42;', + 'uncaught-exception': 'throw new DOMException("AbortError");', + 'caught-exception': 'try { throw new Error; } catch(e) {}', + 'import-malformed-script': 'importScripts("malformed-worker.py?parse-error");', + 'import-no-such-script': 'importScripts("no-such-script.js");'}[request.url_parts.query] + return headers, body diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/mime-type-worker.py b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/mime-type-worker.py new file mode 100644 index 000000000000..a16684de5cb6 --- /dev/null +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/mime-type-worker.py @@ -0,0 +1,4 @@ +def main(request, response): + if 'mime' in request.GET: + return [('Content-Type', request.GET['mime'])], "" + return [], "" diff --git a/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/redirect.py b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/redirect.py new file mode 100644 index 000000000000..20521b00c9ca --- /dev/null +++ b/testing/web-platform/mozilla/tests/service-workers/service-worker/resources/redirect.py @@ -0,0 +1,25 @@ +def main(request, response): + if 'Status' in request.GET: + status = int(request.GET["Status"]) + else: + status = 302 + + headers = [] + + url = request.GET['Redirect'] + headers.append(("Location", url)) + + if "ACAOrigin" in request.GET: + for item in request.GET["ACAOrigin"].split(","): + headers.append(("Access-Control-Allow-Origin", item)) + + for suffix in ["Headers", "Methods", "Credentials"]: + query = "ACA%s" % suffix + header = "Access-Control-Allow-%s" % suffix + if query in request.GET: + headers.append((header, request.GET[query])) + + if "ACEHeaders" in request.GET: + headers.append(("Access-Control-Expose-Headers", request.GET["ACEHeaders"])) + + return status, headers, ""