Attachment 349201 Details for Bug 189198 – Fix

[patch] Fix

189198.diff (text/plain), 6.87 KB, created by Basuke Suzuki on 2018-09-07 15:12:15 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-07 15:12:15 PDT

Size: 6.87 KB

patch

obsolete

>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 08bc975fdb9..fc48adc3b2c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-09-07  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl][WebKitLegacy] Stop sending credential embedded in the url via XHR.
>+        https://bugs.webkit.org/show_bug.cgi?id=189198
>+
>+        Reviewed by Alexey Proskuryakov.
>+
>+        * http/tests/xmlhttprequest/resources/url-with-credentials/authenticate.php: Added.
>+        * http/tests/xmlhttprequest/resources/url-with-credentials/authorize.php: Added.
>+        * http/tests/xmlhttprequest/url-with-credentials-expected.txt: Added.
>+        * http/tests/xmlhttprequest/url-with-credentials.html: Added.
>+
> 2018-09-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [iOS] Unreviewed test gardening.
>diff --git a/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authenticate.php b/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authenticate.php
>new file mode 100644
>index 00000000000..ef132a541d7
>--- /dev/null
>+++ b/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authenticate.php
>@@ -0,0 +1,4 @@
>+<?php
>+
>+header('Content-Type: text/plain');
>+echo 'User: ' . $_SERVER['PHP_AUTH_USER'], ' Password: ' . $_SERVER['PHP_AUTH_PW'];
>diff --git a/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authorize.php b/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authorize.php
>new file mode 100644
>index 00000000000..b56bd727855
>--- /dev/null
>+++ b/LayoutTests/http/tests/xmlhttprequest/resources/url-with-credentials/authorize.php
>@@ -0,0 +1,8 @@
>+<?php
>+
>+if (!isset($_SERVER['PHP_AUTH_USER'])) {
>+    header('HTTP/1.0 401 Unauthorized');
>+    header('WWW-Authenticate: Basic realm="WebKit Test Area"');
>+} else {
>+    include "authenticate.php";
>+}
>diff --git a/LayoutTests/http/tests/xmlhttprequest/url-with-credentials-expected.txt b/LayoutTests/http/tests/xmlhttprequest/url-with-credentials-expected.txt
>new file mode 100644
>index 00000000000..5bfdc800446
>--- /dev/null
>+++ b/LayoutTests/http/tests/xmlhttprequest/url-with-credentials-expected.txt
>@@ -0,0 +1,12 @@
>+If the request contains credentials in its url, they should be stripped from it. Also first attempt shouldn't contain basic auth header.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+PASS () => xhr.responseText is 'User:  Password: '
>+PASS () => xhr.responseText is 'User: foo Password: bar'
>+PASS () => xhr.responseText is 'User:  Password: '
>+PASS () => xhr.responseText is 'User: foo Password: bar'
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/http/tests/xmlhttprequest/url-with-credentials.html b/LayoutTests/http/tests/xmlhttprequest/url-with-credentials.html
>new file mode 100644
>index 00000000000..1d8d2e91b16
>--- /dev/null
>+++ b/LayoutTests/http/tests/xmlhttprequest/url-with-credentials.html
>@@ -0,0 +1,46 @@
>+<head>
>+    <script src="/js-test-resources/js-test.js"></script>
>+    <script>
>+    function doTest() {
>+        description(`If the request contains credentials in its url, they should be stripped from it.
>+                     Also first attempt shouldn't contain basic auth header.`);
>+        window.jsTestIsAsync = true;
>+
>+        window.xhr = new XMLHttpRequest();
>+        const baseUrl = 'http://foo:bar@127.0.0.1:8000/xmlhttprequest/resources/url-with-credentials/';
>+
>+        /* 1.a [Sync] First trial must be access without credentials. */
>+        xhr.open('GET', baseUrl + 'authenticate.php', false);
>+        xhr.send(null);
>+        shouldBe(() => xhr.responseText, "'User:  Password: '");
>+
>+        /* 1.b [Sync] Send auth info after getting authorization header. */
>+        xhr.open('GET', baseUrl + 'authorize.php', false);
>+        xhr.send(null);
>+        shouldBe(() => xhr.responseText, "'User: foo Password: bar'");
>+
>+        /* 2.a [Async] First trial must be access without credentials. */
>+        xhr.open('GET', baseUrl + 'authenticate.php', true);
>+        xhr.onerror = xhr.onload = () => {
>+            shouldBe(() => xhr.responseText, "'User:  Password: '");
>+
>+            /* 2.b [Async] Send auth info after getting authorization header. */
>+            xhr.open('GET', baseUrl + 'authorize.php', true);
>+            xhr.onerror = xhr.onload = () => {
>+                shouldBe(() => xhr.responseText, "'User: foo Password: bar'");
>+
>+                finishJSTest();
>+            };
>+            xhr.send(null);
>+        };
>+        xhr.send(null);
>+    }
>+    </script>
>+</head>
>+
>+<body onload="doTest()">
>+    <div id="description"></div>
>+    <div id="console"></div>
>+</body>
>+
>+</html>
>\ No newline at end of file
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 71e1bdc3c7c..ac590fbab48 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-09-07  Basuke Suzuki  <Basuke.Suzuki@sony.com>
>+
>+        [Curl][WebKitLegacy] Stop sending credential embedded in the url via XHR.
>+        https://bugs.webkit.org/show_bug.cgi?id=189198
>+
>+        Reviewed by Alexey Proskuryakov.
>+
>+        Because sync XHR of Curl port uses passed request directly, the credential information in
>+        url is not removed. Use ResourceHandleInternal's firstRequest because those are
>+        already takne out.
>+
>+        Test: http/tests/xmlhttprequest/url-with-credentials.html
>+
>+        * platform/network/curl/ResourceHandleCurl.cpp:
>+        (WebCore::ResourceHandle::platformLoadResourceSynchronously):
>+
> 2018-09-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Replace "computed" value with "used" value to match spec language
>diff --git a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>index 55aa9b709ba..c42adc378a6 100644
>--- a/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>+++ b/Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp
>@@ -382,7 +382,6 @@ void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* contex
> {
>     ASSERT(isMainThread());
> 
>-    auto localRequest = request;
>     SynchronousLoaderClient client;
>     bool defersLoading = false;
>     bool shouldContentSniff = true;
>@@ -390,12 +389,13 @@ void ResourceHandle::platformLoadResourceSynchronously(NetworkingContext* contex
>     RefPtr<ResourceHandle> handle = adoptRef(new ResourceHandle(context, request, &client, defersLoading, shouldContentSniff, shouldContentEncodingSniff));
>     handle->d->m_messageQueue = &client.messageQueue();
> 
>-    if (localRequest.url().protocolIsData()) {
>+    if (request.url().protocolIsData()) {
>         handle->handleDataURL();
>         return;
>     }
> 
>-    handle->d->m_curlRequest = handle->createCurlRequest(WTFMove(localRequest));
>+    auto requestCopy = handle->firstRequest();
>+    handle->d->m_curlRequest = handle->createCurlRequest(WTFMove(requestCopy));
>     handle->d->m_curlRequest->start();
> 
>     do {

Actions: View | Formatted Diff | Diff

Attachments on bug 189198: 348675 | 348834 | 349070 | 349083 | 349087 | 349195 | 349201