1254856 - Some Flash websites (forgeofempires.com, bet365.com) can't finish loading with "Accept third-party cookies: Never" checked

Status: VERIFIED FIXED

(Core :: Networking: Cookies, defect)

Description

[Mr Alexander]

Attachment: loop

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160303134406

Steps to reproduce:

I opened firefox.  Username and password displayed for either:
en.forgeofempires.com
en.enlvenar.com

I press login.  The site offers the selection of worlds to play in.
On selecting my world, both sites reset to the login page

I cannot play


Actual results:

Firefox 44.0.2 was fine logging in but 45.0 has broken it.  It is getting tedious to have ever more updates.
It loops between login page and partially loading, then returning back


Expected results:

I should be able to play the game

Comment 1

[Mr Alexander]

This only happened since the last update.  The games work on ALL other browsers.
Firefox is broken

Comment 2

[Cosmin Muntean [:cmuntean], Ecosystem QA]

Firefox:45.0, Build ID: 20160303134406
User Agent  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0) Gecko/20100101 Firefox/45.0

Hi Alex,

I have tested this issue on the latest Firefox (45.0) release, latest Nightly (48.0a1 - 20160308030418) build and I could not reproduce it. I have created a new account on "en.enlvenar.com" and "en.forgeofempires.com". When I press the "Login" button, the window with selecting my worlds appear. After I choose my world the game starts and I am able to play.
I have also test this on Windows 7 x64 but I could not reproduce it.

Can you please test this on the latest Firefox release or latest Nightly build (https://nightly.mozilla.org/) and report back the results ? When doing this, please use a new clean Firefox profile, maybe even safe mode, to eliminate custom settings as a possible cause (https://goo.gl/PNe90E). 

Thanks,
Cosmin.

Comment 3

[Loic]

I tried with FF45 on Win 7, it works. Did you try to delete the website's cookies?

In addition, could you test with a fresh profile.
https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles

Comment 4

[Mr Alexander]

Attachment: cookies.jpg

Comment 5

[Mr Alexander]

I found it:  I am now forced to add an exception to accept cookies on the sites, see uploaded image.  This is new in 45.0 as it worked without the need to add the exception.

Comment 6

[Loic]

(In reply to Alex Keel from comment #5)
> I found it:  I am now forced to add an exception to accept cookies on the
> sites, see uploaded image.  This is new in 45.0 as it worked without the
> need to add the exception.

Could you provide your custom settings for "History" in about:preferences#privacy

Comment 7

[Loic]

(without the dialog box of exceptions hidding the settings page)

Comment 8

[Mr Alexander]

Attachment: custom.jpg

See my settings

Comment 9

[Loic]

STR:

1) Set "Accept third-party cookies: Never" in about:preferences#privacy
2) Open https://en.elvenar.com/
3) Log in (at the top-right) with bugzilla:azerty123
4) Select the world called "FELYNDRAL"

Result: game fails to load completely http://i.imgur.com/hqgs0G1.jpg

Reg range:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=97419358e5899b3e6ee2e21e7d89106d90b8fa70&tochange=bc44c9e55ef0e042c5a6c88bd241c08282b27f55

Blake Kaplan — Bug 1171215 - Compute third-partyness in the loadinfo instead of nsIHttpChannelInternal so that other protocols correctly respect the third-party cookie pref. r=sicking/ckerschb

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Clearing the needinfo so this gets picked up by e10s triage.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

To be clear, the fix mentioned in comment 9 changed the behavior for both e10s and non-e10s. Looking through logs, the most obvious hypothesis is that I changed the behavior of the 3rd-party cookie check for plugins somehow. That being said, all of the cookies I saw being blocked were actually 3rd-party to the web site (so IMO correctly blocked). I'll debug tomorrow with an old build to see what cookies we used to block.

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

The loadinfo code doesn't try very hard to get a window out of the passed-in context. In this case, we have an object element (the plugin) which advertises nsIFrameLoaderOwner for the cases that it's acting as a document but doesn't have one for other cases.

I think we should do more to get a window context out of the loading context.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d9d22988d7ce

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch v1

This does fix the bug. Bill, it looks like you added the code in question. I want to make sure that this is OK for Web Extensions. I could target the fix more directly (at object or embed elements) but I don't see any reason not to do things this way, the more info we have about the load the better, AFAICT.

Comment 15

[Kohei Yoshino]

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/some-flash-sites-are-broken-when-third-party-cookies-are-blocked/

