Disallow loads using HTTP 0.9 at the ResourceHandle/NetworkDataTask level
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Nov 2016 16:40:44 +0000 (16:40 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 15 Nov 2016 16:40:44 +0000 (16:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164662
<rdar://problem/29268514>

Source/WebCore:

Reviewed by Reviewed by Alex Christensen and Brady Eidson.

Currently we disallow non-default HTTP 0.9 loads at the ResourceLoader level and disallow
subresource loads using HTTP 0.9 on a default port when the embedding page loads using a
different HTTP version. However loads can still be initiated from other loaders (e.g. FrameLoader)
with regards to the first issue. The latter issue does not afford much protection and
increases code complexity. Instead we should simplify our policy and move our code to the
lowest networking abstraction level, ResourceHandle/NetworkDataTask, so that we disallow
all non-default port loads using HTTP 0.9 regardless of the loader used.

Tests: http/tests/security/http-0.9/image-default-port-allowed.html
       http/tests/security/http-0.9/xhr-blocked.html

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::responseReceived): Remove logic to cancel an HTTP 0.9 load from here.
We will cancel the HTTP 0.9 load at the ResourceHandle/NetworkDataTask level.
* loader/ResourceLoader.cpp:
(WebCore::ResourceLoader::didReceiveResponse): Ditto.
* platform/URL.h: Export stringCenterEllipsizedToLength() so that we can use it in WebKit2.
* platform/network/BlobResourceHandle.cpp:
(WebCore::BlobResourceHandle::notifyResponseOnSuccess): Modified to call ResourceHandle::didReceiveResponse().
(WebCore::BlobResourceHandle::notifyResponseOnError): Ditto.
* platform/network/ResourceHandle.cpp:
(WebCore::ResourceHandle::didReceiveResponse): Added. Fail the load if it is using HTTP 0.9.
Otherwise notify the client that we received a response.
(WebCore::ResourceHandle::platformContinueSynchronousDidReceiveResponse): Added. Perform any
additional platform-specific logic after notifying the resource handle client of the received
response. Only the libsoup backend overwrites this member function to do something meaningful.
* platform/network/ResourceHandle.h:
* platform/network/ResourceResponseBase.h:
* platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:
(WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse): Modified to
call ResourceHandle::didReceiveResponse().
* platform/network/mac/WebCoreResourceHandleAsDelegate.mm:
(-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]): Ditto.
* platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
(-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]): Ditto.
* platform/network/soup/ResourceHandleSoup.cpp:
(WebCore::nextMultipartResponsePartCallback): Ditto.
(WebCore::sendRequestCallback): Ditto.
(WebCore::ResourceHandle::platformContinueSynchronousDidReceiveResponse): Added. Turns around and
calls continueAfterDidReceiveResponse().

Source/WebKit2:

Reviewed by Alex Christensen and Brady Eidson.

Make changes to NetworkDataTask similar to the changes made to ResourceHandle so as to
disallow non-default port HTTP 0.9 loads when using the ENABLE(NETWORK_SESSION) networking
code path in WebKit2.

* NetworkProcess/NetworkDataTask.cpp:
(WebKit::NetworkDataTask::didReceiveResponse): Added. Fail the load if it is using HTTP 0.9.
Otherwise notify the client that we received a response.
* NetworkProcess/NetworkDataTask.h:
* NetworkProcess/NetworkDataTaskBlob.cpp:
(WebKit::NetworkDataTaskBlob::resume): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
as the latter has been renamed to the former.
(WebKit::NetworkDataTaskBlob::getSizeForNext): Ditto.
(WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse): Renamed from didReceiveResponse().
* NetworkProcess/NetworkDataTaskBlob.h:
* NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
* NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
(WebKit::NetworkDataTaskCocoa::didReceiveResponse): Deleted.
* NetworkProcess/soup/NetworkDataTaskSoup.cpp:
(WebKit::NetworkDataTaskSoup::didSendRequest): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
as the latter has been renamed to the former.
(WebKit::NetworkDataTaskSoup::dispatchDidReceiveResponse): Renamed from didReceiveResponse(). Also
remove the local variable response and inline its value into the call to ResourceHandle::didReceiveResponse()
as this variable is used exactly once in this function and its name does not describe its purpose any more
than its value.
(WebKit::NetworkDataTaskSoup::didRequestNextPart): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
as the latter has been renamed to the former.
* NetworkProcess/soup/NetworkDataTaskSoup.h:

LayoutTests:

Reviewed by Reviewed by Alex Christensen and Brady Eidson.

Add a test to ensure that we block a synchronous XHR load using HTTP 0.9.
Renamed test image-default-port-blocked.html to image-default-port-allowed.html
as we now allow a subresource load using HTTP 0.9 on a default port regardless
of the HTTP version the embedding page used.

Update test expectations as DRT/WTR do not emit a localized description for the
error associated with a load failure. Note that a message is emitted to
Web Inspector console.

* http/tests/security/http-0.9/iframe-blocked-expected.txt:
* http/tests/security/http-0.9/iframe-blocked.html: Dump frame load callbacks
to see that load was cancelled as there is no other unique visible indication
of success.
* http/tests/security/http-0.9/image-blocked-expected.txt: Update expected result.
* http/tests/security/http-0.9/image-default-port-allowed-expected.txt: Renamed from LayoutTests/http/tests/security/http-0.9/image-default-port-blocked-expected.txt.
* http/tests/security/http-0.9/image-default-port-allowed.html: Renamed from LayoutTests/http/tests/security/http-0.9/image-default-port-blocked.html.
* http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked-expected.txt: Update expected result.
* http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked.html: Ditto.
* http/tests/security/http-0.9/sandbox-should-not-persist-on-navigation-expected.txt: Ditto.
* http/tests/security/http-0.9/worker-connect-src-blocked-expected.txt: Ditto.
* http/tests/security/http-0.9/worker-importScripts-blocked-expected.txt: Ditto.
* http/tests/security/http-0.9/xhr-asynchronous-blocked-expected.txt: Ditto.
* http/tests/security/http-0.9/xhr-blocked-expected.txt: Added.
* http/tests/security/http-0.9/xhr-blocked.html: Added.
* platform/wk2/TestExpectations: Skip the HTTP-0.9 tests in WebKit2 that use internals.registerDefaultPortForProtocol().
The function internals.registerDefaultPortForProtocol only updates the default-port-to-protocol map in the WebContent
process. However network loads in WebKit2 occur in the NetworkProcess. Further investigation is needed to determine
the best way to support testing with default ports. Ideally, we would run an HTTP server on port 80 for testing and
remove the need for internals.registerDefaultPortForProtocol().

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@208732 268f45cc-cd09-0410-ab3c-d52691b4dbfc

37 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/security/http-0.9/iframe-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/iframe-blocked.html
LayoutTests/http/tests/security/http-0.9/image-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/image-default-port-allowed-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/http-0.9/image-default-port-allowed.html [moved from LayoutTests/http/tests/security/http-0.9/image-default-port-blocked.html with 73% similarity]
LayoutTests/http/tests/security/http-0.9/image-default-port-blocked-expected.txt [deleted file]
LayoutTests/http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked.html
LayoutTests/http/tests/security/http-0.9/sandbox-should-not-persist-on-navigation-expected.txt
LayoutTests/http/tests/security/http-0.9/worker-connect-src-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/worker-importScripts-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/xhr-asynchronous-blocked-expected.txt
LayoutTests/http/tests/security/http-0.9/xhr-blocked-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/http-0.9/xhr-blocked.html [new file with mode: 0644]
LayoutTests/platform/wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/loader/DocumentLoader.cpp
Source/WebCore/loader/ResourceLoader.cpp
Source/WebCore/platform/URL.h
Source/WebCore/platform/network/BlobResourceHandle.cpp
Source/WebCore/platform/network/ResourceHandle.cpp
Source/WebCore/platform/network/ResourceHandle.h
Source/WebCore/platform/network/ResourceResponseBase.h
Source/WebCore/platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm
Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkDataTask.cpp
Source/WebKit2/NetworkProcess/NetworkDataTask.h
Source/WebKit2/NetworkProcess/NetworkDataTaskBlob.cpp
Source/WebKit2/NetworkProcess/NetworkDataTaskBlob.h
Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.h
Source/WebKit2/NetworkProcess/cocoa/NetworkDataTaskCocoa.mm
Source/WebKit2/NetworkProcess/soup/NetworkDataTaskSoup.cpp
Source/WebKit2/NetworkProcess/soup/NetworkDataTaskSoup.h

index 5fc608e..8b10bc9 100644 (file)
@@ -1,3 +1,41 @@
+2016-11-15  Daniel Bates  <dabates@apple.com>
+
+        Disallow loads using HTTP 0.9 at the ResourceHandle/NetworkDataTask level
+        https://bugs.webkit.org/show_bug.cgi?id=164662
+        <rdar://problem/29268514>
+
+        Reviewed by Reviewed by Alex Christensen and Brady Eidson.
+
+        Add a test to ensure that we block a synchronous XHR load using HTTP 0.9.
+        Renamed test image-default-port-blocked.html to image-default-port-allowed.html
+        as we now allow a subresource load using HTTP 0.9 on a default port regardless
+        of the HTTP version the embedding page used.
+
+        Update test expectations as DRT/WTR do not emit a localized description for the
+        error associated with a load failure. Note that a message is emitted to
+        Web Inspector console.
+
+        * http/tests/security/http-0.9/iframe-blocked-expected.txt:
+        * http/tests/security/http-0.9/iframe-blocked.html: Dump frame load callbacks
+        to see that load was cancelled as there is no other unique visible indication
+        of success.
+        * http/tests/security/http-0.9/image-blocked-expected.txt: Update expected result.
+        * http/tests/security/http-0.9/image-default-port-allowed-expected.txt: Renamed from LayoutTests/http/tests/security/http-0.9/image-default-port-blocked-expected.txt.
+        * http/tests/security/http-0.9/image-default-port-allowed.html: Renamed from LayoutTests/http/tests/security/http-0.9/image-default-port-blocked.html.
+        * http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked-expected.txt: Update expected result.
+        * http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked.html: Ditto.
+        * http/tests/security/http-0.9/sandbox-should-not-persist-on-navigation-expected.txt: Ditto.
+        * http/tests/security/http-0.9/worker-connect-src-blocked-expected.txt: Ditto.
+        * http/tests/security/http-0.9/worker-importScripts-blocked-expected.txt: Ditto.
+        * http/tests/security/http-0.9/xhr-asynchronous-blocked-expected.txt: Ditto.
+        * http/tests/security/http-0.9/xhr-blocked-expected.txt: Added.
+        * http/tests/security/http-0.9/xhr-blocked.html: Added.
+        * platform/wk2/TestExpectations: Skip the HTTP-0.9 tests in WebKit2 that use internals.registerDefaultPortForProtocol().
+        The function internals.registerDefaultPortForProtocol only updates the default-port-to-protocol map in the WebContent
+        process. However network loads in WebKit2 occur in the NetworkProcess. Further investigation is needed to determine
+        the best way to support testing with default ports. Ideally, we would run an HTTP server on port 80 for testing and
+        remove the need for internals.registerDefaultPortForProtocol().
+
 2016-11-15  Zalan Bujtas  <zalan@apple.com>
 
         [MultiCol] Render tree should be all clean by the end of FrameView::layout().
index 04cc33c..b6e4b9a 100644 (file)
@@ -1,4 +1,8 @@
-CONSOLE MESSAGE: Stopped document load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-fail.pl' because it is using HTTP/0.9 on a non-default port.
+frame "<!--framePath //<!--frame0-->-->" - didStartProvisionalLoadForFrame
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+frame "<!--framePath //<!--frame0-->-->" - didFailProvisionalLoadWithError
+main frame - didFinishLoadForFrame
 
 
 --------
index b7df2fb..e45b9f0 100644 (file)
@@ -5,6 +5,7 @@
 if (window.testRunner) {
     testRunner.dumpAsText();
     testRunner.dumpChildFramesAsText();
+    testRunner.dumpFrameLoadCallbacks();
 }
 </script>
 </head>
index 971f8ae..9c70321 100644 (file)
@@ -1,3 +1,2 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-image.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
 ALERT: PASS
 
diff --git a/LayoutTests/http/tests/security/http-0.9/image-default-port-allowed-expected.txt b/LayoutTests/http/tests/security/http-0.9/image-default-port-allowed-expected.txt
new file mode 100644 (file)
index 0000000..9c70321
--- /dev/null
@@ -0,0 +1,2 @@
+ALERT: PASS
+
@@ -10,6 +10,6 @@ if (window.testRunner) {
 </script>
 </head>
 <body>
-<img src="resources/nph-image.pl" onload="alert('FAIL')" onerror="alert('PASS')">
+<img src="resources/nph-image.pl" onload="alert('PASS')" onerror="alert('FAIL')">
 </body>
 </html>
diff --git a/LayoutTests/http/tests/security/http-0.9/image-default-port-blocked-expected.txt b/LayoutTests/http/tests/security/http-0.9/image-default-port-blocked-expected.txt
deleted file mode 100644 (file)
index 971f8ae..0000000
+++ /dev/null
@@ -1,3 +0,0 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-image.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
-ALERT: PASS
-
index 242a691..08b9542 100644 (file)
@@ -1,5 +1,10 @@
+http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl - willSendRequest <NSURLRequest URL http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl, main document URL http://127.0.0.1:8000/security/http-0.9/image-on-HTTP-0.9-page-blocked.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/security/http-0.9/image-on-HTTP-0.9-page-blocked.html - didFinishLoading
+http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl - didReceiveResponse <NSURLResponse http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl, http status code 200>
 CONSOLE MESSAGE: Sandboxing 'http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl' because it is using HTTP/0.9.
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8080/security/http-0.9/resources/nph-image.pl' because it is using HTTP/0.9 on a non-default port.
+http://127.0.0.1:8080/security/http-0.9/resources/nph-image.pl - willSendRequest <NSURLRequest URL http://127.0.0.1:8080/security/http-0.9/resources/nph-image.pl, main document URL http://127.0.0.1:8000/security/http-0.9/image-on-HTTP-0.9-page-blocked.html, http method GET> redirectResponse (null)
+http://127.0.0.1:8000/security/http-0.9/resources/nph-image-on-HTTP-0.9-page-blocked.pl - didFinishLoading
+http://127.0.0.1:8080/security/http-0.9/resources/nph-image.pl - didFailLoadingWithError: <NSError domain , code 0, failing URL "http://127.0.0.1:8080/security/http-0.9/resources/nph-image.pl">
 
 
 --------
index 023fa4d..4b9cc06 100644 (file)
@@ -5,6 +5,7 @@
 if (window.testRunner) {
     testRunner.dumpAsText();
     testRunner.dumpChildFramesAsText();
+    testRunner.dumpResourceLoadCallbacks();
     if (window.internals)
         internals.registerDefaultPortForProtocol(8000, "http");
 }
index a39002f..7ef22e9 100644 (file)
@@ -1,2 +1 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-image.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
 PASS
index de33604..9c70321 100644 (file)
@@ -1,3 +1,2 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-worker-fail.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
 ALERT: PASS
 
index de33604..9c70321 100644 (file)
@@ -1,3 +1,2 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-worker-fail.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
 ALERT: PASS
 
index 9f4006a..9c70321 100644 (file)
@@ -1,3 +1,2 @@
-CONSOLE MESSAGE: Cancelled resource load from 'http://127.0.0.1:8000/security/http-0.9/resources/nph-fail.pl' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.
 ALERT: PASS
 
diff --git a/LayoutTests/http/tests/security/http-0.9/xhr-blocked-expected.txt b/LayoutTests/http/tests/security/http-0.9/xhr-blocked-expected.txt
new file mode 100644 (file)
index 0000000..9c70321
--- /dev/null
@@ -0,0 +1,2 @@
+ALERT: PASS
+
diff --git a/LayoutTests/http/tests/security/http-0.9/xhr-blocked.html b/LayoutTests/http/tests/security/http-0.9/xhr-blocked.html
new file mode 100644 (file)
index 0000000..681a5a0
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<script>
+const isAsynchronous = false;
+var xhr = new XMLHttpRequest;
+try {
+    xhr.open("GET", "resources/nph-fail.pl", isAsynchronous);
+    xhr.send();
+
+    alert("FAIL");
+} catch (e) {
+    // FIXME: Assert that a specific exception is thrown.
+    alert("PASS");
+}
+</script>
+</body>
+</html>
index c4a95fb..ce088f7 100644 (file)
@@ -548,6 +548,15 @@ fast/preloader/document-write-2.html [ Pass Failure ]
 ########################################
 ### START OF (4) Features that are not supported in WebKit2 and likely never will be
 
+# Internals.registerDefaultPortForProtocol() does not affect NetworkProcess. We should
+# look to remove it and write these test to make use of an HTTP server running on port 80.
+http/tests/security/http-0.9/default-port-plugin-blocked.html
+http/tests/security/http-0.9/default-port-script-blocked.html
+http/tests/security/http-0.9/image-default-port-allowed.html
+http/tests/security/http-0.9/image-on-HTTP-0.9-default-port-page-allowed-ref-test.html
+http/tests/security/http-0.9/image-on-HTTP-0.9-default-port-page-allowed.html
+http/tests/security/http-0.9/image-on-HTTP-0.9-page-blocked.html
+
 # WebKitTestRunner doesn't have appleScriptController
 platform/mac/fast/AppleScript/001.html
 platform/mac/fast/AppleScript/array.html
index fe5b153..201c25d 100644 (file)
@@ -1,3 +1,52 @@
+2016-11-15  Daniel Bates  <dabates@apple.com>
+
+        Disallow loads using HTTP 0.9 at the ResourceHandle/NetworkDataTask level
+        https://bugs.webkit.org/show_bug.cgi?id=164662
+        <rdar://problem/29268514>
+
+        Reviewed by Reviewed by Alex Christensen and Brady Eidson.
+
+        Currently we disallow non-default HTTP 0.9 loads at the ResourceLoader level and disallow
+        subresource loads using HTTP 0.9 on a default port when the embedding page loads using a
+        different HTTP version. However loads can still be initiated from other loaders (e.g. FrameLoader)
+        with regards to the first issue. The latter issue does not afford much protection and
+        increases code complexity. Instead we should simplify our policy and move our code to the
+        lowest networking abstraction level, ResourceHandle/NetworkDataTask, so that we disallow
+        all non-default port loads using HTTP 0.9 regardless of the loader used.
+
+        Tests: http/tests/security/http-0.9/image-default-port-allowed.html
+               http/tests/security/http-0.9/xhr-blocked.html
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::responseReceived): Remove logic to cancel an HTTP 0.9 load from here.
+        We will cancel the HTTP 0.9 load at the ResourceHandle/NetworkDataTask level.
+        * loader/ResourceLoader.cpp:
+        (WebCore::ResourceLoader::didReceiveResponse): Ditto.
+        * platform/URL.h: Export stringCenterEllipsizedToLength() so that we can use it in WebKit2.
+        * platform/network/BlobResourceHandle.cpp:
+        (WebCore::BlobResourceHandle::notifyResponseOnSuccess): Modified to call ResourceHandle::didReceiveResponse().
+        (WebCore::BlobResourceHandle::notifyResponseOnError): Ditto.
+        * platform/network/ResourceHandle.cpp:
+        (WebCore::ResourceHandle::didReceiveResponse): Added. Fail the load if it is using HTTP 0.9.
+        Otherwise notify the client that we received a response.
+        (WebCore::ResourceHandle::platformContinueSynchronousDidReceiveResponse): Added. Perform any
+        additional platform-specific logic after notifying the resource handle client of the received
+        response. Only the libsoup backend overwrites this member function to do something meaningful.
+        * platform/network/ResourceHandle.h:
+        * platform/network/ResourceResponseBase.h:
+        * platform/network/cf/ResourceHandleCFURLConnectionDelegateWithOperationQueue.cpp:
+        (WebCore::ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse): Modified to
+        call ResourceHandle::didReceiveResponse().
+        * platform/network/mac/WebCoreResourceHandleAsDelegate.mm:
+        (-[WebCoreResourceHandleAsDelegate connection:didReceiveResponse:]): Ditto.
+        * platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm:
+        (-[WebCoreResourceHandleAsOperationQueueDelegate connection:didReceiveResponse:]): Ditto.
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::nextMultipartResponsePartCallback): Ditto.
+        (WebCore::sendRequestCallback): Ditto.
+        (WebCore::ResourceHandle::platformContinueSynchronousDidReceiveResponse): Added. Turns around and
+        calls continueAfterDidReceiveResponse().
+
 2016-11-15  Zalan Bujtas  <zalan@apple.com>
 
         [MultiCol] Render tree should be all clean by the end of FrameView::layout().
