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)

x86
All
defect

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)

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.
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
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"
blocking2.0: --- → ?
Component: DOM: Other → General
Product: Core → Firefox
QA Contact: general → general
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
When this gets blocking+, I can work on this.
Assignee: nobody → honzab.moz
blocking2.0: ? → betaN+
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).
Whiteboard: [sg:want]
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.
Blocks: 599724
Depends on: 600366
Attached patch v1 [Backed out due to oranges] (obsolete) (deleted) — Splinter Review
- 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)
Depends on: 536544
Comment on attachment 479219 [details] [diff] [review] v1 [Backed out due to oranges] r=jst!
Attachment #479219 - Flags: review?(jst) → review+
Attachment #479219 - Flags: superreview+
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
Attachment #479219 - Attachment description: v1 → v1 [Check in comment 10]
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
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 → ---
Status: REOPENED → ASSIGNED
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
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.
Attachment #479219 - Attachment description: v1 [Check in comment 10] → v1 [Backed out due to oranges]
Attached patch v1 update idiff to fix the orange (obsolete) (deleted) — Splinter Review
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)
Attachment #486334 - Flags: review?(jst) → review+
Attached patch v1.1 [Backed out comment 16] (deleted) — Splinter Review
Attachment #479219 - Attachment is obsolete: true
Attachment #486334 - Attachment is obsolete: true
Attachment #486377 - Flags: superreview+
Attachment #486377 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → FIXED
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 → ---
Attachment #486377 - Attachment description: v1.1 as landed [Check in comment 15] → v1.1 [Backed out comment 16]
Status: REOPENED → ASSIGNED
This ~could~ be related to bug 600366. We should then fix it before landing this bug.
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
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+ → -
blocking2.0: - → .x
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.
OS: Windows XP → All
It appears the sessionStorage is not being removed at all by the cookie manager removeAll() method (via the "cookie-changed" "cleared" observer notification).
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)
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)
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?
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.
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).
Hmm.. we treat (or at least would like to) localStorage same as cookies. Are cookies also that hard to replace data?
Attachment #486377 - Flags: superreview+
Attachment #486377 - Flags: review+
Attachment #479219 - Flags: superreview+
Attachment #479219 - Flags: review+
Attachment #486334 - Flags: review+
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.
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.
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.
This is a workaround localStorage.clear(); http://stackoverflow.com/a/7667973
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"
Blocks: 1102808
Flags: firefox-backlog+
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"
Flags: qe-verify?
i don't understand why bug 1047098 and bug 602743 were marked as dups of this one.
Flags: needinfo?(dao)
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)
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"
Priority: -- → P1
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?
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.
tracking-e10s: --- → ?
tracking-p11: --- → ?
blocking2.0: .x+ → ---
status1.9.2: ? → ---
tracking-e10s: ? → ---
tracking-p11: ? → ---
Flags: needinfo?(nwmoussa)
Jan, I think this may rely on you QM work for DOM storage.
Assignee: honzab.moz → nobody
Status: ASSIGNED → NEW
Blocks: 857888
Comment 22 is private: false
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.
A fix for this landed together with bug 1047098.
Status: NEW → RESOLVED
Closed: 14 years ago7 years ago
Resolution: --- → DUPLICATE
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)
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)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: