Closed Bug 1257861 Opened 8 years ago Closed 8 years ago

Firefox 45 fails to send Cookie header with XHR post requests done from a web worker when third-party cookies are blocked

Categories

(Core :: Networking: Cookies, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla48
Tracking Status
firefox45 blocking verified
firefox46 + verified
firefox47 + verified
firefox48 + verified
firefox-esr45 45+ verified
relnote-firefox --- 45+

People

(Reporter: elhambree, Assigned: mrbkap)

References

()

Details

(4 keywords, Whiteboard: [necko-active])

Attachments

(2 files, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160303134406

Steps to reproduce:

I'm loading an upload page that starts a session on the main thread while processing the actual upload from within a web worker. There is no CORS involved; all scripts are communicating with the same domain.

The upload script initiates an XHR post request that performs the uploading process.


Actual results:

The upload process fails due to the fact that the upload script (calling from the Web worker) doesn't include the session cookie with the POST request.


Expected results:

The upload session cookie should have been included but Firefox 45 on Windows 7 fails to do it.

This script has always worked with the previous versions but it has stopped working now with FF-45. 

Interestingly when I tested it  on my FF-45 on Fedora-23 it works. Also, both the current nightly and the developer versions work fine with that script. So, this bug seems to affect FF-45 on Windows (specifically 7--can't test the other versions.)
Component: Untriaged → DOM: Workers
Product: Firefox → Core
Do you have a URL where this can be observed?
Flags: needinfo?(elhambree)
Keywords: testcase-wanted
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1)
> Do you have a URL where this can be observed?

Indeed, you can observe the issue @this URL:
https://push-fouadchk.rhcloud.com/demos/upload

It's a Node.js WebApp.
Flags: needinfo?(elhambree)
I have the same problem, having 3 servers with loadbalancing (haproxy) and data persistency with cookies.

When I upload 3 files from the same session, all files are not redirected on the same server, because haproxy did not detect the session cookie. (marked "NN" on logs)

Same test with Chrome and IE : haproxy detect sessions for each files and redirect them acccordingly. (marked "VN" and showing cookie session on log)
2 people have encountered the issue, marking the status NEW, changing the component for better attention.
Status: UNCONFIRMED → NEW
Component: DOM: Workers → Networking: Cookies
Ever confirmed: true
Not convinced (based on description/comments) this belongs in necko
Component: Networking: Cookies → DOM: Workers
OS: Unspecified → Windows
Hardware: Unspecified → x86
I can reproduce the issue with Comment 2's example on Firefox 45.0.1 and Nightly, OS X 10.11, when third-party cookies are blocked. Guess this is similar to Bug 1254856 fixed with 45.0.1. Changing the topic and component again.
Blocks: 1171215
Component: DOM: Workers → Networking: Cookies
Flags: needinfo?(mrbkap)
OS: Windows → All
Hardware: x86 → All
See Also: → 1254856
Summary: Firefox 45 fails to send cookie header with XHR post requests done from a web worker in Windows 7 → Firefox 45 fails to send Cookie header with XHR post requests done from a web worker when third-party cookies are blocked
Version: 45 Branch → Trunk
I completely agree about those last updates, specifically that this issue arises when third-party cookies are blocked. My FF-45 has them blocked while the developer and the nightly versions didn't. And the behavior reverses when the third-party cookies option is flip from Never to Always (or vice-versa.)

So, I guess you're on the right track on this guys. Good Luck all!
Who owns third party cookie blocking?
Flags: needinfo?(sworkman)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #9)
> Who owns third party cookie blocking?

SecEng :)

Blake, could this be related to Bug 1254856?
Flags: needinfo?(sworkman)
It is related, for sure. I'm going to debug this, fix it, and then switch the default in LoadInfo to assume that we're not in a third-party context. That should cause this bug and all others like it (where we don't pass enough information to compute 3rd party or not) to quietly pass.
Assignee: nobody → mrbkap
Flags: needinfo?(mrbkap)
Whiteboard: [necko-active]
Tracking, regression starting in 45.
Attached patch mochitest (obsolete) — Splinter Review
Attachment #8734963 - Flags: review?(jonas)
Attached patch Fix (obsolete) — Splinter Review
We explicitly don't set a loading context for XHR initiated from a web worker [1] which means that the code was trying to be default-safe and blocked 3rd-party cookies. We can't do that. This patch just fixes things so that we don't block cookies in that case (or other cases where we don't have the data to make the correct decision). I'll file a followup bug on fixing web workers (sicking has ideas) but we should land this everywhere in the meantime.

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

[1] https://dxr.mozilla.org/mozilla-central/rev/d5f3da0cfe7ccf846c354014c9b059fad6ba0de5/dom/base/nsXMLHttpRequest.cpp#1623
Attachment #8734976 - Flags: review?(jonas)
I filed bug 1259873 on comment 14 and updated my patch to point to that bug number in the comment.
Attached patch Fix this test (obsolete) — Splinter Review
This test started failing because it was testing that we assumed third-party. Now we don't. I also marked the loads as being TYPE_DOCUMENT since we now try to get a URI out of the loading principal (which is a system principal) in order to compare the channel against its "parent" which doesn't exist. With TYPE_DOCUMENT, we avoid that.
Attachment #8735557 - Flags: review?(jonas)
Comment on attachment 8735557 [details] [diff] [review]
Fix this test

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

rs=me
Attachment #8735557 - Flags: review?(jonas) → review+
https://hg.mozilla.org/mozilla-central/rev/a0c84b4e0da2
https://hg.mozilla.org/mozilla-central/rev/e9954f2bede1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Would this be good to uplift to aurora/beta? This seems very noticeable on gmail in bug 1257236.
Flags: needinfo?(mrbkap)
This is good to uplift to aurora/beta. I will request approval in a couple of days, once it's baked on Nightly for a bit.
Flags: needinfo?(mrbkap)
Workaround till fix is in, from https://productforums.google.com/forum/#!topic/inbox/owVGYomGTZ0;context-place=forum/inbox


> Anomin said:
>Did you try this ad hoc solution:

>about:preferences#privacy

>- Check Accept cookies from sites
>- Accept third-party cookies: From visited
Attachment #8734963 - Attachment is obsolete: true
Attachment #8734976 - Attachment is obsolete: true
Attachment #8735557 - Attachment is obsolete: true
Comment on attachment 8737941 [details] [diff] [review]
Mochitest patch as it landed

Approval Request Comment
[Feature/regressing bug #]: bug 1171215
[User impact if declined]: Users that disable 3rd party cookies will have sites fail to work if those sites send XHR requests from web workers.
[Describe test coverage new/current, TreeHerder]: I added a mochitest in addition to the existing test coverage for this feature.
[Risks and why]: Low risk, patch should only affect the feature.
[String/UUID change made/needed]: N/A
Attachment #8737941 - Flags: approval-mozilla-beta?
Attachment #8737941 - Flags: approval-mozilla-aurora?
Comment on attachment 8737942 [details] [diff] [review]
Actual fix as it landed

Approval Request Comment
See comment 30.
Attachment #8737942 - Flags: approval-mozilla-beta?
Attachment #8737942 - Flags: approval-mozilla-aurora?
Blake, do you think we should take it for release? Thanks
Flags: needinfo?(mrbkap)
Comment on attachment 8737941 [details] [diff] [review]
Mochitest patch as it landed

Has obvious user impact, includes new tests.
Let's try this fix for beta 9 and for aurora. 
Kohei can you help verify this on beta once it lands?
Flags: needinfo?(kohei.yoshino)
Attachment #8737941 - Flags: approval-mozilla-beta?
Attachment #8737941 - Flags: approval-mozilla-beta+
Attachment #8737941 - Flags: approval-mozilla-aurora?
Attachment #8737941 - Flags: approval-mozilla-aurora+
Florin or Andrei can you verify the fix on nightly, here and maybe with the STR in bug 1257236 as well? Thanks.
Flags: needinfo?(florin.mezei)
Attachment #8737942 - Flags: approval-mozilla-beta?
Attachment #8737942 - Flags: approval-mozilla-beta+
Attachment #8737942 - Flags: approval-mozilla-aurora?
Attachment #8737942 - Flags: approval-mozilla-aurora+
Tested Google Inbox, Google Sheets and the demo uploader in Comment 2 with the latest Nightly. All is well even when Accept third-party cookies: Never. Waiting for Aurora & Beta.
(In reply to Sylvestre Ledru [:sylvestre] from comment #32)
> Blake, do you think we should take it for release? Thanks

This seems like a high-enough profile regression to consider taking for release.
Flags: needinfo?(mrbkap)
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #34)
> Florin or Andrei can you verify the fix on nightly, here and maybe with the
> STR in bug 1257236 as well? Thanks.

Kohei seems to be handling verification here - already verified on Nightly in comment 35, waiting for Aurora and Beta toverify. 

Setting needinfo to Andrei so he can monitor whether our assistance is also needed here.
Flags: needinfo?(florin.mezei) → needinfo?(andrei.vaida)
Comment on attachment 8737942 [details] [diff] [review]
Actual fix as it landed

OK, let's take it.
Taking it into ESR45 too.
Attachment #8737942 - Flags: approval-mozilla-release+
Attachment #8737942 - Flags: approval-mozilla-esr45+
Comment on attachment 8737941 [details] [diff] [review]
Mochitest patch as it landed



[Triage Comment]
Attachment #8737941 - Flags: approval-mozilla-release+
Attachment #8737941 - Flags: approval-mozilla-esr45+
Hi guys,

Verified the fix for this issue on Firefox 45.0.2, 45.0.2 ESR and 46.0b9 candidates builds. Unfortunately this issue still reproduces on ESR build1 on both x32 and x64 architectures and on all OSes. Verified also the candidate build2 for Mac OS (only one available at the moment) and it still reproduces. Probably the results will be the same for other platforms.
 
Tested with the provided testcase and also the two scenarios from Bug 1257236 while having set "Accept third-party cookies: Never" in about:preferences#privacy.

How should we proceed in this case ? Should we log a new issue separately only for ESR builds ?
Flags: needinfo?(mrbkap)
Added to the release notes with "Fix an issue impacting the cookie header when third-party cookies are blocked (1257861)" as wording.
(In reply to Paul Oiegas [:pauloiegasSV] from comment #44)
> How should we proceed in this case ? Should we log a new issue separately
> only for ESR builds ?

Yeah, let's track the ESR problem in a new bug. I'm a little confused though, this is fixed on 45.0.2 ESR but not some other ESR branch? Which branch is that?
Flags: needinfo?(mrbkap)
Flags: needinfo?(kohei.yoshino)
Logged separately the ESR problem in Bug 1263533. Considering this I am marking this one as verified-fixed.
Status: RESOLVED → VERIFIED
Flags: needinfo?(andrei.vaida)
Depends on: 1263533
You need to log in before you can comment on or make changes to this bug.