index 9672e1e..a0d9260 100644 (file)
@@ -731,17 +731,6 @@ void DocumentLoader::responseReceived(const ResourceResponse& response)
     }
 #endif
 
-    if (m_response.isHttpVersion0_9()) {
-        // Non-HTTP responses are interpreted as HTTP/0.9 which may allow exfiltration of data
-        // from non-HTTP services. Therefore cancel if the request was to a non-default port.
-        if (url.port() && !isDefaultPortForProtocol(url.port().value(), url.protocol())) {
-            String message = "Stopped document load from '" + url.string() + "' because it is using HTTP/0.9 on a non-default port.";
-            m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message, identifier);
-            stopLoading();
-            return;
-        }
-    }
-
     frameLoader()->policyChecker().checkContentPolicy(m_response, [this](PolicyAction policy) {
         continueAfterContentPolicy(policy);
     });
index 6a53820..9e2abda 100644 (file)
@@ -437,27 +437,6 @@ void ResourceLoader::didReceiveResponse(const ResourceResponse& r)
 
     m_response = r;
 
-    if (m_response.isHttpVersion0_9()) {
-        auto url = m_response.url();
-        // Non-HTTP responses are interpreted as HTTP/0.9 which may allow exfiltration of data
-        // from non-HTTP services. Therefore cancel if the document was loaded with different
-        // HTTP version or if the resource request was to a non-default port.
-        if (!m_documentLoader->response().isHttpVersion0_9()) {
-            String message = "Cancelled resource load from '" + url.string() + "' because it is using HTTP/0.9 and the document was loaded with a different HTTP version.";
-            m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message, identifier());
-            ResourceError error(emptyString(), 0, url, message);
-            didFail(error);
-            return;
-        }
-        if (url.port() && !isDefaultPortForProtocol(url.port().value(), url.protocol())) {
-            String message = "Cancelled resource load from '" + url.string() + "' because it is using HTTP/0.9 on a non-default port.";
-            m_frame->document()->addConsoleMessage(MessageSource::Security, MessageLevel::Error, message, identifier());
-            ResourceError error(emptyString(), 0, url, message);
-            didFail(error);
-            return;
-        }
-    }
-
     if (FormData* data = m_request.httpBody())
         data->removeGeneratedFilesIfNeeded();
 
index 72c8c1b..d62a207 100644 (file)
@@ -98,7 +98,7 @@ public:
 
     const String& string() const { return m_string; }
 
-    String stringCenterEllipsizedToLength(unsigned length = 1024) const;
+    WEBCORE_EXPORT String stringCenterEllipsizedToLength(unsigned length = 1024) const;
 
     WEBCORE_EXPORT StringView protocol() const;
     WEBCORE_EXPORT String host() const;
index 952e744..e06fa65 100644 (file)
@@ -575,10 +575,7 @@ void BlobResourceHandle::notifyResponseOnSuccess()
     // as if the response had a Content-Disposition header with the filename parameter set to the File's name attribute.
     // Notably, this will affect a name suggested in "File Save As".
 
-    if (usesAsyncCallbacks())
-        client()->didReceiveResponseAsync(this, WTFMove(response));
-    else
-        client()->didReceiveResponse(this, WTFMove(response));
+    didReceiveResponse(WTFMove(response));
 }
 
 void BlobResourceHandle::notifyResponseOnError()
@@ -601,10 +598,7 @@ void BlobResourceHandle::notifyResponseOnError()
         break;
     }
 
-    if (usesAsyncCallbacks())
-        client()->didReceiveResponseAsync(this, WTFMove(response));
-    else
-        client()->didReceiveResponse(this, WTFMove(response));
+    didReceiveResponse(WTFMove(response));
 }
 
 void BlobResourceHandle::notifyReceiveData(const char* data, int bytesRead)
index 60434ca..4773ded 100644 (file)
@@ -150,6 +150,26 @@ void ResourceHandle::clearClient()
     d->m_client = nullptr;
 }
 
+void ResourceHandle::didReceiveResponse(ResourceResponse&& response)
+{
+    if (response.isHttpVersion0_9()) {
+        auto url = response.url();
+        Optional<uint16_t> port = url.port();
+        if (port && !isDefaultPortForProtocol(port.value(), url.protocol())) {
+            cancel();
+            String message = "Cancelled load from '" + url.stringCenterEllipsizedToLength() + "' because it is using HTTP/0.9.";
+            d->m_client->didFail(this, { String(), 0, url, message });
+            return;
+        }
+    }
+    if (d->m_usesAsyncCallbacks)
+        d->m_client->didReceiveResponseAsync(this, WTFMove(response));
+    else {
+        d->m_client->didReceiveResponse(this, WTFMove(response));
+        platformContinueSynchronousDidReceiveResponse();
+    }
+}
+
 #if !PLATFORM(COCOA) && !USE(CFURLCONNECTION) && !USE(SOUP)
 // ResourceHandle never uses async client calls on these platforms yet.
 void ResourceHandle::continueWillSendRequest(ResourceRequest&&)
