From 9fc7f7776626a2ba8a24bcd612504cc0a7bef616 Mon Sep 17 00:00:00 2001 From: Ronald Brill Date: Sun, 4 Jul 2021 11:15:42 +0200 Subject: [PATCH] improve abort handling --- src/changes/changes.xml | 2 +- .../htmlunit/BrowserVersionFeatures.java | 4 + .../javascript/host/xml/XMLHttpRequest.java | 47 +++-- .../host/xml/XMLHttpRequestLifeCycleTest.java | 164 ++++++++++++++++-- 4 files changed, 189 insertions(+), 28 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index d580440b939..7436cab756d 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -9,7 +9,7 @@ - XMLHttpRequest-abort() sets the state to 0. + XMLHttpRequest-abort() sets the state back to 0 and the webResponse to a network error. core-js: Various fixes for undefined. diff --git a/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java b/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java index 9a76e1d6fa7..3bf709947f2 100644 --- a/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java +++ b/src/main/java/com/gargoylesoftware/htmlunit/BrowserVersionFeatures.java @@ -1671,6 +1671,10 @@ public enum BrowserVersionFeatures { @BrowserFeature(IE) XHR_LENGTH_COMPUTABLE, + /** XMLHttpRequest triggers the load events also if the abort was signaled. */ + @BrowserFeature({FF, FF78}) + XHR_LOAD_ALWAYS_AFTER_DONE, + /** XMLHttpRequest triggers the load start event async. */ @BrowserFeature(IE) XHR_LOAD_START_ASYNC, diff --git a/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequest.java b/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequest.java index b7c66ec85e7..0365610e6f4 100644 --- a/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequest.java +++ b/src/main/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequest.java @@ -19,6 +19,7 @@ import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_FIRE_STATE_OPENED_AGAIN_IN_ASYNC_MODE; import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_HANDLE_SYNC_NETWORK_ERRORS; import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_LENGTH_COMPUTABLE; +import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_LOAD_ALWAYS_AFTER_DONE; import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_LOAD_START_ASYNC; import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_NO_CROSS_ORIGIN_TO_ABOUT; import static com.gargoylesoftware.htmlunit.BrowserVersionFeatures.XHR_OPEN_ALLOW_EMTPY_URL; @@ -54,6 +55,7 @@ import org.apache.http.auth.UsernamePasswordCredentials; import com.gargoylesoftware.htmlunit.AjaxController; +import com.gargoylesoftware.htmlunit.BrowserVersion; import com.gargoylesoftware.htmlunit.FormEncodingType; import com.gargoylesoftware.htmlunit.HttpHeader; import com.gargoylesoftware.htmlunit.HttpMethod; @@ -204,7 +206,10 @@ private void fireJavascriptEvent(final String eventName) { return; } + fireJavascriptEventIgnoreAbort(eventName); + } + private void fireJavascriptEventIgnoreAbort(final String eventName) { if (LOG.isDebugEnabled()) { LOG.debug("Firing javascript XHR event: " + eventName); } @@ -371,13 +376,21 @@ public String getStatusText() { public void abort() { getWindow().getWebWindow().getJobManager().stopJob(jobID_); - webResponse_ = null; - setState(DONE); - fireJavascriptEvent(Event.TYPE_READY_STATE_CHANGE); - fireJavascriptEvent(Event.TYPE_ABORT); - fireJavascriptEvent(Event.TYPE_LOAD_END); + if (state_ == OPENED + || state_ == HEADERS_RECEIVED + || state_ == LOADING) { + setState(DONE); + webResponse_ = new NetworkErrorWebResponse(webRequest_, null); + fireJavascriptEvent(Event.TYPE_READY_STATE_CHANGE); + fireJavascriptEvent(Event.TYPE_ABORT); + fireJavascriptEvent(Event.TYPE_LOAD_END); + } + + // ScriptRuntime.constructError("NetworkError", + // "Failed to execute 'send' on 'XMLHttpRequest': Failed to load '" + webRequest_.getUrl() + "'"); setState(UNSENT); + webResponse_ = new NetworkErrorWebResponse(webRequest_, null); aborted_ = true; } @@ -670,7 +683,8 @@ else if (content instanceof Blob) { * @param context the current context */ void doSend() { - if (async_ && getBrowserVersion().hasFeature(XHR_LOAD_START_ASYNC)) { + final BrowserVersion browserVersion = getBrowserVersion(); + if (async_ && browserVersion.hasFeature(XHR_LOAD_START_ASYNC)) { fireJavascriptEvent(Event.TYPE_LOAD_START); } @@ -715,7 +729,7 @@ void doSend() { || statusCode == HttpStatus.SC_NOT_MODIFIED; if (!successful || !isPreflightAuthorized(preflightResponse)) { setState(DONE); - if (async_ || getBrowserVersion().hasFeature(XHR_HANDLE_SYNC_NETWORK_ERRORS)) { + if (async_ || browserVersion.hasFeature(XHR_HANDLE_SYNC_NETWORK_ERRORS)) { fireJavascriptEvent(Event.TYPE_READY_STATE_CHANGE); fireJavascriptEvent(Event.TYPE_ERROR); fireJavascriptEvent(Event.TYPE_LOAD_END); @@ -768,7 +782,7 @@ public String getContentType() { @Override public Charset getContentCharset() { if (charsetNameFinal.isEmpty() - || (charsetFinal == null && getBrowserVersion() + || (charsetFinal == null && browserVersion .hasFeature(XHR_USE_CONTENT_CHARSET))) { return super.getContentCharset(); } @@ -797,8 +811,15 @@ public Charset getContentCharset() { setState(DONE); fireJavascriptEvent(Event.TYPE_READY_STATE_CHANGE); - fireJavascriptEvent(Event.TYPE_LOAD); - fireJavascriptEvent(Event.TYPE_LOAD_END); + + if (browserVersion.hasFeature(XHR_LOAD_ALWAYS_AFTER_DONE)) { + fireJavascriptEventIgnoreAbort(Event.TYPE_LOAD); + fireJavascriptEventIgnoreAbort(Event.TYPE_LOAD_END); + } + else { + fireJavascriptEvent(Event.TYPE_LOAD); + fireJavascriptEvent(Event.TYPE_LOAD_END); + } } catch (final IOException e) { if (LOG.isDebugEnabled()) { @@ -807,7 +828,7 @@ public Charset getContentCharset() { if (async_) { if (e instanceof SocketTimeoutException - && getBrowserVersion().hasFeature(XHR_LOAD_START_ASYNC)) { + && browserVersion.hasFeature(XHR_LOAD_START_ASYNC)) { try { webResponse_ = wc.loadWebResponse(WebRequest.newAboutBlankRequest()); } @@ -820,7 +841,7 @@ && getBrowserVersion().hasFeature(XHR_LOAD_START_ASYNC)) { if (!preflighted && e instanceof NoHttpResponseException - && getBrowserVersion().hasFeature(XHR_PROGRESS_ON_NETWORK_ERROR_ASYNC)) { + && browserVersion.hasFeature(XHR_PROGRESS_ON_NETWORK_ERROR_ASYNC)) { fireJavascriptEvent(Event.TYPE_PROGRESS); } } @@ -839,7 +860,7 @@ && getBrowserVersion().hasFeature(XHR_PROGRESS_ON_NETWORK_ERROR_ASYNC)) { } else { setState(DONE); - if (getBrowserVersion().hasFeature(XHR_HANDLE_SYNC_NETWORK_ERRORS)) { + if (browserVersion.hasFeature(XHR_HANDLE_SYNC_NETWORK_ERRORS)) { fireJavascriptEvent(Event.TYPE_READY_STATE_CHANGE); if (e instanceof SocketTimeoutException) { fireJavascriptEvent(Event.TYPE_TIMEOUT); diff --git a/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequestLifeCycleTest.java b/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequestLifeCycleTest.java index 3a079c4092e..21ad096edd9 100644 --- a/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequestLifeCycleTest.java +++ b/src/test/java/com/gargoylesoftware/htmlunit/javascript/host/xml/XMLHttpRequestLifeCycleTest.java @@ -43,6 +43,7 @@ import com.gargoylesoftware.htmlunit.BrowserRunner; import com.gargoylesoftware.htmlunit.BrowserRunner.Alerts; +import com.gargoylesoftware.htmlunit.BrowserRunner.HtmlUnitNYI; import com.gargoylesoftware.htmlunit.HttpMethod; import com.gargoylesoftware.htmlunit.MiniServer; import com.gargoylesoftware.htmlunit.MockWebConnection; @@ -121,7 +122,7 @@ public boolean isUseOnKeyword() { } private enum Execution { - ONLY_SEND, SEND_ABORT, NETWORK_ERROR, ERROR_403, ERROR_500, TIMEOUT, + ONLY_SEND, SEND_ABORT, DONE_ABORT, NETWORK_ERROR, ERROR_403, ERROR_500, TIMEOUT, ONLY_SEND_PREFLIGHT, ONLY_SEND_PREFLIGHT_FORBIDDEN, WITHOUT_ORIGIN, WITHOUT_ORIGIN_PREFLIGHT, NETWORK_ERROR_PREFLIGHT, @@ -366,6 +367,43 @@ public void addEventListener_sync_preflight_without_origin() throws Exception { verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); } + /** + * @throws Exception if the test fails + */ + @Test + @Alerts({"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "load_4_200_false", "loadend_4_200_false", "send-done: 4_200", "abort-done: 0_0"}) + public void addEventListener_sync_abortTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.SYNC, Execution.SEND_ABORT), URL_FIRST, + servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + + /** + * @throws Exception if the test fails + */ + @Test + @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "ExceptionThrown"}, + FF = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false", "send-done: 0_0"}, + FF78 = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false", "send-done: 0_0"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "load_4_0_false", "abort-done: 4_0", "loadend_4_0_false", "abort-done: 4_0", + "abort-done: 0_0", "send-done: 0_0"}) + @HtmlUnitNYI(CHROME = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}, + EDGE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}) + public void addEventListener_sync_abortAfterDoneTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.SYNC, Execution.DONE_ABORT), URL_FIRST, + servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + /** * @throws Exception if the test fails */ @@ -608,10 +646,10 @@ public void addEventListener_async_preflight_without_origin() throws Exception { */ @Test @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", - "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0", "loadend_4_0_false", - "abort-done: 0_0"}, + "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0_false", + "loadend_4_0_false", "abort-done: 0_0"}, IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", - "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0", + "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0_false", "loadend_4_0_false", "abort-done: 0_0"}) public void addEventListener_async_abortTriggered() throws Exception { final WebDriver driver = loadPage2(buildHtml(Mode.ASYNC, Execution.SEND_ABORT), URL_FIRST, @@ -619,6 +657,37 @@ public void addEventListener_async_abortTriggered() throws Exception { verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); } + /** + * @throws Exception if the test fails + */ + @Test + @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", + "abort-done: 0_0"}, + FF = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false"}, + FF78 = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", + "send-done: 1_0", "loadstart_1_0_false", "readystatechange_2_200_true", + "readystatechange_3_200_true", "progress_3_200_false", "readystatechange_4_200_true", + "load_4_0_false", "abort-done: 4_0", "loadend_4_0_false", "abort-done: 4_0", + "abort-done: 0_0"}) + @HtmlUnitNYI(IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", + "send-done: 1_0", "loadstart_1_0_false", "readystatechange_2_200_true", + "readystatechange_3_200_true", "progress_3_200_false", "readystatechange_4_200_true", + "abort-done: 0_0"}) + public void addEventListener_async_abortAfterDoneTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.ASYNC, Execution.DONE_ABORT), URL_FIRST, + servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + /** * @throws Exception if the test fails */ @@ -805,6 +874,44 @@ public void onKeyWord_sync_preflight_without_origin() throws Exception { verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); } + + /** + * @throws Exception if the test fails + */ + @Test + @Alerts({"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "load_4_200_false", "loadend_4_200_false", "send-done: 4_200", "abort-done: 0_0"}) + public void onKeyWord_sync_abortTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.SYNC_ON_KEYWORD, Execution.SEND_ABORT), + URL_FIRST, servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + + /** + * @throws Exception if the test fails + */ + @Test + @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "ExceptionThrown"}, + FF = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false", "send-done: 0_0"}, + FF78 = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "load_0_0_false", "loadend_0_0_false", "send-done: 0_0"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "load_4_0_false", "abort-done: 4_0", "loadend_4_0_false", "abort-done: 4_0", + "abort-done: 0_0", "send-done: 0_0"}) + @HtmlUnitNYI(CHROME = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}, + EDGE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_4_200_true", + "abort-done: 0_0", "send-done: 0_0"}) + public void onKeyWord_sync_abortAfterDoneTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.SYNC_ON_KEYWORD, Execution.DONE_ABORT), + URL_FIRST, servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + /** * @throws Exception if the test fails */ @@ -971,10 +1078,10 @@ public void onKeyWord_async_preflight() throws Exception { */ @Test @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", - "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0", + "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0_false", "loadend_4_0_false", "abort-done: 0_0"}, IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", - "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0", + "send-done: 1_0", "readystatechange_4_0_true", "abort_4_0_false", "loadend_4_0_false", "abort-done: 0_0"}) public void onKeyWord_async_abortTriggered() throws Exception { final WebDriver driver = loadPage2(buildHtml(Mode.ASYNC_ON_KEYWORD, Execution.SEND_ABORT), @@ -982,6 +1089,36 @@ public void onKeyWord_async_abortTriggered() throws Exception { verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); } + /** + * @throws Exception if the test fails + */ + @Test + @Alerts(DEFAULT = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", "abort-done: 0_0"}, + FF = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", "abort-done: 0_0", + "load_0_0_false", "loadend_0_0_false"}, + FF78 = {"readystatechange_1_0_true", "open-done: 1_0", "loadstart_1_0_false", + "send-done: 1_0", "readystatechange_2_200_true", "readystatechange_3_200_true", + "progress_3_200_false", "readystatechange_4_200_true", "abort-done: 0_0", + "load_0_0_false", "loadend_0_0_false"}, + IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", + "send-done: 1_0", "loadstart_1_0_false", "readystatechange_2_200_true", + "readystatechange_3_200_true", "progress_3_200_false", "readystatechange_4_200_true", + "load_4_0_false", "abort-done: 4_0", "loadend_4_0_false", "abort-done: 4_0", + "abort-done: 0_0"}) + @HtmlUnitNYI(IE = {"readystatechange_1_0_true", "open-done: 1_0", "readystatechange_1_0_true", + "send-done: 1_0", "loadstart_1_0_false", "readystatechange_2_200_true", + "readystatechange_3_200_true", "progress_3_200_false", "readystatechange_4_200_true", + "abort-done: 0_0"}) + public void onKeyWord_async_abortAfterDoneTriggered() throws Exception { + final WebDriver driver = loadPage2(buildHtml(Mode.ASYNC_ON_KEYWORD, Execution.DONE_ABORT), + URL_FIRST, servlets_); + verifyAlerts(() -> extractLog(driver), String.join("\n", getExpectedAlerts())); + } + /** * @throws Exception if the test fails */ @@ -1837,10 +1974,12 @@ else if (Execution.ONLY_SEND_PREFLIGHT_FORBIDDEN.equals(execution)) { htmlBuilder.append(" function alertEventState(event) {\n"); htmlBuilder.append(" logText(event.type + '_' + xhr.readyState + '_'" + "+ xhr.status + '_' + (event.loaded === undefined));\n"); - htmlBuilder.append(" }\n"); - - htmlBuilder.append(" function alertAbort(event) {\n"); - htmlBuilder.append(" logText(event.type + '_' + xhr.readyState + '_' + xhr.status);\n"); + if (Execution.DONE_ABORT.equals(execution)) { + htmlBuilder.append(" if (xhr.readyState === XMLHttpRequest.DONE) {\n"); + htmlBuilder.append(" xhr.abort();\n"); + htmlBuilder.append(" logText('abort-done: ' + xhr.readyState + '_' + xhr.status);"); + htmlBuilder.append(" }\n"); + } htmlBuilder.append(" }\n"); htmlBuilder.append(" function logText(txt) {\n"); @@ -1857,10 +1996,7 @@ else if (Execution.ONLY_SEND_PREFLIGHT_FORBIDDEN.equals(execution)) { } static void registerEventListener(final StringBuffer buffer, final Mode mode, final State state) { - String function = "alertEventState"; - if (State.ABORT.equals(state)) { - function = "alertAbort"; - } + final String function = "alertEventState"; if (mode.isUseOnKeyword()) { buffer.append(" xhr.on").append(state.getEventName_()).append("=").append(function).append(";\n");