Bug 157010 – QuotaExceededError when saving to localStorage in private mode

• Component: Platform
• Status: RESOLVED FIXED

[Dima Voytenko]

`localStorage.setItem` fails in private mode with `QuotaExceededError`. It looks the original bug item for this was https://bugs.webkit.org/show_bug.cgi?id=49329

This is now a clear and "recommended" way for site owners to detect user's private browsing preferences and could be considered a significant privacy issue. See for instance:

http://apple.stackexchange.com/questions/131587/how-can-a-web-site-determine-if-safari-private-browsing-is-turned-on
http://stackoverflow.com/questions/9659103/how-to-detect-users-on-an-iphone-with-private-browsing-enabled

It looks like Safari is the last remaining major browser that still allows leaking of private mode via this approach. The original bug item cited above does not discuss privacy implications. Is it possible to address?

[Rick Byers]

Yeah this is probably bad, will likely lead to some sketchy websites refusing to work (or otherwise complaining to the user) in private mode.  I just filed a related issue on blink: https://crbug.com/606633

[Radar WebKit Bug Importer]

<rdar://problem/25947768>

[Brady Eidson]

I agree this is less than ideal.

However, "pretending to store something permanently" sounds pretty bad, too.

Perhaps you have ideas on how we could both:
1 - Maintain the contract that localStorage is durable
2 - Not store things on localStorage in private browsing?

[Dima Voytenko]

I believe other browsers simply make localStorage equivalent sessionStorage.

[Brady Eidson]