@@ -170,6 +190,13 @@ void ResourceHandle::continueCanAuthenticateAgainstProtectionSpace(bool)
 #endif
 #endif
 
+#if !USE(SOUP)
+void ResourceHandle::platformContinueSynchronousDidReceiveResponse()
+{
+    // Do nothing.
+}
+#endif
+
 ResourceRequest& ResourceHandle::firstRequest()
 {
     return d->m_firstRequest;
index 3d942d1..0dfbe6d 100644 (file)
@@ -103,6 +103,8 @@ public:
     ResourceRequest willSendRequest(ResourceRequest&&, ResourceResponse&&);
 #endif
 
+    void didReceiveResponse(ResourceResponse&&);
+
     bool shouldUseCredentialStorage();
     void didReceiveAuthenticationChallenge(const AuthenticationChallenge&);
     void receivedCredential(const AuthenticationChallenge&, const Credential&) override;
@@ -249,6 +251,8 @@ private:
 
     void platformSetDefersLoading(bool);
 
+    void platformContinueSynchronousDidReceiveResponse();
+
     void scheduleFailure(FailureType);
 
     bool start();
index d896615..f3d2881 100644 (file)
@@ -92,7 +92,7 @@ public:
 
     WEBCORE_EXPORT const String& httpVersion() const;
     WEBCORE_EXPORT void setHTTPVersion(const String&);
-    bool isHttpVersion0_9() const;
+    WEBCORE_EXPORT bool isHttpVersion0_9() const;
 
     WEBCORE_EXPORT const HTTPHeaderMap& httpHeaderFields() const;
 
index a0c6f86..f7a9b2e 100644 (file)
@@ -161,7 +161,7 @@ void ResourceHandleCFURLConnectionDelegateWithOperationQueue::didReceiveResponse
         UNUSED_PARAM(connection);
 #endif
         
-        m_handle->client()->didReceiveResponseAsync(m_handle, WTFMove(resourceResponse));
+        m_handle->didReceiveResponse(WTFMove(resourceResponse));
     });
     dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
 }
index 885f0c0..1ca310f 100644 (file)
@@ -162,7 +162,7 @@ using namespace WebCore;
     UNUSED_PARAM(connection);
 #endif
 
-    m_handle->client()->didReceiveResponse(m_handle, WTFMove(resourceResponse));
+    m_handle->didReceiveResponse(WTFMove(resourceResponse));
 }
 
 #if USE(NETWORK_CFDATA_ARRAY_CALLBACK)
index 452d9db..002e128 100644 (file)
@@ -199,7 +199,7 @@ using namespace WebCore;
 #else
         UNUSED_PARAM(connection);
 #endif
-        m_handle->client()->didReceiveResponseAsync(m_handle, WTFMove(resourceResponse));
+        m_handle->didReceiveResponse(WTFMove(resourceResponse));
     });
 
     dispatch_semaphore_wait(m_semaphore, DISPATCH_TIME_FOREVER);
index d883428..887a9ed 100644 (file)
@@ -449,12 +449,7 @@ static void nextMultipartResponsePartCallback(GObject* /*source*/, GAsyncResult*
 
     d->m_previousPosition = 0;
 
-    if (handle->client()->usesAsyncCallbacks())
-        handle->client()->didReceiveResponseAsync(handle.get(), ResourceResponse(d->m_response));
-    else {
-        handle->client()->didReceiveResponse(handle.get(), ResourceResponse(d->m_response));
-        continueAfterDidReceiveResponse(handle.get());
-    }
+    handle->didReceiveResponse(ResourceResponse(d->m_response));
 }
 
 static void sendRequestCallback(GObject*, GAsyncResult* result, gpointer data)
@@ -513,12 +508,12 @@ static void sendRequestCallback(GObject*, GAsyncResult* result, gpointer data)
     else
         d->m_inputStream = inputStream;
 
-    if (d->client()->usesAsyncCallbacks())
-        handle->client()->didReceiveResponseAsync(handle.get(), ResourceResponse(d->m_response));
-    else {
-        handle->client()->didReceiveResponse(handle.get(), ResourceResponse(d->m_response));
-        continueAfterDidReceiveResponse(handle.get());
-    }
+    handle->didReceiveResponse(ResourceResponse(d->m_response));
+}
+
+void ResourceHandle::platformContinueSynchronousDidReceiveResponse()
+{
+    continueAfterDidReceiveResponse(this);
 }
 
 static void continueAfterDidReceiveResponse(ResourceHandle* handle)
index 5de1405..f3f6500 100644 (file)
@@ -1,3 +1,39 @@
+2016-11-15  Daniel Bates  <dabates@apple.com>
+
+        Disallow loads using HTTP 0.9 at the ResourceHandle/NetworkDataTask level
+        https://bugs.webkit.org/show_bug.cgi?id=164662
+        <rdar://problem/29268514>
+
+        Reviewed by Alex Christensen and Brady Eidson.
+
+        Make changes to NetworkDataTask similar to the changes made to ResourceHandle so as to
+        disallow non-default port HTTP 0.9 loads when using the ENABLE(NETWORK_SESSION) networking
+        code path in WebKit2.
+
+        * NetworkProcess/NetworkDataTask.cpp:
+        (WebKit::NetworkDataTask::didReceiveResponse): Added. Fail the load if it is using HTTP 0.9.
+        Otherwise notify the client that we received a response.
+        * NetworkProcess/NetworkDataTask.h:
+        * NetworkProcess/NetworkDataTaskBlob.cpp:
+        (WebKit::NetworkDataTaskBlob::resume): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
+        as the latter has been renamed to the former.
+        (WebKit::NetworkDataTaskBlob::getSizeForNext): Ditto.
+        (WebKit::NetworkDataTaskBlob::dispatchDidReceiveResponse): Renamed from didReceiveResponse().
+        * NetworkProcess/NetworkDataTaskBlob.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.h:
+        * NetworkProcess/cocoa/NetworkDataTaskCocoa.mm:
+        (WebKit::NetworkDataTaskCocoa::didReceiveResponse): Deleted.
+        * NetworkProcess/soup/NetworkDataTaskSoup.cpp:
+        (WebKit::NetworkDataTaskSoup::didSendRequest): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
+        as the latter has been renamed to the former.
+        (WebKit::NetworkDataTaskSoup::dispatchDidReceiveResponse): Renamed from didReceiveResponse(). Also
+        remove the local variable response and inline its value into the call to ResourceHandle::didReceiveResponse()
+        as this variable is used exactly once in this function and its name does not describe its purpose any more
+        than its value.
+        (WebKit::NetworkDataTaskSoup::didRequestNextPart): Substitute dispatchDidReceiveResponse() for didReceiveResponse()
+        as the latter has been renamed to the former.
+        * NetworkProcess/soup/NetworkDataTaskSoup.h:
+
 2016-11-14  Alex Christensen  <achristensen@webkit.org>
 
         Move SecurityOrigin::createFromDatabaseIdentifier to SecurityOriginData
index e946f41..3125888 100644 (file)
@@ -31,6 +31,8 @@
 #include "NetworkDataTaskBlob.h"
 #include "NetworkLoadParameters.h"
 #include "NetworkSession.h"
+#include <WebCore/ResourceError.h>
+#include <WebCore/ResourceResponse.h>
 #include <wtf/MainThread.h>
 
 #if PLATFORM(COCOA)
@@ -92,6 +94,21 @@ void NetworkDataTask::scheduleFailure(FailureType type)
     m_failureTimer.startOneShot(0);
 }
 
