Closed
Bug 527667
Opened 15 years ago
Closed 7 years ago
DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Recent History" is used with Time range not "Everything"
Categories
(Core :: DOM: Core & HTML, defect, P1)
Tracking
()
RESOLVED
DUPLICATE
of bug 1047098
People
(Reporter: nwmoussa, Unassigned, NeedInfo)
References
(Depends on 1 open bug, Blocks 3 open bugs)
Details
(Keywords: dev-doc-needed, privacy, sec-want, Whiteboard: [sg:want][tor][fingerprinting])
Attachments
(1 file, 2 obsolete files)
32.05 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 (.NET CLR 3.5.30729)
DOM Storage (https://developer.mozilla.org/en/DOM/Storage) allows websites to store cookie-like information in a persistent way. This data appears to be loaded into the user's profile directory into a file called webappsstore.sqlite
This file can contain private and personally-identifiable data, so it should be cleared along with cookies, browsing history, etc. However, when I perform the "Clear Recent History" (which is configured to clear everything), the file persists and still contains its original data. This data should be cleared along with cookies and other persistent-storage.
Reproducible: Always
Steps to Reproduce:
1. Visit a website which uses the DOM Storage feature to store persistent or session data. (e.g. http://www.cnn.com or many others)
2. Clear private history via the Firefox menu
3. Observe that the DOM Storage file, webappsstore.sqlite in the user profile directory, still contains identifiable information (in both binary and plaintext) that identifies the user's browsing history.
Actual Results:
The DOM Storage file, webappsstore.sqlite in the user profile directory, still contains identifiable information (in both binary and plaintext) that identifies the user's browsing history.
Expected Results:
I would expect that DOM Storage is cleared, just like cookies and other data are cleared.
I know DOM Storage is a fairly recent addition, but it is important to address this standard and make sure that it is properly handling data.
Web sites can use DOM Storage for personally-identifiable data - this is the entire reason why we have a "Clear Private Data" option. This persistent storage file (webappsstore.sqlite) should be cleaned or deleted too.
Comment 1•15 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/sanitize.js#133
133 cookies: {
134 clear: function ()
..
139 if (this.range) {
..
150 else {
151 // Remove everything
152 cookieMgr.removeAll();
If the time range is not specified ("Everything"), the cookieMgr.removeAll() call clears the DOM storage data (implemented in bug 352704).
If the time range *is* specified, we try to clear the data by enumerating cookies, and DOM storage items are not part of this enumeration (see also bug 506692).
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: privacy
Summary: DOM Storage data is not cleared when "Clear Private Data" is used → DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Private Data" is used with Time range not "Everything"
Version: unspecified → Trunk
Updated•15 years ago
|
Summary: DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Private Data" is used with Time range not "Everything" → DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Recent History" is used with Time range not "Everything"
Updated•14 years ago
|
blocking2.0: --- → ?
Component: DOM: Other → General
Product: Core → Firefox
QA Contact: general → general
Comment 3•14 years ago
|
||
Saw my bug was a dup of this one. Any thoughts on when this might get addressed?
There has been an increased focus on tracking users via local storage and a few lawsuits filed against ad companies. It seems we should give users the ability to clear local storage data.
http://arstechnica.com/apple/news/2010/09/rldguid-tracking-cookies-in-safari-database-form.ars
Updated•14 years ago
|
blocking2.0: ? → betaN+
Comment 5•14 years ago
|
||
We probably wouldn't actually take this fix in a 3.5.x release, but maybe. We'd definitely want this in 3.6.x (assuming there is unlikely to be any l10n impact from something like this).
Comment 6•14 years ago
|
||
Nickolay, thanks for you comment 1.
Summary a bit out of scope of this bug:
1. localStorage can be used by a common web sites to save data locally on the user's computer, and those are actually close to http cookies
2. localStorage can be used as a storage of data for offline applications ; a web site becomes considered an offline application after user permits that through an already existing UI ; at this moment localStorage can no longer be considered as http cookies, this is (AFAIK) fixed since Gecko 1.9.2
In the first case we are missing few things to really treat localStorage as cookies:
a. we do not present the values in the cookies UI (only http cookies are displayed, bug 506692 seems to be dedicated to that)
b. we do not always delete localStorage data that keeps "localStorage cookies" along with http cookies (this bug should be about it)
--------
To implement deletion of recent localStorage values that are "localStorage cookies" we need to add a new column to the database with date-time of value addition. Not sure we can add such a change to Fx 3.6.x.
Comment 7•14 years ago
|
||
- based on patch from bug 536544 (using a lot of code from that patch)
- new method nsIDOMStorageManager.clearStorageDataSince taking the time range
- it deletes data newer then the range from both temp and disk table or PB or SO database
- each storage instance need to get its mItems cache updated ; achieved by observer call to all existing storages
- mItems entry type now contains insertion time ; used only for sessionStorage (i.e. when not using DB)
- updating the database schema: a new column+index inserttime is added
- added tests for localStorage, also in PB mode, sessionStorage, globalStorage (found bug 600366)
Attachment #479219 -
Flags: review?(jst)
Comment 8•14 years ago
|
||
Comment on attachment 479219 [details] [diff] [review]
v1 [Backed out due to oranges]
r=jst!
Attachment #479219 -
Flags: review?(jst) → review+
Updated•14 years ago
|
Attachment #479219 -
Flags: superreview+
Comment 9•14 years ago
|
||
A new method added:
https://bugzilla.mozilla.org/attachment.cgi?id=479219&action=diff#a/dom/interfaces/storage/nsIDOMStorageManager.idl_sec2
BTW, documentation of nsIDOMStorageManager interface says it "provides methods for managing data stored in the offline apps cache".
It is not true at all, the storage manager is more or less an internal service. It is managing DOM storage data (allow selective deletion and usage determination of that data) and instances (creation of shared localStorage objects). The storage objects and data created with them can also be used by non-offline allowed web sites.
Keywords: dev-doc-needed
Comment 10•14 years ago
|
||
Comment on attachment 479219 [details] [diff] [review]
v1 [Backed out due to oranges]
http://hg.mozilla.org/mozilla-central/rev/2ccadd55dc4c
Attachment #479219 -
Attachment description: v1 → v1 [Check in comment 10]
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 11•14 years ago
|
||
Backed out:
http://hg.mozilla.org/mozilla-central/rev/c6ba385c4435
Caused orange:
s: talos-r3-fed-033
5875 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_cookieSession-phase1.html | undefined - got null, expected "persistent value 1"
5876 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_cookieSession-phase1.html | undefined - got null, expected "persistent value 2"
5880 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_cookieSession-phase2.html | Persistent value present - got null, expected "persistent value 1"
5881 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_cookieSession-phase2.html | Persistent value present - got null, expected "persistent value 2"
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•14 years ago
|
Status: REOPENED → ASSIGNED
Comment 12•14 years ago
|
||
Also seen:
6263 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_localStorageDeleteSinceAPIPrivateBrowsing.html | key2 left - got null, expected "value2"
6264 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_localStorageDeleteSinceAPIPrivateBrowsing.html | key3 left - got null, expected "value3"
6266 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_localStorageDeleteSinceAPIPrivateBrowsing.html | Expected 2 items after delete in localStorage - got 0, expected 2
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288034097.1288037332.7694.gz
Comment 13•14 years ago
|
||
So, the failure from comment 11 is cooperation of 3 tests:
1.
- general/test_windowProperties.html reads all properties of the window, that instantiates an empty localStorage
- we never get or set an item on it, so, we never call nsDOMStorage::CacheStoragePermissions that properly sets mSessionOnly
- mSessionOnly is set to PR_TRUE by default
2.
- globalStorage/test_globalStorageDeleteSinceAPU.html (introduced in this bug) invokes the clearStorageDataSince method that also our previously created localStorage listens to (introduced in this bug)
- it calls CacheKeysFromDB() that leads to call to nsDOMStorageMemoryDB::GetItemsTable (called on nsDOMStorageDBWrapper.mSessionOnlyDB) that creates for the first time an in-mem storage for this localStorage and pre-loads it with data from DB for this localStorage, at this moment empty
- this happens because the storage has mSessionOnly set to PR_TRUE
3.
- localStorage/test_coockieSession-phase1.html stores some values to the localStorage, those are expected to be in the persistent database
- at that moment it really happens as expected because SetItem call on that storage now properly sets mSessionStorage to PR_FALSE
- problem starts at the moment we switch to ACCESS_SESSION behavior
- expected is, that this is the first time we create an in-mem db for the storage and copy data from the persistent DB to it
- but we actually already have created that in-mem DB sooner, so the new values added to the persistent DB are not copied to the in-mem DB as expected
==> persistent key/value pairs are not present in the localStorage when in ACCESS_SESSION mode
At the moment I'm not sure how to fix this. CacheStoragePermission, that properly sets mSessionOnly, is using the subject (caller's) principal to check the permission state against. We don't have it at the moment we observer for cut-off (step 2 above) in nsDOMStorage::Observe for NS_DOMSTORAGE_CUTOFF_OBSERVER.
We could use principal that the storage keeps, as it can never be accessed by a codebase principal different from the one it was created for. I'll check this approach and create a patch in a different bug.
To have a quick fix, that also might be considered correct, is to drop the in-mem db also when switching to ACCESS_SESSION and not only when switching to ACCESS_DEFAULT. It makes perfectly sense, by the way.
Updated•14 years ago
|
Attachment #479219 -
Attachment description: v1 [Check in comment 10] → v1 [Backed out due to oranges]
Comment 14•14 years ago
|
||
I decided to go the simple way. We now drop the session only in-mem DB in both cases when we turn the session only access off and on (off because we want to wipe the data out, on because we want to make sure it will be loaded with actual data from the persistent DB).
Passed try server.
To explain how it works w/o this little fix:
1. it may happen that a session only in-mem DB is created (and at that moment preloaded with data from the persistent DB to keep a copy) while we are in ACCESS_DEFAULT (not session-only) mode for a host
2. we modify the persistent database for the host (by accessing its localStorage)
3. we switch to ACCESS_SESSION and we expect that all data that are in the persistent DB are copied to the session only in-mem DB AT THIS MOMENT ; but it has already been created sooner and is not therefor re-filled with the recent modifications to the persistent DB made in the step 2
Attachment #486334 -
Flags: review?(jst)
Updated•14 years ago
|
Attachment #486334 -
Flags: review?(jst) → review+
Comment 15•14 years ago
|
||
Attachment #479219 -
Attachment is obsolete: true
Attachment #486334 -
Attachment is obsolete: true
Attachment #486377 -
Flags: superreview+
Attachment #486377 -
Flags: review+
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Comment 16•14 years ago
|
||
Backed out again, due to new oranges, not catch by try before:
http://hg.mozilla.org/mozilla-central/rev/1e4671765e79
s: talos-r3-fed-042
5851 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/globalstorage/test_globalStorageDeleteSinceAPI.html | Expected 2 items after delete - got 1, expected 2
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288204013.1288204332.16521.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•14 years ago
|
Attachment #486377 -
Attachment description: v1.1 as landed [Check in comment 15] → v1.1 [Backed out comment 16]
Updated•14 years ago
|
Status: REOPENED → ASSIGNED
Comment 17•14 years ago
|
||
This ~could~ be related to bug 600366. We should then fix it before landing this bug.
Comment 18•14 years ago
|
||
Also seen (intermittent):
s: talos-r3-leopard-032
6280 ERROR TEST-UNEXPECTED-FAIL | /tests/dom/tests/mochitest/localstorage/test_localStorageDeleteSinceAPI.html | Expected 4 items before delete in localStorage - got 0, expected 4
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1288204717.1288207002.29555.gz
Comment 19•14 years ago
|
||
Not a regression, and we need to ship. Not going to block on this now, and probably wouldn't take it given the UI changes and size of the patch.
blocking2.0: betaN+ → -
Updated•14 years ago
|
blocking2.0: - → .x
Comment 20•14 years ago
|
||
Seconded, this is a complex patch and we are probably building it on another regression from bug 536544 (bug 607418) which I was trying to figure out for weeks before working on this bug.
Updated•13 years ago
|
OS: Windows XP → All
Comment 21•13 years ago
|
||
It appears the sessionStorage is not being removed at all by the cookie manager removeAll() method (via the "cookie-changed" "cleared" observer notification).
Comment hidden (off-topic) |
Comment 23•11 years ago
|
||
This is bad for privacy (may also have grave adverse effects on Persona!). Honza, will you find time to work on this for Fx 30?
Flags: needinfo?(honzab.moz)
Comment 24•11 years ago
|
||
Unlikely. As a workaround I suggest to delete just all data, as we do with the HTTP cache. That should be easy to do.
Flags: needinfo?(honzab.moz)
Comment 25•11 years ago
|
||
Workaround is clear (to us), though virtually all users have no idea about that bug and may depend on this working for all time ranges (and not just "everything").
Anybody else who can take this?
Comment 26•11 years ago
|
||
No, what I suggest is to delete all data even when only a time-range is selected. I.e. ALWAYS delete all data. See how the HTTP cache is deleted, we do the same, always clear all.
Comment 27•11 years ago
|
||
HTTP cache generally contains easily replaceable data, it's bit of a different story for DOM Storage though (sometimes used for "valuable"/hard to replace data).
Comment 28•11 years ago
|
||
Hmm.. we treat (or at least would like to) localStorage same as cookies. Are cookies also that hard to replace data?
Updated•11 years ago
|
Attachment #486377 -
Flags: superreview+
Attachment #486377 -
Flags: review+
Updated•11 years ago
|
Attachment #479219 -
Flags: superreview+
Attachment #479219 -
Flags: review+
Updated•11 years ago
|
Attachment #486334 -
Flags: review+
Comment 29•10 years ago
|
||
I don't like the "treat localStorage same as cookies" since localStorage can be used to other things than cookies.
Yes, some sites may use localStorage to store cookies and other identifiable data. I agree there should be an easy way to remove these when needed, but other uses may be affected as well, which may not be what the user wants.
For example, take a game that saves its game state to localStorage. Or a document editor that uses localStorage to store documents. When the user clears the history to remove some tracking cookies for privacy reasons, I am pretty certain the user might want to keep their save state in games or their documents, as these can contain valuable data to the user.
What's needed is probably out of scope for this bug, but it would be more optimal to have a view where the user can select what should be deleted, or just select all if the user doesn't care. If I understand correctly from multiple bug reports I've read so far, currently it's all or nothing, which can make it a hard decision, "Do I want to get rid of these tracking cookies, or do I want to keep my save states and documents?".
Currently it seems like localStorage is cleared when "cookies" are selected in the "Clear recent history", if this hasn't changed and I understand the other bug reports correctly. I don't really like that, and believe localStorage should be separate, and with time, a view for more finegrained control of what should be removed would be implemented. I would gladly look into it myself if I had the knowledge, but my knowledge of C++ isn't good at all and I never really been involved so I'm not the right person to do it.
Now I don't have any examples of games or apps that uses localStorage like this, but it is something I can envision apps and websites wanting to do. Therefore I don't really like this "treat localStorage as cookies", since localStorage isn't just cookies.
Comment 30•10 years ago
|
||
Why nothing can clean and delete these folders inside profile (unless manually)?
storage
|-persistent
|-http+++example.com
|-idb
|-.metadata
|-[...]
|-temporary
|-http+++example2.com
|-[...]
E.g. You may be downloading big files from Mega and something make the download fail and the big incomplete files will stay inside those folders forever.
Other sites create those folders for almost no reason, and more folders start accumulating there.
Imo, this bug is severe and too old already.
Comment 31•10 years ago
|
||
Yes, this is a problem. Sites like this
http://mothereff.in/regexpu
are using DOM Storage, so that means if I want to delete that data I have to
delete ALL my cookies. This is not desirable because some sites have useful
cookies, for example Google Search Settings.
Comment 32•10 years ago
|
||
This is a workaround
localStorage.clear();
http://stackoverflow.com/a/7667973
Updated•10 years ago
|
Summary: DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Recent History" is used with Time range not "Everything" → DOM Storage (localStorage, sessionStorage, indexedDB) data is not cleared when "Clear Recent History" is used with Time range not "Everything"
Updated•10 years ago
|
Component: General → DOM
Product: Firefox → Core
Summary: DOM Storage (localStorage, sessionStorage, indexedDB) data is not cleared when "Clear Recent History" is used with Time range not "Everything" → DOM Storage (localStorage, sessionStorage, indexedDB, asm.js cache) data is not cleared when "Clear Recent History" is used with Time range not "Everything"
Updated•10 years ago
|
Flags: qe-verify?
Comment 35•10 years ago
|
||
i don't understand why bug 1047098 and bug 602743 were marked as dups of this one.
Flags: needinfo?(dao)
Comment 36•10 years ago
|
||
I'm not even sure if bug 602743 was about localStorage or indexedDB; bug 602743 comment 5 implies the former, which makes it a duplicate of this bug even if we don't cover indexedDB here. If covering indexedDB here doesn't make sense, we should reopen bug 1047098. Finally, if bug 602743 was actually about indexedDB and bug 1047098 is reopened, bug 602743 should be made a duplicate of that.
Flags: needinfo?(dao)
Updated•10 years ago
|
Summary: DOM Storage (localStorage, sessionStorage, indexedDB, asm.js cache) data is not cleared when "Clear Recent History" is used with Time range not "Everything" → DOM Storage (localStorage, sessionStorage) data is not cleared when "Clear Recent History" is used with Time range not "Everything"
Updated•10 years ago
|
Priority: -- → P1
Comment 38•9 years ago
|
||
Bug 1125336, duplicated to this one, requests a separate entry in the clear history dialog for both DOM storage and indexedDB (two entries?). Is that a plan on the table here? It seems like a no-brainer to me, but doesn't really seem to have been discussed. Some reason why it shouldn't happen that way?
Comment 39•9 years ago
|
||
Just a comment from a user perspective, it would be nice if there was a way to clear the storage for a particular website / base url. I recently had an issue with some persistent cloudflare caching that i just couldnt get rid of, eventually tracked it down to them storing javascript in local storage to speed up the site load time. It would be really nice to be able to delete local storage for just the site i'm debugging.. fwiw.
blocking2.0: .x+ → ---
status1.9.1:
wanted → ---
status1.9.2:
? → ---
tracking-e10s:
? → ---
tracking-p11:
? → ---
Comment 40•9 years ago
|
||
Jan, I think this may rely on you QM work for DOM storage.
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
Updated•8 years ago
|
Comment 22 is private:
false
Comment 43•7 years ago
|
||
1398303 is not a duplicate as it refers to local storage not being cleaned when time range IS everything, while this bug refers to a time range other than everything.
Updated•7 years ago
|
Blocks: uplift_tor_fingerprinting
Whiteboard: [sg:want] → [sg:want][tor][fingerprinting]
Comment 44•7 years ago
|
||
A fix for this landed together with bug 1047098.
Status: NEW → RESOLVED
Closed: 14 years ago → 7 years ago
Resolution: --- → DUPLICATE
Comment 45•7 years ago
|
||
On following the activity of bug #1047098 I had the impression that the fix does only support cleaning everything and not supporting time ranges. Does the fix really support time ranges for cleaning? If not the duplicate status should maybe be removed.
Flags: needinfo?(amarchesini)
Comment 46•7 years ago
|
||
Or we can file a bug abut supporting time range for Clear Recent History. Note that currently we delete all, ignoring time range, for: localStorage, QuotaManager (indexedDB, ASMJS, DOM Cache), ServiceWorkers.
Flags: needinfo?(amarchesini)
Assignee | ||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•