(In reply to comment #4)
> I believe other browsers simply make localStorage equivalent sessionStorage.

Not entirely accurate, as I assume it's still global...

When we implement an API we try to think it through and reflect on the contract it represents.

localStorage is supposed to be persistent.

So, yes, I already understood that other browsers ignore that constraint.

[Dima Voytenko]

The issue with just ignoring writes is that it's no different really from throwing an exception from the point of view of private mode leak. E.g.

```
function isPrivateMode() {
  try {
    localStorage.setItem('a', 'b');
    return false;
  } catch(e) {
    return true;
  }
}
```

vs

```
function isPrivateMode() {
  localStorage.setItem('a', 'b');
  return localStorage.getItem('a') != 'b';
}
```

[Dima Voytenko]

Also, re:permanent storage. From the page author's point of view, they can only judge permanence of storage per user, which they can alternatively track via cookies. So, resetting localStorage when cookies are reset (as when they close private mode) makes sense.

[Rick Byers]

Right, in chromium I believe the model is that incognito mode is like a separate user profile that is automatically removed when the window is closed.  This doesn't break the API contract around "durability" any more than a user explicitly clearing their browser state (the next visit is like a new user again).

[Darin Adler]

We do understand that Chrome’s Incognito model is "temporary user profile that is removed when window is closed".

Our concern for the Private Browsing features that pre-date Incognito was that if a website stored important, document-like data in localStorage, this because could be a problem, not for the website developer, but for the web browser user. Even though the person is using a private browsing mode they might not understand that closing a window or quitting the web browser has a side effect of deleting all their documents. It seemed that getting an error earlier would help prevent someone from building up some work that then is unexpectedly lost.

In practice, the WebKit engineers may be misunderstanding the type of data that websites would store in localStorage. Or misunderstanding the expectations people will bring to private browsing features.

[Brady Eidson]

(In reply to comment #6)
> The issue with just ignoring writes is that it's no different really from
> throwing an exception from the point of view of private mode leak. E.g.
> 
> ```
> function isPrivateMode() {
>   try {
>     localStorage.setItem('a', 'b');
>     return false;
>   } catch(e) {
>     return true;
>   }
> }
> ```
> 
> vs
> 
> ```
> function isPrivateMode() {
>   localStorage.setItem('a', 'b');
>   return localStorage.getItem('a') != 'b';
> }
> ```

I never suggested ignoring writes. Of course that's silly.

[Brady Eidson]

(In reply to comment #7)
> Also, re:permanent storage. From the page author's point of view, they can
> only judge permanence of storage per user, which they can alternatively
> track via cookies. So, resetting localStorage when cookies are reset (as
> when they close private mode) makes sense.

We thought about this very carefully when we implemented localStorage for the browser before anybody but the Gears plugin

Cookies are a completely different class of storage from localStorage.

Browsers are allowed to make all cookies session cookies, ditch cookies in FIFO order to make room for new ones, ditch big cookies first, ditch cookies in any arbitrary order, decide not to store a cookie "just because it looks weird", etc etc, and still match the spirit of the spec.

LocalStorage was meant to be durable.

All of this said, it's a shame that none of the other browsers considered this the same way.

Note, I'm not *defending* how our behavior reveals private browsing state. Of course that's terrible.

I am lamenting that nobody worked together to try to come up with something better.

[Brady Eidson]

Let me posit this:

If the localStorage area is legitimately full, we're legitimately expected to throw a QuotaExceededError exception.

How can we possibly distinguish that from the "oh, this is webkit in private browsing" detection that is rampant?

[Dima Voytenko]

> If the localStorage area is legitimately full, we're legitimately expected to throw a QuotaExceededError exception.

This probably has to do with probability. I assume page authors simply judge that "if it's a Safari and localStorage.setItem() fails with QuotaExceededError, then it's most likely a private mode". As in "false positives are possible, but rare enough". Obviously, this probably makes things a bit worse for the user.

> nobody worked together to try to come up with something better

I'm not super familiar with the standards process for this kind of spec. It seems to me that in this day and age some discussion of how an API should behave in private mode has to be part of the spec. My quick search shows that it isn't at this time which is disappointing. Would you consider talking to the spec authors to hear their recommendations?

[Dima Voytenko]

For posterity, I filed https://github.com/whatwg/html/issues/1143

[Brady Eidson]

(In reply to comment #13)
> 
> Would you consider talking to the spec authors to hear their recommendations?

I was one of the spec authors. Not listed as an editor, but plenty of feedback given while implementing.

But, it's true - back in 2007-2008 we didn't automatically talk about private browsing, as Safari was one of the few that had it, and IE, for example, didn't.

[Dima Voytenko]

Awesome! I posted above, I filed a bug against the spec: https://github.com/whatwg/html/issues/1143

Looking forward hearing what you decide.

[test]

Safari 10 and Safari Technology Preview 8 not fixed

[Blaze Burg]

Per discussion with Brady, the fix will be to make localStorage in-memory for ephemeral sessions. This means that in private browsing and automation (i.e., WebDriver), we'll allow read/write and the contents do not persist.

[Dima Voytenko]

Awesome! Thanks a lot!

[Brady Eidson]

Created attachment 306941 [details]
Patch

[Alex Christensen]

Comment on attachment 306941 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=306941&action=review

> Source/WebCore/loader/EmptyClients.cpp:680
> +RefPtr<StorageNamespace> EmptyStorageNamespaceProvider::createEphemeralLocalStorageNamespace(Page&, unsigned)

Why doesn't this return a Ref?

> Source/WebKit/Storage/StorageNamespaceImpl.cpp:103
> +    for (auto iter : m_storageAreaMap)

Do we need auto& here?

> Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.cpp:105
> +    RefPtr<EphemeralStorageArea> copy()

Ref

[Brady Eidson]

(In reply to Alex Christensen from comment #21)
> Comment on attachment 306941 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306941&action=review
> 
> > Source/WebCore/loader/EmptyClients.cpp:680
> > +RefPtr<StorageNamespace> EmptyStorageNamespaceProvider::createEphemeralLocalStorageNamespace(Page&, unsigned)
> 
> Why doesn't this return a Ref?

Because the others don't, and I didn't want to expand the scope of the patch.

> 
> > Source/WebKit/Storage/StorageNamespaceImpl.cpp:103
> > +    for (auto iter : m_storageAreaMap)
> 
> Do we need auto& here?

Sure.

> 
> > Source/WebKit2/WebProcess/Storage/StorageNamespaceImpl.cpp:105
> > +    RefPtr<EphemeralStorageArea> copy()
> 
> Ref

We'll see.

[Brady Eidson]

Created attachment 306946 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 306946 [details]
Patch

Clearing flags on attachment: 306946

Committed r215315: <http://trac.webkit.org/changeset/215315>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[David Kilzer (:ddkilzer)]

<rdar://problem/19197190>

[Mahaveer]

I have updated my mac OS X EI Capitan(10.11.6) safari version to 10.1.2(11603.3.8) but still in browser private mode getting the same error. Can anybody please tell me how could i resolve this?

[Brady Eidson]

(In reply to Mahaveer from comment #27)
> I have updated my mac OS X EI Capitan(10.11.6) safari version to
> 10.1.2(11603.3.8) but still in browser private mode getting the same error.
> Can anybody please tell me how could i resolve this?

AFAIK, nobody claimed this would be fixed on El Cap 10.11.6 or in Safari 10.1.2 on any platform.

Have you tried in a WebKit nightly, Safari Technology Preview, or a macOS High Sierra beta?