Comment 16

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8729648 [details] [diff] [review]
Patch v1

Review of attachment 8729648 [details] [diff] [review]:
-----------------------------------------------------------------

Bill probably knows the code better than I do, but it seems totally reasonable to me.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Patch v2

Sicking pointed out that we don't need to do the frameloader dance at all for the 3rd-party checks (and I'd actually forgotten that we already skip to the parent window in ComputeIsThirdPartyContext) so that lets us simplify the logic here a bit.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5fe45d942e3d

Comment 19

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 8729737 [details] [diff] [review]
Patch v2

Review of attachment 8729737 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/LoadInfo.cpp
@@ +86,5 @@
>        if (fl && NS_SUCCEEDED(fl->GetDocShell(getter_AddRefs(docShell))) && docShell) {
>          outerWindow = do_GetInterface(docShell);
>        }
>      } else {
> +      contextOuter.swap(outerWindow);

Nit: I think `outerWindow = contextOuter.forget()` would be slightly more clear and would do the same amount of refcounting. Up to you.

Comment 20

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Blake, with this second patch, don't you need to change ComputeIsThirdPartyContext to stop it from stepping up to the parent if the content type is TYPE_SUBDOCUMENT?

Comment 21

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Also, I'm worried that we're still computing the outer window wrong for WebExtensions in the case of <object>. Can we do a combination of the first patch and the second one?

Comment 22

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Combined patch

Here's a combined patch. I must have attached the patch before rebasing it to include the removal of that code in ComputeIsThirdPartyContext.  Sorry about that.

I played with the idea of asserting that this only happens for <object>s, but I've been so snakebitten every time I've tried asserting that sort of thing in this code that I backed off. If we don't have a loader, we use the enclosing window for everything.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5aef82f8d5b8

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Tracking as it might be part of the 45.0.1 dot release.
Blake, let me know what you think about this fix being part of the dot releases. Thanks

Comment 26

[Loic]

inbox.google.com is affected too.

Comment 27

[Sylvestre Ledru [:Sylvestre]]

inbox uses flash?

Comment 28

[kodtaku]

Additional Info on inbox.google.com:
- Check Accept cookies from sites
- Accept third-party cookies: From visited

The above works.

Comment 29

[Loic]

(In reply to kodtaku from comment #28)
> Additional Info on inbox.google.com:
> - Check Accept cookies from sites
> - Accept third-party cookies: From visited
> 
> The above works.

The interesting case here is "Accept third-party cookies: Never". Does it work in such a case?

Comment 30

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8729808 [details] [diff] [review]
Combined patch

Review of attachment 8729808 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks Blake. I think some tests would really be great here.

Comment 31

[kodtaku]

(In reply to Loic from comment #29)
> (In reply to kodtaku from comment #28)
> > Additional Info on inbox.google.com:
> > - Check Accept cookies from sites
> > - Accept third-party cookies: From visited
> > 
> > The above works.
> 
> The interesting case here is "Accept third-party cookies: Never". Does it
> work in such a case?

You can change it at runtime to get the failure behavoir.  
Fail: Accept third-party cookies: Never
Success: Accept third-party cookies: From visited

Just refresh the page in-between.

Comment 32

[Sylvestre Ledru [:Sylvestre]]

Jonas, Blake, could you fill the uplift request asap? 
I would like to start the build of 45.0.1 & 45.0.1esr asap and we will need this patch. Thanks

Comment 33

[Blake Kaplan (:mrbkap) (inactive)]

This needs to land on central first, right? As soon as the tree reopens I'll land it.

Comment 34

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/47f638b8918d

Comment 35

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8729808 [details] [diff] [review]
Combined patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Plugins will misbehave on some sites requiring login/cookies.
Fix Landed on Version: About to land on Nightly.
Risk to taking this patch (and alternatives if risky): Users can accept cookies from 3rd parties.
String or UUID changes made by this patch: N/A

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/regressing bug #]: bug 1171215
[User impact if declined]: see above
[Describe test coverage new/current, TreeHerder]: I'm working on an automated test for this now. We have tests for other instances of the 3rd-party cookie blocking behavior.
[Risks and why]: We have decent automated testing coverage for non-plugin 3rd party cookie behavior, but obviously are lacking coverage for plugins. That being said, the code changes here should be quite safe.
[String/UUID change made/needed]: n/a

Comment 36

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #35)
> [Risks and why]: We have decent automated testing coverage for non-plugin
> 3rd party cookie behavior, but obviously are lacking coverage for plugins.
> That being said, the code changes here should be quite safe.

What kind of plugins would we need? Just Flash? If that is the case we have firefox-ui-tests which we might be able to use for such a test. Mind giving me some details what would be necessary?

Comment 37

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Henrik Skupin (:whimboo) from comment #36)
> What kind of plugins would we need? Just Flash? If that is the case we have
> firefox-ui-tests which we might be able to use for such a test. Mind giving
> me some details what would be necessary?

I don't know enough about the details of how flash works. The key is to have a flash plugin that doesn't work if we don't send cookies with any requests that the plugin itself makes.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/47f638b8918d

Comment 39

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've reproduced the initial issue on Firefox DevEdition 47.0a2 and on Firefox 45. 

I've tested on Windows 7 64bit and on Mac OSX 10.9.5 using latest Nightly 48.0a1 (buildID: 20160315030230): in about:preferences#privacy I've set "Accept third-party cookies: Never" and the games (https://en.forgeofempires.com/ ; https://en.elvenar.com/) correctly work, i'm able to play. On the "inbox.google.com" I've seen the message "server is temporarily unavailable" and the page keeps refreshing on and on - with "Accept third-party cookies: Never"; when I set "Accept third-party cookies: Always", the "inbox.google.com" page correctly works. Any thoughts about this?

Comment 40

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8729808 [details] [diff] [review]
Combined patch

need for the 45.0.1 dot release.

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/df42969f5a97

Comment 42

[Sylvestre Ledru [:Sylvestre]]

Jonas, seems that the patch doesn't apply to beta or the other branches, could you rebase it for Blake? We need it asap.

Comment 43

[Sylvestre Ledru [:Sylvestre]]

(thanks)

Comment 44

[Ryan VanderMeulen [:RyanVM]]

Nevermind, was a pretty straightforward rebase.

https://hg.mozilla.org/releases/mozilla-beta/rev/f487458614db

Comment 45

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/c728f7d9d0dc

Comment 46

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr45/rev/c728f7d9d0dc

Comment 47

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Camelia Badau, QA [:cbadau] from comment #39)
> ... On the "inbox.google.com" ...

I checked out a version of Firefox from before my fix for bug 1171215 and it took forever to load inbox.google.com (close to two minutes). I observed the same behavior with a version of Firefox with my fix for this bug. Therefore, I think that this is a separate issue and should be filed and tracked as a separate bug.

Comment 48

[Blake Kaplan (:mrbkap) (inactive)]

To be clear, I was trying with hg revision 97419358e5899b3e6ee2e21e7d89106d90b8fa70

Comment 49

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Mochitest

This test fails without my patch from this bug and succeeds with it. It ensures that we do the right things when we never send 3rd party cookies. I wanted to extend the test to make sure that we do the right thing in the default setting, but we don't send cross-origin cookies for XHR (even if the server allows CORS connections) so I backed off on that.

Comment 50

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We'll send cookies for cross-site XHR requests if you set xhr.withCredentials=true

Comment 51

[Sylvestre Ledru [:Sylvestre]]

Added to the release notes with "Fix some loading issues when Accept third-party cookies: was set to Never (1254856)" as wording

Comment 52

[Camelia Badau [:cbadau], Desktop Test Engineering]

(In reply to Blake Kaplan (:mrbkap) (please use needinfo!) from comment #47)
> (In reply to Camelia Badau, QA [:cbadau] from comment #39)
> > ... On the "inbox.google.com" ...
> 
> I checked out a version of Firefox from before my fix for bug 1171215 and it
> took forever to load inbox.google.com (close to two minutes). I observed the
> same behavior with a version of Firefox with my fix for this bug. Therefore,
> I think that this is a separate issue and should be filed and tracked as a
> separate bug.

Thank you! 

I've tested also on Mac OSX 10.9.5 and Windows 7 64bit using Firefox 45.0.1 (buildID: 20160315153207) and Firefox ESR 45.0.1 (buildID: 20160315153150) and the games (https://en.forgeofempires.com/ ; https://en.elvenar.com/) correctly work with "Accept third-party cookies: Never" (in about:preferences#privacy).

Since the issue is verified on Firefox 48.0a1, Firefox 45.0.1 and Firefox ESR 45.0.1 build 1, I will mark this VERIFIED FIXED.  

For the issue from "inbox.google.com" page, I filled bug 1257236.

Comment 53

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Jonas Sicking (:sicking) from comment #50)
> We'll send cookies for cross-site XHR requests if you set
> xhr.withCredentials=true

That can be done as a followup. Right now the test uses sync xhr which throws if you set xhr.withCredentials.

Comment 54

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8731004 [details] [diff] [review]
Mochitest

Review of attachment 8731004 [details] [diff] [review]:
-----------------------------------------------------------------

Ok, I got it all now, but please do add a small description of the test. It's not that obvious right away that frameLoaded() checks for the correct results.

::: dom/plugins/test/mochitest/file_checkcookie.sjs
@@ +2,5 @@
> +  try {
> +    var cookie = request.getHeader("Cookie");
> +  } catch (e) {
> +    cookie = "EMPTY_COOKIE";
> +  }

Just to be on the safe side:

// avoid confusing cache behaviors
response.setHeader("Cache-Control", "no-cache", false);

::: dom/plugins/test/mochitest/mochitest.ini
@@ +136,5 @@
>  skip-if = toolkit == "cocoa"
>  [test_zero_opacity.html]
>  [test_bug1165981.html]
>  skip-if = !(os == "win" && processor == "x86_64" && !e10s) # Bug 1253957
> +[test_bug1245545.html]

is this just a line ending issue?

::: dom/plugins/test/mochitest/test_pluginstream_3rdparty.html
@@ +11,5 @@
> +  <script type="text/javascript">
> +    setTestPluginEnabledState(SpecialPowers.Ci.nsIPluginTag.STATE_ENABLED);
> +  </script>
> +  <link rel="stylesheet" type="text/css" 
> +        href="/tests/SimpleTest/test.css" />

Nit: could you remove trailing whitespaces and also add <meta charset=UTF-8> as you did in the other file.

@@ +15,5 @@
> +        href="/tests/SimpleTest/test.css" />
> +</head>
> +<body>
> +  <p id="display"></p>
> +

would you mind adding a two sentence description of the test? Whenever tests need to be updated I find it super useful to understand what the test is doing right away, because this is plugin specific cookie testing.

Comment 55

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #54)
> is this just a line ending issue?

Yeah, the existing file doesn't end with a newline (which a few programs still don't like), so my editor automatically adds one.

Comment 56

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ee273d5e868

Comment 57

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ee273d5e868

Comment 58

[Andrei Vaida [:avaida]]

Setting the flag for a spot check on 46.

Comment 59

[b1883900]

Huge thanks and gratitude to all those that reported and fixed this issue.

I am not sure if this is what was causing my issue but this is the only thing mentioned in 45.0.1 release notes that seems like it might have been.

Going from 44.0.2 to 45 had caused Hulu not to work for me.  Everything on the site was functioning as normal except whenever clicking a show to watch.  Then it was like Hulu wasn't able to see my account so I was unable to watch anything other than free shows (in lower quality, commercials, etc.) as it would just say this show requires a free trial, a paid subscription or to log in (even though it already recognized me as logged in).  

The site appeared normal otherwise as it would show my watched shows on the main page, my queue, history, favorites, account info showing paid, etc.  Just actually trying to watch a show would only allow me to do so as an unlogged in free member.  Nothing I tried fixed it and Hulu was no help (just use Chrome).

I tried deleting cookies, cache, logging in and out, uninstalling and re-installing Flash, resetting license files, trying about:config settings, enabling/disabling DRM (as Hulu was popping up a must enable DRM banner around this time also), thinking it might be a Noscript setting issue (as 45 seems like it caused a click to allow issue with Noscript also that apparently is unrelated to this).  

Hasn't been a pleasant week or two trying to figure this out as this is above my knowledge level. I couldn't find anyone else mentioning the Hulu issue anywhere and I didn't understand how to ask for help here or file a bug report.

So thank you to all those involved in fixing this.  Grateful.

Comment 62

[Kohei Yoshino]

This bug didn't fix the Google Inbox issue which is currently tracked in Bug 1257861. Reverting the summary and keyword for clarification.

Comment 63

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've tested on Windows 7 64bit and on Mac OSX 10.9.5 using Firefox 46 Beta 6 (buildID: 20160328182534 ) and Aurora 47.0a2 (buildID: 20160323004040): in about:preferences#privacy I've set "Accept third-party cookies: Never" and the games (https://en.forgeofempires.com/ ; https://en.elvenar.com/) correctly work, i'm able to play.