[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
beidson at apple.com
beidson at apple.com
Thu Apr 8 02:17:57 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit fdddcab0e2aa66a0eb562c8eb04dafd73fe37231
Author: beidson at apple.com <beidson at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Mar 9 19:52:07 2010 +0000
Plug-ins don't always respect the cookie accept policy.
<rdar://problem/7338359> and https://bugs.webkit.org/show_bug.cgi?id=26391
Reviewed by Darin Adler.
WebCore:
The problem is that the various plug-in implementations call into a ResourceLoader
directly instead of filtering the request through FrameLoader. This bypassed the step
of adding extra fields to the requests, such as the firstPartyForCookies URL.
Since plug-in code is currently so strewn about and very platform specific, I
think reworking it needs to be a task for domain experts. I don't know the implications
of adding *all* the extra fields to plug-in requests, for example.
There's no harm in this targeted fix for the hole in our cookie accept policy until
plug-ins can more fundamentally change.
Test: http/tests/plugins/third-party-cookie-accept-policy.html
* loader/ResourceLoader.cpp:
(WebCore::ResourceLoader::load): Don't load a resource without first giving the request
a firstPartyForCookies URL.
LayoutTests:
* http/tests/cookies/resources/cookie-utility.php:
* http/tests/plugins/resources/third-party-cookie-accept-policy-iframe.html: Added.
* http/tests/plugins/third-party-cookie-accept-policy.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55738 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 080e210..d4e3d86 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2010-03-09 Brady Eidson <beidson at apple.com>
+
+ Reviewed by Darin Adler.
+
+ Plug-ins don't always respect the cookie accept policy.
+ <rdar://problem/7338359> and https://bugs.webkit.org/show_bug.cgi?id=26391
+
+ * http/tests/cookies/resources/cookie-utility.php:
+ * http/tests/plugins/resources/third-party-cookie-accept-policy-iframe.html: Added.
+ * http/tests/plugins/third-party-cookie-accept-policy.html: Added.
+
2010-03-09 Andy Estes <aestes at apple.com>
Reviewed by Adele Peterson.
diff --git a/LayoutTests/http/tests/cookies/resources/cookie-utility.php b/LayoutTests/http/tests/cookies/resources/cookie-utility.php
index f77aa38..a33e7ac 100644
--- a/LayoutTests/http/tests/cookies/resources/cookie-utility.php
+++ b/LayoutTests/http/tests/cookies/resources/cookie-utility.php
@@ -3,7 +3,7 @@ parse_str($_SERVER["QUERY_STRING"]);
function deleteCookie($value, $name)
{
- setcookie($name, "deleted", time() - 86400);
+ setcookie($name, "deleted", time() - 86400, '/');
}
if ($queryfunction == "deleteCookies") {
@@ -13,14 +13,14 @@ if ($queryfunction == "deleteCookies") {
}
if ($queryfunction == "setFooCookie") {
- setcookie("foo", "awesomevalue", time() + 86400);
+ setcookie("foo", "awesomevalue", time() + 86400, '/');
echo "Set the foo cookie";
return;
}
if ($queryfunction == "setFooAndBarCookie") {
- setcookie("foo", "awesomevalue", time() + 86400);
- setcookie("bar", "anotherawesomevalue", time() + 86400);
+ setcookie("foo", "awesomevalue", time() + 86400, '/');
+ setcookie("bar", "anotherawesomevalue", time() + 86400, '/');
echo "Set the foo and bar cookies";
return;
}
diff --git a/LayoutTests/http/tests/plugins/resources/third-party-cookie-accept-policy-iframe.html b/LayoutTests/http/tests/plugins/resources/third-party-cookie-accept-policy-iframe.html
new file mode 100644
index 0000000..c9cb3cf
--- /dev/null
+++ b/LayoutTests/http/tests/plugins/resources/third-party-cookie-accept-policy-iframe.html
@@ -0,0 +1,39 @@
+<html>
+<head>
+<script>
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+</script>
+<body>
+<embed id="plugin" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+var plugin = document.getElementById("plugin");
+
+if (window.layoutTestController)
+ layoutTestController.setAlwaysAcceptCookies(true);
+plugin.getURLNotify("http://localhost:8000/cookies/resources/cookie-utility.php?queryfunction=deleteCookies", null, "trySetCookie");
+
+function trySetCookie()
+{
+ alert("Cookies should be clear, and are: '" + document.cookie + "'");
+ alert("About to set a cookie, but on localhost instead of 127.0.0.1, which is our main document domain - This should fail.");
+ if (window.layoutTestController)
+ layoutTestController.setAlwaysAcceptCookies(false);
+ plugin.getURLNotify("http://localhost:8000/cookies/resources/cookie-utility.php?queryfunction=setFooCookie", null, "completeTest");
+}
+
+function completeTest()
+{
+ alert("Cookies should still be clear, and are: '" + document.cookie + "'");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy-expected.txt b/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy-expected.txt
new file mode 100644
index 0000000..e083f8c
--- /dev/null
+++ b/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy-expected.txt
@@ -0,0 +1,5 @@
+ALERT: Cookies should be clear, and are: ''
+ALERT: About to set a cookie, but on localhost instead of 127.0.0.1, which is our main document domain - This should fail.
+ALERT: Cookies should still be clear, and are: ''
+This tests that plug-ins cannot set cookies in violation of the 3rd party cookie policy.
+
diff --git a/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy.html b/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy.html
new file mode 100644
index 0000000..bab585c
--- /dev/null
+++ b/LayoutTests/http/tests/plugins/third-party-cookie-accept-policy.html
@@ -0,0 +1,16 @@
+<html>
+<head>
+<script>
+
+if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+}
+
+</script>
+<body>
+This tests that plug-ins cannot set cookies in violation of the 3rd party cookie policy.<br>
+<iframe src="http://localhost:8000/plugins/resources/third-party-cookie-accept-policy-iframe.html"></iframe>
+
+</body>
+</html>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 34b79e8..3055b23 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2010-03-09 Brady Eidson <beidson at apple.com>
+
+ Reviewed by Darin Adler.
+
+ Plug-ins don't always respect the cookie accept policy.
+ <rdar://problem/7338359> and https://bugs.webkit.org/show_bug.cgi?id=26391
+
+ The problem is that the various plug-in implementations call into a ResourceLoader
+ directly instead of filtering the request through FrameLoader. This bypassed the step
+ of adding extra fields to the requests, such as the firstPartyForCookies URL.
+
+ Since plug-in code is currently so strewn about and very platform specific, I
+ think reworking it needs to be a task for domain experts. I don't know the implications
+ of adding *all* the extra fields to plug-in requests, for example.
+
+ There's no harm in this targeted fix for the hole in our cookie accept policy until
+ plug-ins can more fundamentally change.
+
+ Test: http/tests/plugins/third-party-cookie-accept-policy.html
+
+ * loader/ResourceLoader.cpp:
+ (WebCore::ResourceLoader::load): Don't load a resource without first giving the request
+ a firstPartyForCookies URL.
+
2010-03-09 Andy Estes <aestes at apple.com>
Reviewed by Adele Peterson.
diff --git a/WebCore/loader/ResourceLoader.cpp b/WebCore/loader/ResourceLoader.cpp
index 69e8633..1cbb843 100644
--- a/WebCore/loader/ResourceLoader.cpp
+++ b/WebCore/loader/ResourceLoader.cpp
@@ -111,6 +111,17 @@ bool ResourceLoader::load(const ResourceRequest& r)
ASSERT(!m_documentLoader->isSubstituteLoadPending(this));
ResourceRequest clientRequest(r);
+
+ // https://bugs.webkit.org/show_bug.cgi?id=26391
+ // The various plug-in implementations call directly to ResourceLoader::load() instead of piping requests
+ // through FrameLoader. As a result, they miss the FrameLoader::addExtraFieldsToRequest() step which sets
+ // up the 1st party for cookies URL. Until plug-in implementations can be reigned in to pipe through that
+ // method, we need to make sure there is always a 1st party for cookies set.
+ if (clientRequest.firstPartyForCookies().isNull()) {
+ if (Document* document = m_frame->document())
+ clientRequest.setFirstPartyForCookies(document->firstPartyForCookies());
+ }
+
willSendRequest(clientRequest, ResourceResponse());
if (clientRequest.isNull()) {
didFail(frameLoader()->cancelledError(r));
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list