- Revision
- 89055
- Author
- [email protected]
- Date
- 2011-06-16 11:54:09 -0700 (Thu, 16 Jun 2011)
Log Message
2011-06-16 Keunsoon Lee <[email protected]>
Reviewed by Martin Robinson.
[SOUP] Abnormal operation if server sends 5xx status code without HTTP body
https://bugs.webkit.org/show_bug.cgi?id=60875
* http/tests/loading/resources/status-code-error-with-response-body.php: Added.
Accepting status code and creating response having body with the received status code
* http/tests/loading/resources/status-code-error-without-response-body.php: Added.
Accepting status code and creating response having no body with the received status code
* http/tests/loading/status-code-error-without-response-body-expected.txt: Added.
* http/tests/loading/status-code-error-without-response-body.html: Added.
Sending XMLHttpRequest and check if return callback is onerror or onload for all 4xx and 5xx status codes.
* platform/chromium/test_expectations.txt: add BUGWK60875
2011-06-16 Keunsoon Lee <[email protected]>
Reviewed by Martin Robinson.
[SOUP] Abnormal operation if server sends 5xx status code without HTTP body
https://bugs.webkit.org/show_bug.cgi?id=60875
Handle status code 4xx and 5xx after receiving HTTP body
Webkit soup port premises that server always sends error page
in case server sends error code, 4xx and 5xx.
But, sometimes there is no HTTP body for error page.
In that case, Webkit does nothing or tries to download the HTTP response
according to received MIME Type even if there is no HTTP body. (abnormal operation)
With this modification,
Browser will show received error page if server sends error page.
Or it will show its own error page from each port if server sends nothing.
Modified algorithm
1) libsoup sends status code 4xx or 5xx (client error or server error)
2) Webkit soup port orders to accumulate HTTP body chunks to libsoup.
3) Webkit soup port ignores gotHeadersCallback, contentSniffedCallback and gotChunkCallback.
4) Webkit soup port checks there is HTTP body or not on sendRequestCallback and;
4-1) if there is HTTP body, calls didReceiveResponse()
4-2) if there is no HTTP body, calls didFail() - prevent the abnormal operation
Test: http/tests/loading/status-code-error-without-response-body.html
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::statusWillBeHandledBySoup):
Add a new condition, SOUP_STATUS_IS_CLIENT_ERROR() and SOUP_STATUS_IS_SERVER_ERROR()
(WebCore::soupErrorAndHaveNotReceivedBody):
Add a new helper function, it checks received body is exist if soup sends error code
(WebCore::soupErrorShouldCauseLoadFailure):
Add a new condition, soupErrorAndHaveNotReceivedBody()
(WebCore::convertSoupErrorToResourceError):
Add a new condition, soupErrorAndHaveNotReceivedBody()
(WebCore::sendRequestCallback):
Check handle->client() again, because didReceiveResponse() can make it zero
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (89054 => 89055)
--- trunk/LayoutTests/ChangeLog 2011-06-16 18:46:44 UTC (rev 89054)
+++ trunk/LayoutTests/ChangeLog 2011-06-16 18:54:09 UTC (rev 89055)
@@ -1,3 +1,19 @@
+2011-06-16 Keunsoon Lee <[email protected]>
+
+ Reviewed by Martin Robinson.
+
+ [SOUP] Abnormal operation if server sends 5xx status code without HTTP body
+ https://bugs.webkit.org/show_bug.cgi?id=60875
+
+ * http/tests/loading/resources/status-code-error-with-response-body.php: Added.
+ Accepting status code and creating response having body with the received status code
+ * http/tests/loading/resources/status-code-error-without-response-body.php: Added.
+ Accepting status code and creating response having no body with the received status code
+ * http/tests/loading/status-code-error-without-response-body-expected.txt: Added.
+ * http/tests/loading/status-code-error-without-response-body.html: Added.
+ Sending XMLHttpRequest and check if return callback is onerror or onload for all 4xx and 5xx status codes.
+ * platform/chromium/test_expectations.txt: add BUGWK60875
+
2011-06-16 Chang Shu <[email protected]>
Reviewed by Sam Weinig.
Added: trunk/LayoutTests/http/tests/loading/resources/status-code-error-with-response-body.php (0 => 89055)
--- trunk/LayoutTests/http/tests/loading/resources/status-code-error-with-response-body.php (rev 0)
+++ trunk/LayoutTests/http/tests/loading/resources/status-code-error-with-response-body.php 2011-06-16 18:54:09 UTC (rev 89055)
@@ -0,0 +1,8 @@
+<?php
+header("Content-Type: text/html", TRUE, (int)$_REQUEST['statusCode']);
+?>
+<html>
+<body>
+<p>status code is <?php echo $_REQUEST['statusCode'];?></p>
+</body>
+</html>
Added: trunk/LayoutTests/http/tests/loading/resources/status-code-error-without-response-body.php (0 => 89055)
--- trunk/LayoutTests/http/tests/loading/resources/status-code-error-without-response-body.php (rev 0)
+++ trunk/LayoutTests/http/tests/loading/resources/status-code-error-without-response-body.php 2011-06-16 18:54:09 UTC (rev 89055)
@@ -0,0 +1,3 @@
+<?php
+header("Content-Length: 0", TRUE, (int)$_REQUEST['statusCode']);
+?>
Added: trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body-expected.txt (0 => 89055)
--- trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body-expected.txt 2011-06-16 18:54:09 UTC (rev 89055)
@@ -0,0 +1,17 @@
+main frame - didStartProvisionalLoadForFrame
+main frame - didCommitLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+Test for https://bugs.webkit.org/show_bug.cgi?id=60875
+This test verifies that FrameLoader properly handles 4xx and 5xx errors without response bodies.
+If this test succeeds, you should see a series of "PASS" messages and then "DONE".
+
+** Test for normal; receiving response body
+PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS
+
+** Test for abnormal; not receiving response body
+PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS PASS
+
+DONE
+
Added: trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body.html (0 => 89055)
--- trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body.html (rev 0)
+++ trunk/LayoutTests/http/tests/loading/status-code-error-without-response-body.html 2011-06-16 18:54:09 UTC (rev 89055)
@@ -0,0 +1,87 @@
+<html>
+<body>
+Test for <a href="" <br>
+This test verifies that FrameLoader properly handles 4xx and 5xx errors without response bodies.<br>
+If this test succeeds, you should see a series of "PASS" messages and then "DONE".<br><br>
+
+<pre id="result">
+</pre>
+
+<script>
+function log(message)
+{
+ document.getElementById("result").innerHTML += message;
+}
+
+function done()
+{
+ log("\n\nDONE\n");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+function runTest(url, expectSuccess)
+{
+ req = new XMLHttpRequest();
+ req._onload_ = function() {
+ log((expectSuccess ? "PASS " : "FAIL:" + req.status + " "));
+ nextTest();
+ }
+ req._onerror_ = function() {
+ log((expectSuccess ? "FAIL " : "PASS "));
+ nextTest();
+ }
+
+ try {
+ req.open("GET", url, true);
+ req.send(null);
+ } catch(e) {
+ log("Exception: " + e.message + "\n");
+ }
+}
+
+var statusCodes = [400, 401, 402, 403, 404, 405, 406, 407, 408, 409, 410, 411, 412, 413, 414, 415, 416, 417, 500, 501, 502, 503, 504, 505]
+var tests = [
+ ["resources/status-code-error-with-response-body.php?statusCode=", true],
+ ["resources/status-code-error-without-response-body.php?statusCode=", false]
+]
+var currentStatusCode = 0;
+var currentTest = 0;
+
+function nextTest()
+{
+ if (currentStatusCode == 0) {
+ if (currentTest == 0)
+ log("** Test for normal; receiving response body\n");
+ else if (currentTest == 1)
+ log("\n\n** Test for abnormal; not receiving response body\n");
+ }
+
+ if (currentTest >= tests.length) {
+ done();
+ return;
+ }
+
+ if (currentStatusCode < statusCodes.length) {
+ runTest(tests[currentTest][0] + statusCodes[currentStatusCode++], tests[currentTest][1]);
+ return;
+ }
+
+ currentStatusCode = 0;
+ currentTest++;
+ nextTest();
+}
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+
+ nextTest();
+} else {
+ log("This test needs to be run in DumpRenderTree.\n");
+}
+
+</script>
+
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/test_expectations.txt (89054 => 89055)
--- trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-06-16 18:46:44 UTC (rev 89054)
+++ trunk/LayoutTests/platform/chromium/test_expectations.txt 2011-06-16 18:54:09 UTC (rev 89055)
@@ -211,6 +211,8 @@
// Unskip after implementing LayoutTestController::setDefersLoading and ::goBack.
BUGWK60877 SKIP : loader/navigation-while-deferring-loads.html = FAIL
+BUGWK60875 SKIP : http/tests/loading/status-code-error-without-response-body.html = FAIL
+
// -----------------------------------------------------------------
// WONTFIX TESTS
// -----------------------------------------------------------------
Modified: trunk/Source/WebCore/ChangeLog (89054 => 89055)
--- trunk/Source/WebCore/ChangeLog 2011-06-16 18:46:44 UTC (rev 89054)
+++ trunk/Source/WebCore/ChangeLog 2011-06-16 18:54:09 UTC (rev 89055)
@@ -1,3 +1,44 @@
+2011-06-16 Keunsoon Lee <[email protected]>
+
+ Reviewed by Martin Robinson.
+
+ [SOUP] Abnormal operation if server sends 5xx status code without HTTP body
+ https://bugs.webkit.org/show_bug.cgi?id=60875
+
+ Handle status code 4xx and 5xx after receiving HTTP body
+
+ Webkit soup port premises that server always sends error page
+ in case server sends error code, 4xx and 5xx.
+ But, sometimes there is no HTTP body for error page.
+ In that case, Webkit does nothing or tries to download the HTTP response
+ according to received MIME Type even if there is no HTTP body. (abnormal operation)
+
+ With this modification,
+ Browser will show received error page if server sends error page.
+ Or it will show its own error page from each port if server sends nothing.
+
+ Modified algorithm
+ 1) libsoup sends status code 4xx or 5xx (client error or server error)
+ 2) Webkit soup port orders to accumulate HTTP body chunks to libsoup.
+ 3) Webkit soup port ignores gotHeadersCallback, contentSniffedCallback and gotChunkCallback.
+ 4) Webkit soup port checks there is HTTP body or not on sendRequestCallback and;
+ 4-1) if there is HTTP body, calls didReceiveResponse()
+ 4-2) if there is no HTTP body, calls didFail() - prevent the abnormal operation
+
+ Test: http/tests/loading/status-code-error-without-response-body.html
+
+ * platform/network/soup/ResourceHandleSoup.cpp:
+ (WebCore::statusWillBeHandledBySoup):
+ Add a new condition, SOUP_STATUS_IS_CLIENT_ERROR() and SOUP_STATUS_IS_SERVER_ERROR()
+ (WebCore::soupErrorAndHaveNotReceivedBody):
+ Add a new helper function, it checks received body is exist if soup sends error code
+ (WebCore::soupErrorShouldCauseLoadFailure):
+ Add a new condition, soupErrorAndHaveNotReceivedBody()
+ (WebCore::convertSoupErrorToResourceError):
+ Add a new condition, soupErrorAndHaveNotReceivedBody()
+ (WebCore::sendRequestCallback):
+ Check handle->client() again, because didReceiveResponse() can make it zero
+
2011-06-16 Vsevolod Vlasov <[email protected]>
Reviewed by Pavel Feldman.
Modified: trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp (89054 => 89055)
--- trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2011-06-16 18:46:44 UTC (rev 89054)
+++ trunk/Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp 2011-06-16 18:54:09 UTC (rev 89055)
@@ -194,6 +194,8 @@
static gboolean statusWillBeHandledBySoup(guint statusCode)
{
if (SOUP_STATUS_IS_TRANSPORT_ERROR(statusCode)
+ || SOUP_STATUS_IS_CLIENT_ERROR(statusCode)
+ || SOUP_STATUS_IS_SERVER_ERROR(statusCode)
|| (SOUP_STATUS_IS_REDIRECTION(statusCode) && (statusCode != SOUP_STATUS_NOT_MODIFIED))
|| (statusCode == SOUP_STATUS_UNAUTHORIZED))
return true;
@@ -403,10 +405,23 @@
handle->deref();
}
+static bool receivedClientOrServerErrorWithoutBody(SoupMessage* message)
+{
+ if ((message && SOUP_STATUS_IS_CLIENT_ERROR(message->status_code))
+ || (message && SOUP_STATUS_IS_SERVER_ERROR(message->status_code))) {
+ if (!message->response_body || !message->response_body->length)
+ return true;
+ }
+
+ return false;
+}
+
static bool soupErrorShouldCauseLoadFailure(GError* error, SoupMessage* message)
{
// Libsoup treats some non-error conditions as errors, including redirects and 304 Not Modified responses.
- return message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code) || error->domain == G_IO_ERROR;
+ return (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code))
+ || receivedClientOrServerErrorWithoutBody(message)
+ || error->domain == G_IO_ERROR;
}
static ResourceError convertSoupErrorToResourceError(GError* error, SoupRequest* request, SoupMessage* message = 0)
@@ -415,7 +430,7 @@
ASSERT(request);
GOwnPtr<char> uri(soup_uri_to_string(soup_request_get_uri(request), FALSE));
- if (message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) {
+ if ((message && SOUP_STATUS_IS_TRANSPORT_ERROR(message->status_code)) || receivedClientOrServerErrorWithoutBody(message)) {
return ResourceError(g_quark_to_string(SOUP_HTTP_ERROR),
static_cast<gint>(message->status_code),
uri.get(),
@@ -467,6 +482,13 @@
d->m_response.updateFromSoupMessage(soupMessage);
client->didReceiveResponse(handle.get(), d->m_response);
+ // handle->client() can be zero on above client->didReceiveResponse() if there is something wrong on received response.
+ // It means cancel from WebCore, so, we need to check it again here.
+ if (d->m_cancelled || !(handle->client())) {
+ cleanupSoupRequestOperation(handle.get());
+ return;
+ }
+
// WebCore might have cancelled the job in the while. We
// must check for response_body->length and not
// response_body->data as libsoup always creates the
