2009-02-25 David Levin <levin@chromium.org>
Reviewed by Alexey Proskuryakov.
Bug 24089: ThreadableLoader::loadResourceSynchronously should do callbacks like the async code.
<https://bugs.webkit.org/show_bug.cgi?id=24089>
Make threadable loader callbacks to happen during the sync load call.
Changes the behavior of sync xhr for insecure redirects in two ways:
+ Sends an error event instead of an abort event (which is the same as async xhr's behavior).
+ Throws a network exception which is what other browsers do and what the spec
says to do (http://www.w3.org/TR/XMLHttpRequest/).
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::loadResourceSynchronously):
* loader/DocumentThreadableLoader.h:
* loader/ThreadableLoader.cpp:
(WebCore::ThreadableLoader::loadResourceSynchronously):
* loader/ThreadableLoader.h:
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::XMLHttpRequest):
(WebCore::XMLHttpRequest::loadRequestSynchronously):
(WebCore::XMLHttpRequest::loadRequestAsynchronously):
(WebCore::XMLHttpRequest::didFail):
(WebCore::XMLHttpRequest::didFailRedirectCheck):
* xml/XMLHttpRequest.h:
LayoutTests:
2009-02-25 David Levin <levin@chromium.org>
Reviewed by Alexey Proskuryakov.
Bug 24089: ThreadableLoader::loadResourceSynchronously should do callbacks like the async code.
<https://bugs.webkit.org/show_bug.cgi?id=24089>
Modified the test to account for behavior change and output more information to better detect
future changes in behavior.
* http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt:
* http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@41215
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2009-02-25 David Levin <levin@chromium.org>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Bug 24089: ThreadableLoader::loadResourceSynchronously should do callbacks like the async code.
+ <https://bugs.webkit.org/show_bug.cgi?id=24089>
+
+ Modified the test to account for behavior change and output more information to better detect
+ future changes in behavior.
+
+ * http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect-expected.txt:
+ * http/tests/xmlhttprequest/xmlhttprequest-unsafe-redirect.html:
+
2009-02-24 Chris Marrin <cmarrin@apple.com>
Reviewed by David Hyatt.
This tests that unsafe redirects won't be allowed when making an XMLHttpRequest.
+Sync XHR started.
+readyState change 1
+Error event.
+Exception: Error: NETWORK_ERR: XMLHttpRequest Exception 101
+Async XHR started.
+readyState change 1
SUCCESS: Error handler was called with readyState 1
layoutTestController.notifyDone();
}
+function log(message)
+{
+ var logElement = document.getElementById('log');
+ logElement.appendChild(document.createTextNode(message));
+ logElement.appendChild(document.createElement("p"));
+}
+
function onReqreadystatechange()
{
+ log('readyState change ' + req.readyState);
if (req.readyState == 4)
didReadFile();
}
-function onReqError()
+function onReqAbort()
+{
+ log('Abort event.');
+}
+
+function onSyncReqError()
+{
+ log('Error event.');
+}
+
+function onReqError(e)
{
document.getElementById('result').innerHTML = 'SUCCESS: Error handler was called with readyState ' + req.readyState;
if (window.layoutTestController)
}
// First, make a sync request.
+ log ('Sync XHR started.');
req = new XMLHttpRequest();
+ req.onreadystatechange = onReqreadystatechange;
+ req.onerror = onSyncReqError;
+ req.onabort = onReqAbort;
req.open('GET', '/xmlhttprequest/resources/redirect.php?url=http://localhost:8080/xmlhttprequest/resources/forbidden.txt', false);
- req.send(null);
+ try {
+ req.send(null);
+ } catch (e) {
+ log('Exception: ' + (e.description ? e.description : e));
+ }
+
if (req.responseText != '') {
didReadFile();
return;
}
-
+
+ log ('Async XHR started.');
req = new XMLHttpRequest();
req.onreadystatechange = onReqreadystatechange;
req.onerror = onReqError;
+ req.onabort = onReqAbort;
req.open('GET', '/xmlhttprequest/resources/redirect.php?url=http://localhost:8080/xmlhttprequest/resources/forbidden.txt');
req.send(null);
}
</script>
<body onload="runTest()">
<div>This tests that unsafe redirects won't be allowed when making an XMLHttpRequest.</div>
+<div id="log"></div>
<div id="result">FAILURE</div>
</body>
</html>
+2009-02-25 David Levin <levin@chromium.org>
+
+ Reviewed by Alexey Proskuryakov.
+
+ Bug 24089: ThreadableLoader::loadResourceSynchronously should do callbacks like the async code.
+ <https://bugs.webkit.org/show_bug.cgi?id=24089>
+
+ Make threadable loader callbacks to happen during the sync load call.
+
+ Changes the behavior of sync xhr for insecure redirects in two ways:
+ + Sends an error event instead of an abort event (which is the same as async xhr's behavior).
+ + Throws a network exception which is what other browsers do and what the spec
+ says to do (http://www.w3.org/TR/XMLHttpRequest/).
+
+ * loader/DocumentThreadableLoader.cpp:
+ (WebCore::DocumentThreadableLoader::loadResourceSynchronously):
+ * loader/DocumentThreadableLoader.h:
+ * loader/ThreadableLoader.cpp:
+ (WebCore::ThreadableLoader::loadResourceSynchronously):
+ * loader/ThreadableLoader.h:
+ * xml/XMLHttpRequest.cpp:
+ (WebCore::XMLHttpRequest::XMLHttpRequest):
+ (WebCore::XMLHttpRequest::loadRequestSynchronously):
+ (WebCore::XMLHttpRequest::loadRequestAsynchronously):
+ (WebCore::XMLHttpRequest::didFail):
+ (WebCore::XMLHttpRequest::didFailRedirectCheck):
+ * xml/XMLHttpRequest.h:
+
2009-02-24 Chris Marrin <cmarrin@apple.com>
Reviewed by David Hyatt.
#include "AuthenticationChallenge.h"
#include "Document.h"
+#include "DocumentThreadableLoader.h"
+#include "Frame.h"
+#include "FrameLoader.h"
#include "ResourceRequest.h"
#include "SecurityOrigin.h"
#include "SubresourceLoader.h"
namespace WebCore {
+void DocumentThreadableLoader::loadResourceSynchronously(Document* document, const ResourceRequest& request, ThreadableLoaderClient& client)
+{
+ bool sameOriginRequest = document->securityOrigin()->canRequest(request.url());
+
+ Vector<char> data;
+ ResourceError error;
+ ResourceResponse response;
+ unsigned long identifier = std::numeric_limits<unsigned long>::max();
+ if (document->frame())
+ identifier = document->frame()->loader()->loadResourceSynchronously(request, error, response, data);
+
+ // No exception for file:/// resources, see <rdar://problem/4962298>.
+ // Also, if we have an HTTP response, then it wasn't a network error in fact.
+ if (!error.isNull() && !request.url().isLocalFile() && response.httpStatusCode() <= 0) {
+ client.didFail(error);
+ return;
+ }
+
+ // FIXME: This check along with the one in willSendRequest is specific to xhr and
+ // should be made more generic.
+ if (sameOriginRequest && !document->securityOrigin()->canRequest(response.url())) {
+ client.didFailRedirectCheck();
+ return;
+ }
+
+ client.didReceiveResponse(response);
+
+ const char* bytes = static_cast<const char*>(data.data());
+ int len = static_cast<int>(data.size());
+ client.didReceiveData(bytes, len);
+
+ client.didFinishLoading(identifier);
+}
+
PassRefPtr<DocumentThreadableLoader> DocumentThreadableLoader::create(Document* document, ThreadableLoaderClient* client, const ResourceRequest& request, LoadCallbacks callbacksSetting, ContentSniff contentSniff)
{
ASSERT(document);
class DocumentThreadableLoader : public RefCounted<DocumentThreadableLoader>, public ThreadableLoader, private SubresourceLoaderClient {
public:
+ static void loadResourceSynchronously(Document*, const ResourceRequest&, ThreadableLoaderClient&);
static PassRefPtr<DocumentThreadableLoader> create(Document*, ThreadableLoaderClient*, const ResourceRequest&, LoadCallbacks, ContentSniff);
virtual ~DocumentThreadableLoader();
#include "config.h"
#include "ThreadableLoader.h"
-#include "DocumentThreadableLoader.h"
-#include "Document.h"
-#include "Frame.h"
-#include "FrameLoader.h"
#include "ScriptExecutionContext.h"
+#include "Document.h"
+#include "DocumentThreadableLoader.h"
#include "WorkerContext.h"
#include "WorkerRunLoop.h"
#include "WorkerThreadableLoader.h"
return DocumentThreadableLoader::create(static_cast<Document*>(context), client, request, callbacksSetting, contentSniff);
}
-unsigned long ThreadableLoader::loadResourceSynchronously(ScriptExecutionContext* context, const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
+void ThreadableLoader::loadResourceSynchronously(ScriptExecutionContext* context, const ResourceRequest& request, ThreadableLoaderClient& client)
{
ASSERT(context);
ASSERT(context->isDocument());
-
- Document* document = static_cast<Document*>(context);
- if (!document->frame())
- return std::numeric_limits<unsigned long>::max();
- return document->frame()->loader()->loadResourceSynchronously(request, error, response, data);
+ DocumentThreadableLoader::loadResourceSynchronously(static_cast<Document*>(context), request, client);
}
} // namespace WebCore
// just able to run on threads other than the main thread).
class ThreadableLoader : Noncopyable {
public:
+ static void loadResourceSynchronously(ScriptExecutionContext*, const ResourceRequest&, ThreadableLoaderClient&);
static PassRefPtr<ThreadableLoader> create(ScriptExecutionContext*, ThreadableLoaderClient*, const ResourceRequest&, LoadCallbacks, ContentSniff);
- static unsigned long loadResourceSynchronously(ScriptExecutionContext*, const ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data);
virtual void cancel() = 0;
void ref() { refThreadableLoader(); }
, m_inPreflight(false)
, m_receivedLength(0)
, m_lastSendLineNumber(0)
+ , m_exceptionCode(0)
{
initializeXMLHttpRequestStaticData();
}
void XMLHttpRequest::loadRequestSynchronously(ResourceRequest& request, ExceptionCode& ec)
{
ASSERT(!m_async);
- Vector<char> data;
- ResourceError error;
- ResourceResponse response;
- unsigned long identifier = ThreadableLoader::loadResourceSynchronously(scriptExecutionContext(), request, error, response, data);
m_loader = 0;
-
- // No exception for file:/// resources, see <rdar://problem/4962298>.
- // Also, if we have an HTTP response, then it wasn't a network error in fact.
- if (error.isNull() || request.url().isLocalFile() || response.httpStatusCode() > 0) {
- processSyncLoadResults(identifier, data, response, ec);
- return;
- }
-
- if (error.isCancellation()) {
- abortError();
- ec = XMLHttpRequestException::ABORT_ERR;
- return;
- }
-
- networkError();
- ec = XMLHttpRequestException::NETWORK_ERR;
+ m_exceptionCode = 0;
+ ThreadableLoader::loadResourceSynchronously(scriptExecutionContext(), request, *this);
+ if (!m_exceptionCode && m_error)
+ m_exceptionCode = XMLHttpRequestException::NETWORK_ERR;
+ ec = m_exceptionCode;
}
-
void XMLHttpRequest::loadRequestAsynchronously(ResourceRequest& request)
{
ASSERT(m_async);
+ m_exceptionCode = 0;
// SubresourceLoader::create can return null here, for example if we're no longer attached to a page.
// This is true while running onunload handlers.
// FIXME: We need to be able to send XMLHttpRequests from onunload, <http://bugs.webkit.org/show_bug.cgi?id=10904>.
// We need to keep content sniffing enabled for local files due to CFNetwork not providing a MIME type
// for local files otherwise, <rdar://problem/5671813>.
LoadCallbacks callbacks = m_inPreflight ? DoNotSendLoadCallbacks : SendLoadCallbacks;
- ContentSniff contentSniff = request.url().isLocalFile() ? SniffContent : DoNotSniffContent;
+ ContentSniff contentSniff = request.url().isLocalFile() ? SniffContent : DoNotSniffContent;
m_loader = ThreadableLoader::create(scriptExecutionContext(), this, request, callbacks, contentSniff);
if (m_loader) {
return String();
}
-void XMLHttpRequest::processSyncLoadResults(unsigned long identifier, const Vector<char>& data, const ResourceResponse& response, ExceptionCode& ec)
-{
- if (m_sameOriginRequest && !scriptExecutionContext()->securityOrigin()->canRequest(response.url())) {
- abort();
- return;
- }
-
- didReceiveResponse(response);
- changeState(HEADERS_RECEIVED);
-
- const char* bytes = static_cast<const char*>(data.data());
- int len = static_cast<int>(data.size());
- didReceiveData(bytes, len);
-
- didFinishLoading(identifier);
- if (m_error)
- ec = XMLHttpRequestException::NETWORK_ERR;
-}
-
void XMLHttpRequest::didFail(const ResourceError& error)
{
// If we are already in an error state, for instance we called abort(), bail out early.
return;
if (error.isCancellation()) {
+ m_exceptionCode = XMLHttpRequestException::ABORT_ERR;
abortError();
return;
}
+ m_exceptionCode = XMLHttpRequestException::NETWORK_ERR;
networkError();
}
void didReceiveResponsePreflight(const ResourceResponse&);
void didFinishLoadingPreflight();
- void processSyncLoadResults(unsigned long identifier, const Vector<char>& data, const ResourceResponse&, ExceptionCode&);
void updateAndDispatchOnProgress(unsigned int len);
String responseMIMEType() const;
unsigned m_lastSendLineNumber;
String m_lastSendURL;
+ ExceptionCode m_exceptionCode;
};
} // namespace WebCore