Bug 1695216 - Follow the XMLHttpRequest spec more closely for network and other errors; r=kershaw,sunil

- clear the response on network failure (bad chunks, etc).
- throw the expected error for abort/timeout/error for sync XHR failures.

Differential Revision: https://phabricator.services.mozilla.com/D183782
This commit is contained in:
Thomas Wisniewski
2023-07-21 02:41:22 +00:00
parent d206d4737a
commit 0412a44244
7 changed files with 193 additions and 56 deletions

View File

@@ -3,28 +3,24 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
var gExpectedStatus = null;
var gNextTestFunc = null;
var prefs = Services.prefs;
var asyncXHR = {
load() {
var request = new XMLHttpRequest();
request.open("GET", "http://localhost:4444/test_error_code.xml", true);
function asyncXHR(expectedStatus, nextTestFunc) {
var xhr = new XMLHttpRequest();
xhr.open("GET", "http://localhost:4444/test_error_code.xml", true);
var self = this;
request.addEventListener("error", function (event) {
self.onError(event);
});
request.send(null);
},
onError: function doAsyncRequest_onError(event) {
var sawError = false;
xhr.addEventListener("loadend", function doAsyncRequest_onLoad(event) {
Assert.ok(sawError, "Should have received an error");
nextTestFunc();
});
xhr.addEventListener("error", function doAsyncRequest_onError(event) {
var request = event.target.channel.QueryInterface(Ci.nsIRequest);
Assert.equal(request.status, gExpectedStatus);
gNextTestFunc();
},
};
Assert.equal(request.status, expectedStatus);
sawError = true;
});
xhr.send(null);
}
function run_test() {
do_test_pending();
@@ -41,10 +37,8 @@ function run_test_pt1() {
// We always resolve localhost as it's hardcoded without the following pref:
prefs.setBoolPref("network.proxy.allow_hijacking_localhost", true);
gExpectedStatus = Cr.NS_ERROR_OFFLINE;
gNextTestFunc = run_test_pt2;
dump("Testing error returned by async XHR when the network is offline\n");
asyncXHR.load();
asyncXHR(Cr.NS_ERROR_OFFLINE, run_test_pt2);
}
// connection refused
@@ -53,10 +47,8 @@ function run_test_pt2() {
prefs.clearUserPref("network.dns.offline-localhost");
prefs.clearUserPref("network.proxy.allow_hijacking_localhost");
gExpectedStatus = Cr.NS_ERROR_CONNECTION_REFUSED;
gNextTestFunc = end_test;
dump("Testing error returned by aync XHR when the connection is refused\n");
asyncXHR.load();
asyncXHR(Cr.NS_ERROR_CONNECTION_REFUSED, end_test);
}
function end_test() {

View File

@@ -7,9 +7,12 @@
#include "XMLHttpRequest.h"
#include "XMLHttpRequestMainThread.h"
#include "XMLHttpRequestWorker.h"
#include "mozilla/Logging.h"
#include "mozilla/net/CookieJarSettings.h"
#include "nsGlobalWindowInner.h"
mozilla::LazyLogModule gXMLHttpRequestLog("XMLHttpRequest");
namespace mozilla::dom {
/* static */

View File

@@ -111,6 +111,8 @@
# undef CreateFile
#endif
extern mozilla::LazyLogModule gXMLHttpRequestLog;
using namespace mozilla::net;
namespace mozilla::dom {
@@ -206,6 +208,7 @@ XMLHttpRequestMainThread::XMLHttpRequestMainThread(
mRequestSentTime(0),
mTimeoutMilliseconds(0),
mErrorLoad(ErrorType::eOK),
mErrorLoadDetail(NS_OK),
mErrorParsingXML(false),
mWaitingForOnStopRequest(false),
mProgressTimerIsActive(false),
@@ -909,29 +912,43 @@ void XMLHttpRequestMainThread::GetStatusText(nsACString& aStatusText,
}
}
void XMLHttpRequestMainThread::TerminateOngoingFetch() {
void XMLHttpRequestMainThread::TerminateOngoingFetch(nsresult detail) {
if ((mState == XMLHttpRequest_Binding::OPENED && mFlagSend) ||
mState == XMLHttpRequest_Binding::HEADERS_RECEIVED ||
mState == XMLHttpRequest_Binding::LOADING) {
CloseRequest();
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Info,
("%p TerminateOngoingFetch(0x%" PRIx32 ")", this,
static_cast<uint32_t>(detail)));
CloseRequest(detail);
}
}
void XMLHttpRequestMainThread::CloseRequest() {
void XMLHttpRequestMainThread::CloseRequest(nsresult detail) {
mWaitingForOnStopRequest = false;
mErrorLoad = ErrorType::eTerminated;
mErrorLoadDetail = detail;
if (mChannel) {
mChannel->CancelWithReason(NS_BINDING_ABORTED,
"XMLHttpRequestMainThread::CloseRequest"_ns);
}
if (mTimeoutTimer) {
mTimeoutTimer->Cancel();
}
CancelTimeoutTimer();
}
void XMLHttpRequestMainThread::CloseRequestWithError(
const ProgressEventType aType) {
CloseRequest();
MOZ_LOG(
gXMLHttpRequestLog, LogLevel::Debug,
("%p CloseRequestWithError(%hhu)", this, static_cast<uint8_t>(aType)));
nsresult detail = NS_ERROR_DOM_UNKNOWN_ERR;
if (aType == ProgressEventType::abort) {
detail = NS_ERROR_DOM_ABORT_ERR;
} else if (aType == ProgressEventType::error) {
detail = NS_ERROR_DOM_NETWORK_ERR;
} else if (aType == ProgressEventType::timeout) {
detail = NS_ERROR_DOM_TIMEOUT_ERR;
}
CloseRequest(detail);
ResetResponse();
@@ -968,11 +985,20 @@ void XMLHttpRequestMainThread::CloseRequestWithError(
void XMLHttpRequestMainThread::RequestErrorSteps(
const ProgressEventType aEventType, const nsresult aOptionalException,
ErrorResult& aRv) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p RequestErrorSteps(%hhu,0x%" PRIx32 ")", this,
static_cast<uint8_t>(aEventType),
static_cast<uint32_t>(aOptionalException)));
// Cancel our timers first before setting our state to done, so we don't
// trip any assertions if one fires and asserts that state != done.
CancelTimeoutTimer();
CancelSyncTimeoutTimer();
StopProgressEventTimer();
// Step 1
mState = XMLHttpRequest_Binding::DONE;
StopProgressEventTimer();
// Step 2
mFlagSend = false;
@@ -1012,21 +1038,23 @@ void XMLHttpRequestMainThread::RequestErrorSteps(
void XMLHttpRequestMainThread::Abort(ErrorResult& aRv) {
NOT_CALLABLE_IN_SYNC_SEND_RV
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug, ("%p Abort()", this));
AbortInternal(aRv);
}
void XMLHttpRequestMainThread::AbortInternal(ErrorResult& aRv) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug, ("%p AbortInternal()", this));
mFlagAborted = true;
DisconnectDoneNotifier();
// Step 1
TerminateOngoingFetch();
TerminateOngoingFetch(NS_ERROR_DOM_ABORT_ERR);
// Step 2
if ((mState == XMLHttpRequest_Binding::OPENED && mFlagSend) ||
mState == XMLHttpRequest_Binding::HEADERS_RECEIVED ||
mState == XMLHttpRequest_Binding::LOADING) {
RequestErrorSteps(ProgressEventType::abort, NS_OK, aRv);
RequestErrorSteps(ProgressEventType::abort, NS_ERROR_DOM_ABORT_ERR, aRv);
}
// Step 3
@@ -1278,6 +1306,15 @@ void XMLHttpRequestMainThread::DispatchProgressEvent(
ProgressEvent::Constructor(aTarget, typeString, init);
event->SetTrusted(true);
if (MOZ_LOG_TEST(gXMLHttpRequestLog, LogLevel::Debug)) {
nsAutoString type;
event->GetType(type);
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("firing %s event (%u,%u,%" PRIu64 ",%" PRIu64 ")",
NS_ConvertUTF16toUTF8(type).get(), aTarget == mUpload,
aTotal != -1, aLoaded, (aTotal == -1) ? 0 : aTotal));
}
DispatchOrStoreEvent(aTarget, event);
// If we're sending a load, error, timeout or abort event, then
@@ -1487,7 +1524,7 @@ void XMLHttpRequestMainThread::Open(const nsACString& aMethod,
}
// Step 10
TerminateOngoingFetch();
TerminateOngoingFetch(NS_OK);
// Step 11
// timeouts are handled without a flag
@@ -1835,6 +1872,7 @@ XMLHttpRequestMainThread::OnStartRequest(nsIRequest* request) {
request->GetStatus(&status);
if (mErrorLoad == ErrorType::eOK && NS_FAILED(status)) {
mErrorLoad = ErrorType::eRequest;
mErrorLoadDetail = status;
}
// Upload phase is now over. If we were uploading anything,
@@ -2105,7 +2143,7 @@ XMLHttpRequestMainThread::OnStopRequest(nsIRequest* request, nsresult status) {
if (status == NS_BINDING_ABORTED) {
mFlagParseBody = false;
IgnoredErrorResult rv;
RequestErrorSteps(ProgressEventType::abort, NS_OK, rv);
RequestErrorSteps(ProgressEventType::abort, NS_ERROR_DOM_ABORT_ERR, rv);
ChangeState(XMLHttpRequest_Binding::UNSENT, false);
return NS_OK;
}
@@ -2208,7 +2246,28 @@ XMLHttpRequestMainThread::OnStopRequest(nsIRequest* request, nsresult status) {
// reasons are that the user leaves the page or hits the ESC key.
mErrorLoad = ErrorType::eUnreachable;
mErrorLoadDetail = status;
mResponseXML = nullptr;
// Handle network errors specifically per spec.
if (NS_ERROR_GET_MODULE(status) == NS_ERROR_MODULE_NETWORK) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p detected networking error 0x%" PRIx32 "\n", this,
static_cast<uint32_t>(status)));
IgnoredErrorResult rv;
mFlagParseBody = false;
RequestErrorSteps(ProgressEventType::error, NS_ERROR_DOM_NETWORK_ERR, rv);
// RequestErrorSteps will not call ChangeStateToDone for sync XHRs, so we
// do so here to ensure progress events are sent and our state is sane.
if (mFlagSynchronous) {
ChangeStateToDone(wasSync);
}
return NS_OK;
}
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p detected unreachable error 0x%" PRIx32 "\n", this,
static_cast<uint32_t>(status)));
}
// If we're uninitialized at this point, we encountered an error
@@ -2316,9 +2375,7 @@ void XMLHttpRequestMainThread::ChangeStateToDoneInternal() {
mFlagSend = false;
if (mTimeoutTimer) {
mTimeoutTimer->Cancel();
}
CancelTimeoutTimer();
// Per spec, fire the last download progress event, if any,
// before readystatechange=4/done. (Note that 0-sized responses
@@ -2706,6 +2763,7 @@ nsresult XMLHttpRequestMainThread::InitiateFetch(
mChannel = nullptr;
mErrorLoad = ErrorType::eChannelOpen;
mErrorLoadDetail = rv;
// Per spec, we throw on sync errors, but not async.
if (mFlagSynchronous) {
@@ -2897,16 +2955,18 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
// we have internal code relying on the channel being created in open().
if (!mChannel) {
mErrorLoad = ErrorType::eChannelOpen;
mErrorLoadDetail = NS_ERROR_DOM_NETWORK_ERR;
mFlagSend = true; // so CloseRequestWithError sets us to DONE.
aRv = MaybeSilentSendFailure(NS_ERROR_DOM_NETWORK_ERR);
aRv = MaybeSilentSendFailure(mErrorLoadDetail);
return;
}
// non-GET requests aren't allowed for blob.
if (IsBlobURI(mRequestURL) && !mRequestMethod.EqualsLiteral("GET")) {
mErrorLoad = ErrorType::eChannelOpen;
mErrorLoadDetail = NS_ERROR_DOM_NETWORK_ERR;
mFlagSend = true; // so CloseRequestWithError sets us to DONE.
aRv = MaybeSilentSendFailure(NS_ERROR_DOM_NETWORK_ERR);
aRv = MaybeSilentSendFailure(mErrorLoadDetail);
return;
}
@@ -2919,6 +2979,7 @@ void XMLHttpRequestMainThread::SendInternal(const BodyExtractorBase* aBody,
// By default we don't have any upload, so mark upload complete.
mUploadComplete = true;
mErrorLoad = ErrorType::eOK;
mErrorLoadDetail = NS_OK;
mLoadTotal = -1;
nsCOMPtr<nsIInputStream> uploadStream;
nsAutoCString uploadContentType;
@@ -3181,9 +3242,7 @@ void XMLHttpRequestMainThread::StartTimeoutTimer() {
return;
}
if (mTimeoutTimer) {
mTimeoutTimer->Cancel();
}
CancelTimeoutTimer();
if (!mTimeoutMilliseconds) {
return;
@@ -3364,6 +3423,7 @@ nsresult XMLHttpRequestMainThread::OnRedirectVerifyCallback(nsresult result,
}
} else {
mErrorLoad = ErrorType::eRedirect;
mErrorLoadDetail = result;
}
mNewRedirectChannel = nullptr;
@@ -3522,6 +3582,13 @@ void XMLHttpRequestMainThread::HandleTimeoutCallback() {
CloseRequestWithError(ProgressEventType::timeout);
}
void XMLHttpRequestMainThread::CancelTimeoutTimer() {
if (mTimeoutTimer) {
mTimeoutTimer->Cancel();
mTimeoutTimer = nullptr;
}
}
NS_IMETHODIMP
XMLHttpRequestMainThread::Notify(nsITimer* aTimer) {
if (mProgressNotifier == aTimer) {
@@ -3625,6 +3692,7 @@ void XMLHttpRequestMainThread::HandleSyncTimeoutTimer() {
CancelSyncTimeoutTimer();
Abort();
mErrorLoadDetail = NS_ERROR_DOM_TIMEOUT_ERR;
}
void XMLHttpRequestMainThread::CancelSyncTimeoutTimer() {

View File

@@ -333,7 +333,7 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
void Abort() {
IgnoredErrorResult rv;
AbortInternal(rv);
MOZ_ASSERT(!rv.Failed());
MOZ_ASSERT(!rv.Failed() || rv.ErrorCodeIs(NS_ERROR_DOM_ABORT_ERR));
}
virtual void Abort(ErrorResult& aRv) override;
@@ -409,6 +409,8 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
void SetSource(UniquePtr<ProfileChunkedBuffer> aSource);
nsresult ErrorDetail() const { return mErrorLoadDetail; }
virtual uint16_t ErrorCode() const override {
return static_cast<uint16_t>(mErrorLoad);
}
@@ -680,6 +682,7 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
nsCOMPtr<nsITimer> mTimeoutTimer;
void StartTimeoutTimer();
void HandleTimeoutCallback();
void CancelTimeoutTimer();
nsCOMPtr<nsIRunnable> mResumeTimeoutRunnable;
@@ -692,6 +695,7 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
void CancelSyncTimeoutTimer();
ErrorType mErrorLoad;
nsresult mErrorLoadDetail;
bool mErrorParsingXML;
bool mWaitingForOnStopRequest;
bool mProgressTimerIsActive;
@@ -715,9 +719,9 @@ class XMLHttpRequestMainThread final : public XMLHttpRequest,
/**
* Close the XMLHttpRequest's channels.
*/
void CloseRequest();
void CloseRequest(nsresult detail);
void TerminateOngoingFetch();
void TerminateOngoingFetch(nsresult detail);
/**
* Close the XMLHttpRequest's channels and dispatch appropriate progress

View File

@@ -43,6 +43,8 @@
#include "mozilla/UniquePtr.h"
extern mozilla::LazyLogModule gXMLHttpRequestLog;
namespace mozilla::dom {
/**
@@ -117,6 +119,7 @@ class Proxy final : public nsIDOMEventListener {
uint64_t mLastTotal;
uint64_t mLastUploadLoaded;
uint64_t mLastUploadTotal;
nsresult mLastErrorDetailAtLoadend;
bool mIsSyncXHR;
bool mLastLengthComputable;
bool mLastUploadLengthComputable;
@@ -150,6 +153,7 @@ class Proxy final : public nsIDOMEventListener {
mLastTotal(0),
mLastUploadLoaded(0),
mLastUploadTotal(0),
mLastErrorDetailAtLoadend(NS_OK),
mIsSyncXHR(false),
mLastLengthComputable(false),
mLastUploadLengthComputable(false),
@@ -450,6 +454,7 @@ class EventRunnable final : public MainThreadProxyRunnable {
bool mProgressEvent;
bool mLengthComputable;
nsresult mStatusResult;
nsresult mErrorDetail;
// mScopeObj is used in PreDispatch only. We init it in our constructor, and
// reset() in PreDispatch, to ensure that it's not still linked into the
// runtime once we go off-thread.
@@ -471,6 +476,7 @@ class EventRunnable final : public MainThreadProxyRunnable {
mProgressEvent(true),
mLengthComputable(aLengthComputable),
mStatusResult(NS_OK),
mErrorDetail(NS_OK),
mScopeObj(RootingCx(), aScopeObj) {}
EventRunnable(Proxy* aProxy, bool aUploadEvent, const nsString& aType,
@@ -487,6 +493,7 @@ class EventRunnable final : public MainThreadProxyRunnable {
mProgressEvent(false),
mLengthComputable(0),
mStatusResult(NS_OK),
mErrorDetail(NS_OK),
mScopeObj(RootingCx(), aScopeObj) {}
private:
@@ -1047,6 +1054,8 @@ bool EventRunnable::PreDispatch(WorkerPrivate* /* unused */) {
mStatus = xhr->GetStatus(rv);
mStatusResult = rv.StealNSResult();
mErrorDetail = xhr->ErrorDetail();
xhr->GetStatusText(mStatusText, rv);
MOZ_ASSERT(!rv.Failed());
@@ -1068,6 +1077,10 @@ bool EventRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) {
return true;
}
if (mType.EqualsASCII(sEventStrings[STRING_loadend])) {
mProxy->mLastErrorDetailAtLoadend = mErrorDetail;
}
if (mType.EqualsASCII(sEventStrings[STRING_loadstart])) {
if (mUploadEvent) {
mProxy->mSeenUploadLoadStart = true;
@@ -1164,6 +1177,15 @@ bool EventRunnable::WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) {
event->SetTrusted(true);
if (MOZ_LOG_TEST(gXMLHttpRequestLog, LogLevel::Debug)) {
nsAutoString type;
event->GetType(type);
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p firing %s event (%u,%u,%" PRIu64 ",%" PRIu64 ")",
mProxy->mXHR.get(), NS_ConvertUTF16toUTF8(type).get(),
mUploadEvent, mLengthComputable, mLoaded, mTotal));
}
target->DispatchEvent(*event);
return true;
@@ -1588,6 +1610,13 @@ void XMLHttpRequestWorker::DispatchPrematureAbortEvent(
event->SetTrusted(true);
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p firing %s pre-abort event (%u,%u,%" PRIu64 ",%" PRIu64, this,
NS_ConvertUTF16toUTF8(aEventType).get(), aUploadTarget,
aUploadTarget ? mProxy->mLastUploadLengthComputable
: mProxy->mLastLengthComputable,
aUploadTarget ? mProxy->mLastUploadLoaded : mProxy->mLastLoaded,
aUploadTarget ? mProxy->mLastUploadTotal : mProxy->mLastTotal));
aTarget->DispatchEvent(*event);
}
@@ -1688,12 +1717,39 @@ void XMLHttpRequestWorker::SendInternal(const BodyExtractorBase* aBody,
bool succeeded = NS_SUCCEEDED(autoSyncLoop->Run());
mStateData->mFlagSend = false;
// Throw appropriately If a sync XHR failed per spec's RequestErrorSteps
if (isSyncXHR && mProxy) {
nsresult error = mProxy->mLastErrorDetailAtLoadend;
if (error == NS_ERROR_DOM_ABORT_ERR) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Info,
("%p throwing NS_ERROR_DOM_ABORT_ERR", this));
aRv.Throw(error);
return;
}
if (error == NS_ERROR_DOM_TIMEOUT_ERR) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Info,
("%p throwing NS_ERROR_DOM_TIMEOUT_ERR", this));
aRv.Throw(error);
return;
}
if (error == NS_ERROR_DOM_NETWORK_ERR ||
NS_ERROR_GET_MODULE(error) == NS_ERROR_MODULE_NETWORK) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Info,
("%p throwing NS_ERROR_DOM_NETWORK_ERR (0x%" PRIx32 ")", this,
static_cast<uint32_t>(error)));
aRv.Throw(NS_ERROR_DOM_NETWORK_ERR);
return;
}
}
// Don't clobber an existing exception that we may have thrown on aRv
// already... though can there really be one? In any case, it seems to me
// that this autoSyncLoop->Run() can never fail, since the StopSyncLoop call
// for it will come from ProxyCompleteRunnable and that always passes true for
// the second arg.
if (!succeeded && !aRv.Failed()) {
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p SendInternal failed; throwing NS_ERROR_FAILURE", this));
aRv.Throw(NS_ERROR_FAILURE);
}
}
@@ -1705,6 +1761,10 @@ void XMLHttpRequestWorker::Open(const nsACString& aMethod,
ErrorResult& aRv) {
mWorkerPrivate->AssertIsOnWorkerThread();
MOZ_LOG(gXMLHttpRequestLog, LogLevel::Debug,
("%p Open(%s,%s,%d)", this, nsAutoCString(aMethod).get(),
NS_ConvertUTF16toUTF8(aUrl).get(), aAsync));
if (mCanceled) {
aRv.ThrowUncatchableException();
return;

View File

@@ -96,7 +96,24 @@ RequestTracker.prototype = {
}, this.resetAfter);
}
req.send(null);
var gotException;
var expectTimeoutException =
!this.async && inWorker && this.timeLimit > 0 && this.timeLimit < 3000;
try {
req.send(null);
} catch (e) {
gotException = e;
if (expectTimeoutException) {
ok(e.name == "TimeoutError", "Should be a TimeoutError");
}
}
if (gotException && !expectTimeoutException) {
ok(false, `expected no exception, got ${gotException}`);
} else if (!gotException && expectTimeoutException) {
ok(false, "expected timeout exception");
}
},
/**

View File

@@ -1,15 +1,8 @@
[response-body-errors.any.html]
expected:
if (os == "android") and fission: [OK, TIMEOUT]
[Asynchronous XMLHttpRequest should clear response on bad chunk]
expected: FAIL
[response-body-errors.any.worker.html]
expected:
if (os == "android") and fission: [OK, TIMEOUT]
[Synchronous XMLHttpRequest should throw on bad chunk]
expected: FAIL
[Asynchronous XMLHttpRequest should clear response on bad chunk]
expected: FAIL