Bug 1411384 - nested sync XHR should throw, r=smaug
☠☠ backed out by d138e03573ff ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Fri, 10 Nov 2017 19:25:31 +0100
changeset 391221 35bb5af0f317f65c257b69bff7bbfdb42ece6afe
parent 391220 dedc4be5ce80ac0e2e3864fba91f77c5583a8e17
child 391222 dc82839201a696be532b0309ceddd31b73960035
push id97232
push useramarchesini@mozilla.com
push dateFri, 10 Nov 2017 18:30:07 +0000
treeherdermozilla-inbound@a9246983376b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1411384
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1411384 - nested sync XHR should throw, r=smaug
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
dom/xhr/tests/mochitest.ini
dom/xhr/tests/test_nestedSyncXHR.html
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -168,16 +168,28 @@ private:
 static void AddLoadFlags(nsIRequest *request, nsLoadFlags newFlags)
 {
   nsLoadFlags flags;
   request->GetLoadFlags(&flags);
   flags |= newFlags;
   request->SetLoadFlags(flags);
 }
 
+// We are in a sync event loop.
+#define NOT_CALLABLE_IN_SYNC_SEND                              \
+  if (mFlagSyncLooping) {                                      \
+    return NS_ERROR_DOM_INVALID_STATE_XHR_HAS_INVALID_CONTEXT; \
+  }
+
+#define NOT_CALLABLE_IN_SYNC_SEND_RV                               \
+  if (mFlagSyncLooping) {                                          \
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_XHR_HAS_INVALID_CONTEXT); \
+    return;                                                        \
+  }
+
 /////////////////////////////////////////////
 //
 //
 /////////////////////////////////////////////
 
 bool
 XMLHttpRequestMainThread::sDontWarnAboutSyncXHR = false;
 
