4.47 KB, patch
|Details | Diff | Splinter Review|
5.18 KB, patch
|Details | Diff | Splinter Review|
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?
(In reply to Kyle Huey [:khuey] (firstname.lastname@example.org) 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.
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)
Keywords: dev-doc-needed, regression, regressionwindow-wanted, site-compat
status-firefox45: --- → affected
status-firefox-esr45: --- → affected
tracking-firefox45: --- → ?
tracking-firefox-esr45: --- → ?
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
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.
Component: DOM: Workers → Networking: Cookies
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
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?
(In reply to Kyle Huey [:khuey] (email@example.com) from comment #9) > Who owns third party cookie blocking? SecEng :) Blake, could this be related to Bug 1254856?
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
Tracking, regression starting in 45.
tracking-firefox46: ? → +
tracking-firefox47: ? → +
tracking-firefox48: ? → +
Created attachment 8734963 [details] [diff] [review] mochitest
Attachment #8734963 - Flags: review?(jonas)
Attachment #8734963 - Flags: review?(jonas) → review+
Created attachment 8734976 [details] [diff] [review] Fix We explicitly don't set a loading context for XHR initiated from a web worker  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  https://dxr.mozilla.org/mozilla-central/rev/d5f3da0cfe7ccf846c354014c9b059fad6ba0de5/dom/base/nsXMLHttpRequest.cpp#1623
Attachment #8734976 - Flags: review?(jonas)
Attachment #8734976 - Flags: review?(jonas) → review+
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+
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.
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.
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
Created attachment 8737941 [details] [diff] [review] Mochitest patch as it landed
Attachment #8737941 - Flags: review+
Attachment #8734963 - Attachment is obsolete: true
Created attachment 8737942 [details] [diff] [review] Actual fix as it landed
Attachment #8737942 - Flags: review+
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
Comment on attachment 8737942 [details] [diff] [review] Actual fix as it landed Approval Request Comment See comment 30.
Blake, do you think we should take it for release? Thanks
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?
Florin or Andrei can you verify the fix on nightly, here and maybe with the STR in bug 1257236 as well? Thanks.
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.
status-firefox47: affected → fixed
status-firefox46: affected → fixed
(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.
Comment on attachment 8737941 [details] [diff] [review] Mochitest patch as it landed [Triage Comment]
tracking-firefox45: ? → blocking
tracking-firefox-esr45: ? → 45+
status-firefox45: affected → fixed
status-firefox-esr45: affected → fixed
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 ?
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+
(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?
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
status-firefox45: fixed → verified disabled
status-firefox46: fixed → verified
status-firefox47: fixed → verified
status-firefox48: fixed → verified
status-firefox45: verified disabled → verified
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.