You need to
before you can comment on or make changes to this bug.
On my system, I have the HTTPCookieStorage cookieAcceptPolicy set to NSHTTPCookieAcceptPolicyOnlyFromMainDocumentDomain (corresponding to the Safari preference "Only from sites I visit"). However, when I load the page http://www.time.com/time/world/article/0,8599,1904577,00.html with WebKit, I get a cookie from ".clearspring.com", which is obviously not within the domain of "time.com".
Steps to reproduce:
1) Launch Safari 4 on Mac OS X 10.5.7.
2) Open Safari preferences
3) Select Accept cookies: Only from sites I visit
4) Click "Show Cookies"
5) Click "Remove All" and "Done"
6) Load the page http://www.time.com/time/world/article/0,8599,1904577,00.html
7) Open preferences again
8) Click "Show Cookies" again
I only see cookies from "time.com"
In addition to cookies from "time.com", I see one cookies from ".clearspring.com"
This bug also occurred with the immediately preceding version of Safari on Mac OS X 10.5.7. Can't remember the exact version #, but it was 3.2.x. The bug also occurs if you use "/Developer/Examples/WebKit/MiniBrowser" to load the page rather than Safari.
Running in the debugger, I set breakpoints at -[NSHTTPCookieStorage setCookie:], -[NSHTTPCookieStorage setCookieAcceptPolicy:], and -[NSHTTPCookieStorage setCookies:forURL:mainDocumentURL:], as well as at setCookies() in "WebKit/WebCore/platform/mac/CookieJar.mm" with git commit f8f4e69a4ceb5909eb64ea91fe1de7108710c552 (corresponding to svn r43960). The methods setCookie: and setCookieAcceptPolicy: never get called. The method setCookies:forURL:mainDocumentURL: does get called frequently, also from setCookies() in WebKit, but when printing the cookies I never see ".clearspring.com". Thus, it's a mystery to me where that cookie is getting set. I guess it must be set somehow without NSHTTPCookieStorage, which may explain why it's not respecting the cookieAcceptPolicy.
Typo in the notes: it should say "The method
setCookies:forURL:mainDocumentURL: does get called frequently, ALWAYS from
setCookies() in WebKit" rather than "The method
setCookies:forURL:mainDocumentURL: does get called frequently, ALSO from
setCookies() in WebKit"
In my testing, it seems that the cookie is set when the page loads the application/x-shockwave-flash URL http://widgets.clearspring.com/o/475db72337b302d4/-/-/-/
The URL request is created in -[WebNetscapePluginView loadStream]. The request by default has HTTPShouldHandleCookies YES. Then in ResourceRequest::doUpdatePlatformRequest() it calls [nsRequest setMainDocumentURL:firstPartyForCookies()], but firstPartyForCookies() returns nil.
May be related to bug 9445.
*** Bug 9445 has been marked as a duplicate of this bug. ***
(In reply to comment #2)
Using <rdar://problem/7338359> to track this now
Created an attachment (id=50280) [details]
Work in progress.
Almost done, actually. I'm still thinking through the ResourceLoader level being the right place where the firstPartyForCookies url is set.
Currently FrameLoader handles this in ::addExtraFieldsToRequest() before handing the request off to SubresourceLoader and MainResourceLoader, which both then pipe down to ResourceLoader.
It's kind of sad that the FrameLoader::addExtraFieldsToRequest() doesn't happen at the ResourceLoader level itself but I don't know that simply moving it is a great idea.
I'll decide something tomorrow and hopefully put it up for review in the morning.
Created an attachment (id=50321) [details]
Don't start a ResourceLoader with a null firstPartyForCookies
After sleeping on it, I think this is the right way to go for now. It's a shame that plugin code doesn't go through FrameLoader, and therefore doesn't automatically get the extra fields added.
Since plug-in code is currently so strewn about and very platform specific, I think reworking it needs to be a task for that domain's expert. I don't think there's any harm in this targeted fix for the hole in our cookie accept policy until that can happen.
(From update of attachment 50321 [details])
Better changelog coming
Created an attachment (id=50322) [details]
(From update of attachment 50322 [details])
This needs a "why" comment. The change log explains it well but the code needs something too.
http/tests/cookies/third-party-cookie-relaxing.html -> failed
failed on the commit-bot when doing the "does trunk actually build and test" check before trying to commit.
I believe that failure is due to this checkin moments ago and that the bots will soon roll red. :(
(In reply to comment #13)
> http/tests/cookies/third-party-cookie-relaxing.html -> failed
> failed on the commit-bot when doing the "does trunk actually build and test"
> check before trying to commit.
> I believe that failure is due to this checkin moments ago and that the bots
> will soon roll red. :(
Looking at the layouttest failures, it's apparently that the various cookie tests don't reset themselves to a consistent state. Amazing we didn't have problems like this with cookie tests in the past. I will try to fix the two tests I've added recently that have gone red because of this.
(In reply to comment #14)
> (In reply to comment #13)
> > http/tests/cookies/third-party-cookie-relaxing.html -> failed
> > failed on the commit-bot when doing the "does trunk actually build and test"
> > check before trying to commit.
> > I believe that failure is due to this checkin moments ago and that the bots
> > will soon roll red. :(
> Looking at the layouttest failures, it's apparently that the various cookie
> tests don't reset themselves to a consistent state. Amazing we didn't have
> problems like this with cookie tests in the past. I will try to fix the two
> tests I've added recently that have gone red because of this.
There were some cookie utility functions that were used by *SOME* cookie tests to clear the cookie state. It doesn't seem to be used consistently.
I'm using it before and after each of these two new tests to keep things in a consistent state.
Created an attachment (id=50341) [details]
Quick review, perhaps?
(From update of attachment 50341 [details])
(In reply to comment #18)
That fix apparently didn't work. I still don't know why not.
Clearly these tests are leaking cookies, causing the failure. I just don't know why the bots are seeing this and I can't reproduce here.
SnowLeopard release bot and Leopard debug bots are clear.
I'm still lost as to why Tiger and Leopard release are still failing these tests.
Verified as fixed with git commit 83853ec7af2cf0f4937a743fe99371b6592017b1 svn r55820.
The original time.com URL is no longer demonstrating the original bug with stock 10.6.2 WebKit, but I found another web site that does reproduce the original bug with 10.6.2 WebKit. Fixed in the trunk.
And for full closure on the failing test issue, we made changes to *really* keep the tests consistent and they seem to have stuck.