@@ -694,16 +706,18 @@ NS_IMETHODIMP XMLHttpRequestMainThread::
 
   return NS_OK;
 }
 
 void
 XMLHttpRequestMainThread::SetResponseType(XMLHttpRequestResponseType aResponseType,
                                           ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   if (mState == State::loading || mState == State::done) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_XHR_MUST_NOT_BE_LOADING_OR_DONE);
     return;
   }
 
   // sync request is not allowed setting responseType in window context
   if (HasOrHasHadOwner() && mState != State::unsent && mFlagSynchronous) {
     LogMessage("ResponseTypeSyncXHRWarning", GetOwner());
@@ -1088,16 +1102,23 @@ XMLHttpRequestMainThread::RequestErrorSt
 
   // Steps 7 and 8 (loadend is fired for us)
   DispatchProgressEvent(this, aEventType, 0, -1);
 }
 
 void
 XMLHttpRequestMainThread::Abort(ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+  AbortInternal(aRv);
+}
+
+void
+XMLHttpRequestMainThread::AbortInternal(ErrorResult& aRv)
+{
   mFlagAborted = true;
 
   // Step 1
   TerminateOngoingFetch();
 
   // Step 2
   if ((mState == State::opened && mFlagSend) ||
        mState == State::headers_received ||
@@ -1184,16 +1205,18 @@ XMLHttpRequestMainThread::GetAllResponse
   GetAllResponseHeaders(aOut, rv);
   return rv.StealNSResult();
 }
 
 void
 XMLHttpRequestMainThread::GetAllResponseHeaders(nsACString& aResponseHeaders,
                                                 ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   aResponseHeaders.Truncate();
 
   // If the state is UNSENT or OPENED,
   // return the empty string and terminate these steps.
   if (mState == State::unsent || mState == State::opened) {
     return;
   }
 
@@ -1249,16 +1272,18 @@ XMLHttpRequestMainThread::GetResponseHea
   GetResponseHeader(aHeader, aResult, rv);
   return rv.StealNSResult();
 }
 
 void
 XMLHttpRequestMainThread::GetResponseHeader(const nsACString& header,
                                             nsACString& _retval, ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   _retval.SetIsVoid(true);
 
   nsCOMPtr<nsIHttpChannel> httpChannel = GetCurrentHttpChannel();
 
   if (!httpChannel) {
     // If the state is UNSENT or OPENED,
     // return null and terminate these steps.
     if (mState == State::unsent || mState == State::opened) {
@@ -1510,17 +1535,20 @@ XMLHttpRequestMainThread::Open(const nsA
   }
 }
 
 nsresult
 XMLHttpRequestMainThread::Open(const nsACString& aMethod,
                                const nsACString& aUrl,
                                bool aAsync,
                                const nsAString& aUsername,
-                               const nsAString& aPassword) {
+                               const nsAString& aPassword)
+{
+  NOT_CALLABLE_IN_SYNC_SEND
+
   // Gecko-specific
   if (!aAsync && !DontWarnAboutSyncXHR() && GetOwner() &&
       GetOwner()->GetExtantDoc()) {
     GetOwner()->GetExtantDoc()->WarnOnceAbout(nsIDocument::eSyncXMLHttpRequest);
   }
 
   Telemetry::Accumulate(Telemetry::XMLHTTPREQUEST_ASYNC_OR_SYNC, aAsync ? 0 : 1);
 
@@ -2856,16 +2884,18 @@ XMLHttpRequestMainThread::UnsuppressEven
   }
 }
 
 void
 XMLHttpRequestMainThread::Send(JSContext* aCx,
                                const Nullable<DocumentOrBlobOrArrayBufferViewOrArrayBufferOrFormDataOrURLSearchParamsOrUSVString>& aData,
                                ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   if (aData.IsNull()) {
     aRv = SendInternal(nullptr);
     return;
   }
 
 
   if (aData.Value().IsDocument()) {
     BodyExtractor<nsIDocument> body(&aData.Value().GetAsDocument());
@@ -3151,16 +3181,18 @@ XMLHttpRequestMainThread::IsLowercaseRes
   return sIsLowercaseResponseHeaderEnabled;
 }
 
 // http://dvcs.w3.org/hg/xhr/raw-file/tip/Overview.html#dom-xmlhttprequest-setrequestheader
 NS_IMETHODIMP
 XMLHttpRequestMainThread::SetRequestHeader(const nsACString& aName,
                                            const nsACString& aValue)
 {
+  NOT_CALLABLE_IN_SYNC_SEND
+
   // Step 1
   if (mState != State::opened) {
     return NS_ERROR_DOM_INVALID_STATE_XHR_MUST_BE_OPENED;
   }
 
   // Step 2
   if (mFlagSend) {
     return NS_ERROR_DOM_INVALID_STATE_XHR_MUST_NOT_BE_SENDING;
@@ -3214,16 +3246,18 @@ XMLHttpRequestMainThread::SetTimeout(uin
   ErrorResult rv;
   SetTimeout(aTimeout, rv);
   return rv.StealNSResult();
 }
 
 void
 XMLHttpRequestMainThread::SetTimeout(uint32_t aTimeout, ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   if (mFlagSynchronous && mState != State::unsent && HasOrHasHadOwner()) {
     /* Timeout is not supported for synchronous requests with an owning window,
        per XHR2 spec. */
     LogMessage("TimeoutSyncXHRWarning", GetOwner());
     aRv.Throw(NS_ERROR_DOM_INVALID_ACCESS_XHR_TIMEOUT_AND_RESPONSETYPE_UNSUPPORTED_FOR_SYNC);
     return;
   }
 
@@ -3309,18 +3343,22 @@ XMLHttpRequestMainThread::ReadyState() c
     case State::done:
       return DONE;
     default:
       MOZ_CRASH("Unknown state");
   }
   return 0;
 }
 
-void XMLHttpRequestMainThread::OverrideMimeType(const nsAString& aMimeType, ErrorResult& aRv)
+void
+XMLHttpRequestMainThread::OverrideMimeType(const nsAString& aMimeType,
+                                           ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   if (mState == State::loading || mState == State::done) {
     ResetResponse();
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_XHR_MUST_NOT_BE_LOADING_OR_DONE);
     return;
   }
 
   mOverrideMimeType = aMimeType;
 }
@@ -3390,16 +3428,18 @@ XMLHttpRequestMainThread::SetWithCredent
   ErrorResult rv;
   SetWithCredentials(aWithCredentials, rv);
   return rv.StealNSResult();
 }
 
 void
 XMLHttpRequestMainThread::SetWithCredentials(bool aWithCredentials, ErrorResult& aRv)
 {
+  NOT_CALLABLE_IN_SYNC_SEND_RV
+
   // Return error if we're already processing a request.  Note that we can't use
   // ReadyState() here, because it can't differentiate between "opened" and
   // "sent", so we use mState directly.
 
   if ((mState != State::unsent && mState != State::opened) ||
       mFlagSend || mIsAnon) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_XHR_MUST_NOT_BE_SENDING);
     return;
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -347,18 +347,18 @@ public:
   void
   RequestErrorSteps(const ProgressEventType aEventType,
                     const nsresult aOptionalException,
                     ErrorResult& aRv);
 
   void
   Abort()
   {
-    ErrorResult rv;
-    Abort(rv);
+    IgnoredErrorResult rv;
+    AbortInternal(rv);
     MOZ_ASSERT(!rv.Failed());
   }
 
   virtual void
   Abort(ErrorResult& aRv) override;
 
   // response
   virtual void
@@ -558,16 +558,19 @@ protected:
 
   void DispatchOrStoreEvent(DOMEventTargetHelper* aTarget, Event* aEvent);
 
   already_AddRefed<nsXMLHttpRequestXPCOMifier> EnsureXPCOMifier();
 
   void SuspendEventDispatching();
   void ResumeEventDispatching();
 
+  void
+  AbortInternal(ErrorResult& aRv);
+
   struct PendingEvent
   {
     RefPtr<DOMEventTargetHelper> mTarget;
     RefPtr<Event> mEvent;
   };
 
   nsTArray<PendingEvent> mPendingEvents;
 
--- a/dom/xhr/tests/mochitest.ini
+++ b/dom/xhr/tests/mochitest.ini
@@ -108,8 +108,9 @@ skip-if = (buildapp == 'b2g') # b2g-debu
 [test_XHR_timeout.html]
 skip-if = buildapp == 'b2g' || (android_version == '18' && debug) # b2g(flaky on B2G, bug 960743) b2g-debug(flaky on B2G, bug 960743) b2g-desktop(flaky on B2G, bug 960743)
 support-files = test_XHR_timeout.js
 [test_xhr_withCredentials.html]
 [test_XHRDocURI.html]
 [test_XHRResponseURL.html]
 [test_XHRSendData.html]
 [test_sync_xhr_document_write_with_iframe.html]
+[test_nestedSyncXHR.html]
new file mode 100644
--- /dev/null
+++ b/dom/xhr/tests/test_nestedSyncXHR.html
@@ -0,0 +1,88 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test for sync XHR into sync XHRs</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+  <script type="application/javascript">
+
+
+let xhr = new XMLHttpRequest();
+
+let frame = document.createElement('frame');
+frame.addEventListener('load', function() {
+  try {
+    xhr.responseType = "blob";
+    ok(false, "xhr.responseType cannot be settable");
+  } catch(e) {
+    ok(true, "xhr.responseType cannot be settable");
+  }
+
+  try {
+    xhr.abort();
+    ok(false, "xhr.abort should throw");
+  } catch(e) {
+    ok(true, "xhr.abort should throw");
+  }
+
+  try {
+    xhr.getAllResponseHeaders();
+    ok(false, "xhr.getAllResponseHeaders should throw");
+  } catch(e) {
+    ok(true, "xhr.getAllResponseHeaders should throw");
+  }
+
+  try {
+    xhr.getResponseHeader("foo");
+    ok(false, "xhr.getResponseHeader should throw");
+  } catch(e) {
+    ok(true, "xhr.getResponseHeader should throw");
+  }
+
+  try {
+    xhr.open('POST', location, false);
+    ok(false, "xhr.open should throw");
+  } catch(e) {
+    ok(true, "xhr.open should throw");
+  }
+
+  try {
+    xhr.send();
+    ok(false, "xhr.send should throw");
+  } catch(e) {
+    ok(true, "xhr.send should throw");
+  }
+
+  try {
+    xhr.timeout = 42;
+    ok(false, "xhr.timeout cannot be settable");
+  } catch(e) {
+    ok(true, "xhr.timeout cannot be settable");
+  }
+
+  try {
+    xhr.withCredentials = false;
+    ok(false, "xhr.withCredentials cannot be settable");
+  } catch(e) {
+    ok(true, "xhr.withCredentials cannot be settable");
+  }
+
+  try {
+    xhr.overrideMimeType("wow")
+    ok(false, "xhr.overrideMimeType should throw");
+  } catch(e) {
+    ok(true, "xhr.overrideMimeType should throw");
+  }
+}, { once: true });
+
+document.documentElement.appendChild(frame);
+xhr.open('POST', location, false);
+xhr.send('X');
+
+frame.remove();
+  </script>
+</body>
+</html>