Closed Bug 722683 Opened 12 years ago Closed 8 years ago

The request that fetches the HTML5 manifest file doesn't send cookies from main domain when third-party cookies are disabled

Categories

(Core :: Networking: Cookies, defect)

10 Branch
x86
All
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: maxime.rety, Assigned: mayhemer)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111220165912

Steps to reproduce:

* disable third-party cookies
* ensure your backend server is creating a session for each visitor of your domain
* a "_session_id" cookie (belonging to the same single domain) will be set by the first page you download from that domain, and sent again with each subsequent request (http, xmlhttrequest...)
* create an "index" page with a manifest
* download that index page in firefox


Actual results:

The request to download the manifest file, triggered implicitely by firefox, is sent without the domain cookies set by the index page (including _session_id).


Expected results:

The request to download the manifest file must send the cookies (including _session_id) since we did not disable cookies, but only third-party cookies.

It's a problem that can bite you if you serve manifest file only to authenticated clients.

The problem does not occur if visitor did not disable third-party cookies.
OS: Mac OS X → All
Summary: Request downloading the HTML5 manifest file doesn't send cookies from same domain when third-party cookies are disabled → The request that fetches the HTML5 manifest file doesn't send cookies from main domain when third-party cookies are disabled
Component: Untriaged → Networking
Product: Firefox → Core
QA Contact: untriaged → networking
Component: Networking → Networking: Cookies
QA Contact: networking → networking.cookies
Status: UNCONFIRMED → NEW
Ever confirmed: true
I stumbled across this report while debugging a web application with offline cache, which relies on a session cookie for authentication.

Could a developer please specify if this is a desired feature or a bug (my assumption)?
Looking into this (at last :))
Assignee: nobody → honzab.moz
Problem is that channels that update is invoking are not associated with the load context, i.e. a window.  This is actually correct, since a) two updates can be invoked by more then a single window, b) update algorithm is running on the chrome process in e10s env where we don't have a window.

One option is to always allow third party cookies on update channels.  But that could allow tracking when updates are invoked in iframes.

IMO, the best way is to check whether the window invoking the update is third party or not and then allow all loads that are first party according the manifest URI (that is same-origin with the window) to send and store cookies by call to SetForceAllowThirdPartyCookie(true), that bypasses check for load context in ThirdPartyUtil::IsThirdPartyChannel.

Any objections?
Attached patch v1Splinter Review
Attachment #643025 - Flags: review?(jduell.mcbugs)
Status: NEW → ASSIGNED
Comment on attachment 643025 [details] [diff] [review]
v1

Honza has promised me a new unbitrotted version, and I've promised to review it more quickly :)
Attachment #643025 - Flags: review?(jduell.mcbugs)
Jason, maybe you could at least look at the patches and give comments regardless the patches are outdated..
Comment on attachment 643025 [details] [diff] [review]
v1

sure, will do.
Attachment #643025 - Flags: feedback?(jduell.mcbugs)
Hello,

Just a little question, in which version of Firefox this bug will be fixed?

Thank you very much.
Ping. This is killing our app. Is a fix for this planned?
Meant to add that since FF now *defaults to disabling third-party cookies*, this bug's exposure has increased significantly.
(In reply to Jordan from comment #12)
> Meant to add that since FF now *defaults to disabling third-party cookies*,
> this bug's exposure has increased significantly.

This is only true on Nightly and Aurora builds, which are a distinct minority.
I am not sure it makes sense to fix this bug by sending cookies for updates of the manifest file. Another way to fix this bug may be to *never* send cookies on manifest requests, including the first one. That is, don't allow the "only serve the manifest to logged-in users" thing to work.

I think it is extremely problematic to serve the manifest only to logged-in users. If the user clears his/her cookies, then he cannot receive the updated version of the app. On B2G, if I understand correctly, this means that potentially the app will be stuck on the last downloaded version *forever* with no way for the user to log into the app unless the old version of the app has a way to log in. And, even if the app does have a way to log in then it still runs the risk that some bug in the login functionality of the app will cause the app to get stuck in its old version forever. That would be very bad if the version of the app the user is stuck on is in need of a critical security update that would then be un-downloadable for them.

The AppCache spec doesn't say anything useful about cookies. I think we should strongly consider making AppCache completely cookie-free. That is, never send cookies for the manifest and never send cookies for any of the resources listed in the manifest.

In contract, I think HTTP auth is actually OK to require because we'll (IIUC) automatically prompt the user for the needed credentials automatically when we get the 401 Unauthorized response.

Anyway, it seems to me that any way of doing this is problematic in one way or another, so we should be careful to make the right change.
Brian, I think we should be in parity with other here.  I don't share your no-cookies paranoia here.

I'll test how other browsers (IE 10, Chr) work and fix our code accordingly.
Brian, it make sense to serve the manifest only for logged-in users!
In the app we have developed, this functionnality is used only for logged-in user to let them copy some data into the AppCache for an offline usage.

When the browser calls the server to get the content of the manifest file, if you remove the cookie, the server cannot know which manifest content it must provide.

So, do not change this to have a cookie-free AppCache or will have a very BIG problem in our app.
In case there's still any question, I'd like to re-enforce what Jimmy said. It seems that there are two classes of appcached apps: 1) B2G app store style 2) traditional Web apps with offline components/modes. For #1, authors should understand that their manifests must be uniformly available. For #2, the manifest itself, and many of the resources in it, may be dynamic generted based on the session/user.

#2 has worked very well for the offline portion of our app across both Chrome and FF (except for this bug, which has suddenly begun hitting our users for some reason.) Making this bug into a feature will eliminate the class of apps in #2, putting some projects and companies in very difficult positions.

I understand that the AppCache spec is vague on this point. Whether through design or accident, sending cookies to manifest files and their resources has been the behavior of all the browsers, except for this single edge case. In that time, it's been used to create some very useful applications.
Hello,

Any update?

Thanks.
I also want to up-vote Jimmy and Jordan's sentiments.

Let the app-writers worry about whether their app will update properly within the framework of their authentication mechanism.  They always have the option, via script, of ensuring the proper authentication cookies and then calling applicationCache.update() manually.

Cookies are an enormously powerful, ubiquitous, HTML platform primitive. Unless otherwise forbidden via specification, it's entirely unexpected for something like this to not work.
Status: ASSIGNED → NEW
Attachment #643025 - Flags: feedback?(jduell.mcbugs)
Appcache is dead.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: