Skip to content

Commit b788227

Browse files
author
Julien Chaffraix
committed
[XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR
https://bugs.webkit.org/show_bug.cgi?id=37781 <rdar://problem/7905150> Reviewed by Alexey Proskuryakov. WebCore: Tests: http/tests/xmlhttprequest/access-control-preflight-credential-async.html http/tests/xmlhttprequest/access-control-preflight-credential-sync.html Rolling the patch in as I could not reproduce Qt results locally. * loader/DocumentThreadableLoader.cpp: (WebCore::DocumentThreadableLoader::DocumentThreadableLoader): Now we remove the credential from the request here to avoid forgetting to do so in the different code path. (WebCore::DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest): Just add the "Origin" header. (WebCore::DocumentThreadableLoader::loadRequest): Check here the the credential have been removed so that we don't leak them. Also tweaked a comment to make it clear that the URL check has issue when credential is involved. LayoutTests: Test that doing a cross-origin request with a preflight check does not raise a NETWORK_ERR exception and does not send the credentials. * http/tests/xmlhttprequest/access-control-preflight-credential-async-expected.txt: Added. * http/tests/xmlhttprequest/access-control-preflight-credential-async.html: Added. * http/tests/xmlhttprequest/access-control-preflight-credential-sync-expected.txt: Added. * http/tests/xmlhttprequest/access-control-preflight-credential-sync.html: Added. * http/tests/xmlhttprequest/resources/basic-auth/access-control-auth-basic.php: Added. * platform/mac-tiger/Skipped: * platform/qt/Skipped: Added those 2 tests to the Skipped lists. Canonical link: https://commits.webkit.org/49679@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@58409 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 68677a8 commit b788227

10 files changed

Lines changed: 166 additions & 11 deletions

‎LayoutTests/ChangeLog‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
2010-04-28 Julien Chaffraix <jchaffraix@webkit.org>
2+
3+
Reviewed by Alexey Proskuryakov.
4+
5+
[XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR
6+
https://bugs.webkit.org/show_bug.cgi?id=37781
7+
<rdar://problem/7905150>
8+
9+
Test that doing a cross-origin request with a preflight check does
10+
not raise a NETWORK_ERR exception and does not send the credentials.
11+
12+
* http/tests/xmlhttprequest/access-control-preflight-credential-async-expected.txt: Added.
13+
* http/tests/xmlhttprequest/access-control-preflight-credential-async.html: Added.
14+
* http/tests/xmlhttprequest/access-control-preflight-credential-sync-expected.txt: Added.
15+
* http/tests/xmlhttprequest/access-control-preflight-credential-sync.html: Added.
16+
* http/tests/xmlhttprequest/resources/basic-auth/access-control-auth-basic.php: Added.
17+
18+
* platform/mac-tiger/Skipped:
19+
* platform/qt/Skipped:
20+
Added those 2 tests to the Skipped lists.
21+
122
2010-04-28 Marcus Bulach <bulach@chromium.org>
223

324
Reviewed by Jeremy Orlow.

‎LayoutTests/http/tests/xmlhttprequest/access-control-preflight-credential-async-expected.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Test case for bug 37781: [XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR
2+
3+
PASSED
4+

‎LayoutTests/http/tests/xmlhttprequest/access-control-preflight-credential-async.html‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<html>
2+
<body>
3+
<p>Test case for bug <a href="https://bugs.webkit.org/show_bug.cgi?id=37781">37781</a>: [XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR</p>
4+
<pre id='console'></pre>
5+
<script type="text/javascript">
6+
function log(message)
7+
{
8+
document.getElementById('console').appendChild(document.createTextNode(message + "\n"));
9+
}
10+
11+
if (window.layoutTestController) {
12+
layoutTestController.dumpAsText();
13+
layoutTestController.waitUntilDone();
14+
}
15+
16+
try {
17+
var xhr = new XMLHttpRequest;
18+
xhr.open("PUT", "http://localhost:8000/xmlhttprequest/resources/basic-auth/access-control-auth-basic.php?uid=fooUser", false, "fooUser", "barPass");
19+
xhr.onerror = function (e) {
20+
log("FAILED: received error");
21+
if (window.layoutTestController)
22+
layoutTestController.notifyDone();
23+
};
24+
xhr.onreadystatechange = function () {
25+
if (xhr.readyState == 4) {
26+
log((xhr.status == 401) ? "PASSED" : "FAILED: credential send!");
27+
if (window.layoutTestController)
28+
layoutTestController.notifyDone();
29+
}
30+
};
31+
xhr.send();
32+
} catch(e) {
33+
log("FAILED: got exception " + e.message);
34+
}
35+
</script>
36+
</body>
37+
</html>

‎LayoutTests/http/tests/xmlhttprequest/access-control-preflight-credential-sync-expected.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Test case for bug 37781: [XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR
2+
3+
PASSED
4+

‎LayoutTests/http/tests/xmlhttprequest/access-control-preflight-credential-sync.html‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<html>
2+
<body>
3+
<p>Test case for bug <a href="https://bugs.webkit.org/show_bug.cgi?id=37781">37781</a>: [XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR</p>
4+
<pre id='console'></pre>
5+
<script type="text/javascript">
6+
function log(message)
7+
{
8+
document.getElementById('console').appendChild(document.createTextNode(message + "\n"));
9+
}
10+
11+
if (window.layoutTestController) {
12+
layoutTestController.dumpAsText();
13+
layoutTestController.waitUntilDone();
14+
}
15+
16+
try {
17+
var xhr = new XMLHttpRequest;
18+
xhr.open("PUT", "http://localhost:8000/xmlhttprequest/resources/basic-auth/access-control-auth-basic.php?uid=fooUser", false, "fooUser", "barPass");
19+
xhr.onerror = function (e) {
20+
log("FAILED: received error");
21+
if (window.layoutTestController)
22+
layoutTestController.notifyDone();
23+
};
24+
xhr.onreadystatechange = function () {
25+
if (xhr.readyState == 4) {
26+
log((xhr.status == 401) ? "PASSED" : "FAILED: credential send!");
27+
if (window.layoutTestController)
28+
layoutTestController.notifyDone();
29+
}
30+
};
31+
xhr.send();
32+
} catch(e) {
33+
log("FAILED: got exception " + e.message);
34+
}
35+
</script>
36+
</body>
37+
</html>

‎LayoutTests/http/tests/xmlhttprequest/resources/basic-auth/access-control-auth-basic.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
header("Access-Control-Allow-Origin: http://127.0.0.1:8000/");
4+
header("Access-Control-Allow-Credentials: true");
5+
header("Access-Control-Allow-Methods: PUT");
6+
7+
if ($_SERVER['REQUEST_METHOD'] != "OPTIONS") {
8+
if (!isset($_SERVER['PHP_AUTH_USER']) || !isset($_REQUEST['uid']) || ($_REQUEST['uid'] != $_SERVER['PHP_AUTH_USER'])) {
9+
header('WWW-Authenticate: Basic realm="WebKit Test Realm/Cross Origin"');
10+
header('HTTP/1.0 401 Unauthorized');
11+
echo 'Authentication canceled';
12+
exit;
13+
} else {
14+
echo "User: {$_SERVER['PHP_AUTH_USER']}, password: {$_SERVER['PHP_AUTH_PW']}.";
15+
}
16+
}
17+
?>

‎LayoutTests/platform/mac-tiger/Skipped‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,8 @@ platform/mac/fonts/color-bitmap.html
141141
# https://bugs.webkit.org/show_bug.cgi?id=38000 - r58107 causes video-play-stall and video-play-stall-seek to fail
142142
http/tests/media/video-play-stall.html
143143
http/tests/media/video-play-stall-seek.html
144+
145+
# https://bugs.webkit.org/show_bug.cgi?id=38265
146+
# LayoutTests/http/tests/xmlhttprequest/access-control-preflight-credential-[a]sync.html fails on Tiger
147+
http/tests/xmlhttprequest/access-control-preflight-credential-async.html
148+
http/tests/xmlhttprequest/access-control-preflight-credential-sync.html

‎LayoutTests/platform/qt/Skipped‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4985,6 +4985,8 @@ http/tests/xmlhttprequest/access-control-basic-post-fail-non-simple-content-type
49854985
http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html
49864986
http/tests/xmlhttprequest/access-control-preflight-async-header-denied.html
49874987
http/tests/xmlhttprequest/access-control-preflight-async-method-denied.html
4988+
http/tests/xmlhttprequest/access-control-preflight-credential-async.html
4989+
http/tests/xmlhttprequest/access-control-preflight-credential-sync.html
49884990
http/tests/xmlhttprequest/access-control-preflight-headers-async.html
49894991
http/tests/xmlhttprequest/access-control-preflight-headers-sync.html
49904992
http/tests/xmlhttprequest/access-control-preflight-sync-header-denied.html

‎WebCore/ChangeLog‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
2010-04-28 Julien Chaffraix <jchaffraix@webkit.org>
2+
3+
Reviewed by Alexey Proskuryakov.
4+
5+
[XHR] Cross-Origin synchronous request with credential raises NETWORK_ERR
6+
https://bugs.webkit.org/show_bug.cgi?id=37781
7+
<rdar://problem/7905150>
8+
9+
Tests: http/tests/xmlhttprequest/access-control-preflight-credential-async.html
10+
http/tests/xmlhttprequest/access-control-preflight-credential-sync.html
11+
12+
Rolling the patch in as I could not reproduce Qt results locally.
13+
14+
* loader/DocumentThreadableLoader.cpp:
15+
(WebCore::DocumentThreadableLoader::DocumentThreadableLoader): Now we remove the
16+
credential from the request here to avoid forgetting to do so in the different code path.
17+
(WebCore::DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest): Just add the
18+
"Origin" header.
19+
(WebCore::DocumentThreadableLoader::loadRequest): Check here the the credential have
20+
been removed so that we don't leak them. Also tweaked a comment to make it clear that
21+
the URL check has issue when credential is involved.
22+
123
2010-04-28 Noam Rosenthal <noam.rosenthal@nokia.com>
224

325
Reviewed by Kenneth Rohde Christiansen.

‎WebCore/loader/DocumentThreadableLoader.cpp‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,19 @@ DocumentThreadableLoader::DocumentThreadableLoader(Document* document, Threadabl
8181

8282
ASSERT(m_options.crossOriginRequestPolicy == UseAccessControl);
8383

84-
if (!m_options.forcePreflight && isSimpleCrossOriginAccessRequest(request.httpMethod(), request.httpHeaderFields()))
85-
makeSimpleCrossOriginAccessRequest(request);
84+
OwnPtr<ResourceRequest> crossOriginRequest(new ResourceRequest(request));
85+
crossOriginRequest->removeCredentials();
86+
crossOriginRequest->setAllowCookies(m_options.allowCredentials);
87+
88+
if (!m_options.forcePreflight && isSimpleCrossOriginAccessRequest(crossOriginRequest->httpMethod(), crossOriginRequest->httpHeaderFields()))
89+
makeSimpleCrossOriginAccessRequest(*crossOriginRequest);
8690
else {
87-
m_actualRequest.set(new ResourceRequest(request));
88-
m_actualRequest->setAllowCookies(m_options.allowCredentials);
91+
m_actualRequest.set(crossOriginRequest.release());
8992

90-
if (CrossOriginPreflightResultCache::shared().canSkipPreflight(document->securityOrigin()->toString(), request.url(), m_options.allowCredentials, request.httpMethod(), request.httpHeaderFields()))
93+
if (CrossOriginPreflightResultCache::shared().canSkipPreflight(document->securityOrigin()->toString(), m_actualRequest->url(), m_options.allowCredentials, m_actualRequest->httpMethod(), m_actualRequest->httpHeaderFields()))
9194
preflightSuccess();
9295
else
93-
makeCrossOriginAccessRequestWithPreflight(request);
96+
makeCrossOriginAccessRequestWithPreflight(*m_actualRequest);
9497
}
9598
}
9699

@@ -106,8 +109,6 @@ void DocumentThreadableLoader::makeSimpleCrossOriginAccessRequest(const Resource
106109

107110
// Make a copy of the passed request so that we can modify some details.
108111
ResourceRequest crossOriginRequest(request);
109-
crossOriginRequest.removeCredentials();
110-
crossOriginRequest.setAllowCookies(m_options.allowCredentials);
111112
crossOriginRequest.setHTTPOrigin(m_document->securityOrigin()->toString());
112113

113114
loadRequest(crossOriginRequest, DoSecurityCheck);
@@ -297,6 +298,11 @@ void DocumentThreadableLoader::preflightFailure()
297298

298299
void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, SecurityCheckPolicy securityCheck)
299300
{
301+
// Any credential should have been removed from the cross-site requests.
302+
const KURL& requestURL = request.url();
303+
ASSERT(m_sameOriginRequest || requestURL.user().isEmpty());
304+
ASSERT(m_sameOriginRequest || requestURL.pass().isEmpty());
305+
300306
if (m_async) {
301307
// Don't sniff content or send load callbacks for the preflight request.
302308
bool sendLoadCallbacks = m_options.sendLoadCallbacks && !m_actualRequest;
@@ -320,15 +326,15 @@ void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, Secur
320326

321327
// No exception for file:/// resources, see <rdar://problem/4962298>.
322328
// Also, if we have an HTTP response, then it wasn't a network error in fact.
323-
if (!error.isNull() && !request.url().isLocalFile() && response.httpStatusCode() <= 0) {
329+
if (!error.isNull() && !requestURL.isLocalFile() && response.httpStatusCode() <= 0) {
324330
m_client->didFail(error);
325331
return;
326332
}
327333

328334
// FIXME: FrameLoader::loadSynchronously() does not tell us whether a redirect happened or not, so we guess by comparing the
329335
// request and response URLs. This isn't a perfect test though, since a server can serve a redirect to the same URL that was
330-
// requested.
331-
if (request.url() != response.url() && !isAllowedRedirect(response.url())) {
336+
// requested. Also comparing the request and response URLs as strings will fail if the requestURL still has its credentials.
337+
if (requestURL != response.url() && !isAllowedRedirect(response.url())) {
332338
m_client->didFailRedirectCheck();
333339
return;
334340
}

0 commit comments

Comments
 (0)