+void NetworkDataTask::didReceiveResponse(ResourceResponse&& response, ResponseCompletionHandler&& completionHandler)
+{
+    ASSERT(m_client);
+    if (response.isHttpVersion0_9()) {
+        auto url = response.url();
+        Optional<uint16_t> port = url.port();
+        if (port && !isDefaultPortForProtocol(port.value(), url.protocol())) {
+            cancel();
+            m_client->didCompleteWithError({ String(), 0, url, "Cancelled load from '" + url.stringCenterEllipsizedToLength() + "' because it is using HTTP/0.9." });
+            return;
+        }
+    }
+    m_client->didReceiveResponseNetworkSession(WTFMove(response), WTFMove(completionHandler));
+}
+
 void NetworkDataTask::failureTimerFired()
 {
     RefPtr<NetworkDataTask> protectedThis(this);
index 66ecaa9..a57a289 100644 (file)
@@ -82,6 +82,8 @@ public:
     virtual void resume() = 0;
     virtual void invalidateAndCancel() = 0;
 
+    void didReceiveResponse(WebCore::ResourceResponse&&, ResponseCompletionHandler&&);
+
     enum class State {
         Running,
         Suspended,
index 23acb18..6f4bbf5 100644 (file)
@@ -143,7 +143,7 @@ void NetworkDataTaskBlob::resume()
         // Parse the "Range" header we care about.
         String range = m_firstRequest.httpHeaderField(HTTPHeaderName::Range);
         if (!range.isEmpty() && !parseRange(range, m_rangeOffset, m_rangeEnd, m_rangeSuffixLength)) {
-            didReceiveResponse(Error::RangeError);
+            dispatchDidReceiveResponse(Error::RangeError);
             return;
         }
 
@@ -185,7 +185,7 @@ void NetworkDataTaskBlob::getSizeForNext()
     // Do we finish validating and counting size for all items?
     if (m_sizeItemCount >= m_blobData->items().size()) {
         seek();
-        didReceiveResponse();
+        dispatchDidReceiveResponse();
         return;
     }
 
@@ -265,9 +265,9 @@ void NetworkDataTaskBlob::seek()
         m_totalRemainingSize -= m_rangeOffset;
 }
 
-void NetworkDataTaskBlob::didReceiveResponse(Error errorCode)
+void NetworkDataTaskBlob::dispatchDidReceiveResponse(Error errorCode)
 {
-    LOG(NetworkSession, "%p - NetworkDataTaskBlob::didReceiveResponse(%u)", this, static_cast<unsigned>(errorCode));
+    LOG(NetworkSession, "%p - NetworkDataTaskBlob::dispatchDidReceiveResponse(%u)", this, static_cast<unsigned>(errorCode));
 
     Ref<NetworkDataTaskBlob> protectedThis(*this);
     ResourceResponse response(m_firstRequest.url(), errorCode != Error::NoError ? "text/plain" : m_blobData->contentType(), errorCode != Error::NoError ? 0 : m_totalRemainingSize, String());
@@ -301,7 +301,7 @@ void NetworkDataTaskBlob::didReceiveResponse(Error errorCode)
         break;
     }
 
-    m_client->didReceiveResponseNetworkSession(WTFMove(response), [this, protectedThis = WTFMove(protectedThis), errorCode](PolicyAction policyAction) {
+    didReceiveResponse(WTFMove(response), [this, protectedThis = WTFMove(protectedThis), errorCode](PolicyAction policyAction) {
         LOG(NetworkSession, "%p - NetworkDataTaskBlob::didReceiveResponse completionHandler (%u)", this, static_cast<unsigned>(policyAction));
 
         if (m_state == State::Canceling || m_state == State::Completed) {
index 48ce504..5193594 100644 (file)
@@ -83,7 +83,7 @@ private:
 
     void clearStream();
     void getSizeForNext();
-    void didReceiveResponse(Error = Error::NoError);
+    void dispatchDidReceiveResponse(Error = Error::NoError);
     void seek();
     void consumeData(const char* data, int bytesRead);
     void read();
index 3ed8028..cc63727 100644 (file)
@@ -51,7 +51,6 @@ public:
     void didSendData(uint64_t totalBytesSent, uint64_t totalBytesExpectedToSend);
     void didReceiveChallenge(const WebCore::AuthenticationChallenge&, ChallengeCompletionHandler&&);
     void didCompleteWithError(const WebCore::ResourceError&);
-    void didReceiveResponse(WebCore::ResourceResponse&&, ResponseCompletionHandler&&);
     void didReceiveData(Ref<WebCore::SharedBuffer>&&);
 
     void willPerformHTTPRedirection(WebCore::ResourceResponse&&, WebCore::ResourceRequest&&, RedirectCompletionHandler&&);
index a512a2c..4cefa8e 100644 (file)
@@ -155,16 +155,6 @@ void NetworkDataTaskCocoa::didCompleteWithError(const WebCore::ResourceError& er
         m_client->didCompleteWithError(error);
 }
 
-void NetworkDataTaskCocoa::didReceiveResponse(WebCore::ResourceResponse&& response, ResponseCompletionHandler&& completionHandler)
-{
-    if (m_client)
-        m_client->didReceiveResponseNetworkSession(WTFMove(response), WTFMove(completionHandler));
-    else {
-        ASSERT_NOT_REACHED();
-        completionHandler(WebCore::PolicyAction::PolicyIgnore);
-    }
-}
-
 void NetworkDataTaskCocoa::didReceiveData(Ref<WebCore::SharedBuffer>&& data)
 {
     if (m_client)
index 93015a7..3b55c2d 100644 (file)
@@ -345,15 +345,14 @@ void NetworkDataTaskSoup::didSendRequest(GRefPtr<GInputStream>&& inputStream)
         m_inputStream = WTFMove(inputStream);
     }
 
-    didReceiveResponse();
+    dispatchDidReceiveResponse();
 }
 
-void NetworkDataTaskSoup::didReceiveResponse()
+void NetworkDataTaskSoup::dispatchDidReceiveResponse()
 {
     ASSERT(!m_response.isNull());
 
-    auto response = ResourceResponse(m_response);
-    m_client->didReceiveResponseNetworkSession(WTFMove(response), [this, protectedThis = makeRef(*this)](PolicyAction policyAction) {
+    didReceiveResponse(ResourceResponse(m_response), [this, protectedThis = makeRef(*this)](PolicyAction policyAction) {
         if (m_state == State::Canceling || m_state == State::Completed) {
             clearRequest();
             return;
@@ -772,7 +771,7 @@ void NetworkDataTaskSoup::didRequestNextPart(GRefPtr<GInputStream>&& inputStream
     m_response = ResourceResponse();
     m_response.setURL(m_firstRequest.url());
     m_response.updateFromSoupMessageHeaders(soup_multipart_input_stream_get_headers(m_multipartInputStream.get()));
-    didReceiveResponse();
+    dispatchDidReceiveResponse();
 }
 
 void NetworkDataTaskSoup::didFinishRequestNextPart()
index 0bfe595..9a15532 100644 (file)
@@ -62,7 +62,7 @@ private:
     void clearRequest();
     static void sendRequestCallback(SoupRequest*, GAsyncResult*, NetworkDataTaskSoup*);
     void didSendRequest(GRefPtr<GInputStream>&&);
-    void didReceiveResponse();
+    void dispatchDidReceiveResponse();
 
     static void tlsErrorsChangedCallback(SoupMessage*, GParamSpec*, NetworkDataTaskSoup*);
     void tlsErrorsChanged();