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

VERIFIED FIXED in Firefox 45

Status

()

Core
Networking: Cookies
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: elhambree, Assigned: mrbkap)

Tracking

(4 keywords)

Trunk
mozilla48
dev-doc-complete, regression, site-compat, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45blocking verified, firefox46+ verified, firefox47+ verified, firefox48+ verified, firefox-esr4545+ verified, relnote-firefox 45+)

Details

(Whiteboard: [necko-active], URL)

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

2 years ago
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.)

Updated

2 years ago
Component: Untriaged → DOM: Workers
Product: Firefox → Core
Do you have a URL where this can be observed?
Flags: needinfo?(elhambree)
Keywords: testcase-wanted
(Reporter)

Comment 2

2 years ago
(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)

Comment 3

2 years ago
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)

Updated

2 years ago
Keywords: dev-doc-needed, regression, regressionwindow-wanted, site-compat

Updated

2 years ago
status-firefox45: --- → affected
status-firefox-esr45: --- → affected
tracking-firefox45: --- → ?
tracking-firefox-esr45: --- → ?
Keywords: testcase-wanted
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
(Reporter)

Updated

2 years ago
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)
Keywords: regressionwindow-wanted → testcase
OS: Windows → All
Hardware: x86 → All
See Also: → bug 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
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/cookies-are-missing-in-xhr-post-requests-from-workers-when-third-party-cookies-are-blocked/
status-firefox46: --- → affected
status-firefox47: --- → affected
status-firefox48: --- → affected
tracking-firefox46: --- → ?
tracking-firefox47: --- → ?
tracking-firefox48: --- → ?
relnote-firefox: --- → ?
Keywords: dev-doc-needed → dev-doc-complete
(Reporter)

Comment 8

2 years ago
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)
(Assignee)

Comment 11

2 years ago
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.
tracking-firefox46: ? → +
tracking-firefox47: ? → +
tracking-firefox48: ? → +
(Assignee)

Comment 13

2 years ago
Created attachment 8734963 [details] [diff] [review]
mochitest
Attachment #8734963 - Flags: review?(jonas)
(Assignee)

Comment 14

2 years ago
Created attachment 8734976 [details] [diff] [review]
Fix

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)
(Assignee)

Updated

2 years ago
Duplicate of this bug: 1249083
(Assignee)

Comment 16

2 years ago
I filed bug 1259873 on comment 14 and updated my patch to point to that bug number in the comment.

Updated

2 years ago
Duplicate of this bug: 1255082

Updated

2 years ago
Duplicate of this bug: 1258159

Updated

2 years ago
Duplicate of this bug: 1257236
(Assignee)

Comment 20

2 years ago
Created attachment 8735557 [details] [diff] [review]
Fix this test

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+

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a0c84b4e0da2
https://hg.mozilla.org/mozilla-central/rev/e9954f2bede1
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox48: affected → fixed
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)
(Assignee)

Comment 25

2 years ago
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)

Comment 26

2 years ago
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

Updated

2 years ago
Duplicate of this bug: 1260404
(Assignee)

Comment 28

2 years ago
Created attachment 8737941 [details] [diff] [review]
Mochitest patch as it landed
Attachment #8737941 - Flags: review+
(Assignee)

Updated

2 years ago
Attachment #8734963 - Attachment is obsolete: true
(Assignee)

Updated

2 years ago
Attachment #8734976 - Attachment is obsolete: true
Attachment #8735557 - Attachment is obsolete: true
(Assignee)

Comment 29

2 years ago
Created attachment 8737942 [details] [diff] [review]
Actual fix as it landed
Attachment #8737942 - Flags: review+
(Assignee)

Comment 30

2 years ago
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?
(Assignee)

Comment 31

2 years ago
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.
(Assignee)

Comment 36

2 years ago
(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+
tracking-firefox45: ? → blocking
tracking-firefox-esr45: ? → 45+
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.
relnote-firefox: ? → 45+
(Assignee)

Comment 46

2 years ago
(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)

Updated

2 years ago
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
status-firefox-esr45: fixed → affected
Flags: needinfo?(andrei.vaida)
status-firefox45: fixed → verified disabled
status-firefox46: fixed → verified
status-firefox47: fixed → verified
status-firefox48: fixed → verified
status-firefox45: verified disabled → verified
Depends on: 1263533
Just verified the fix for remaining issue on Bug 1263533 and it's all good.
status-firefox-esr45: affected → verified
You need to log in before you can comment on or make changes to this bug.