blitz icon indicating copy to clipboard operation
blitz copied to clipboard

Unguarded localStorage access causes uncaught crashes when dealing with auth

Open tmcw opened this issue 3 years ago • 2 comments

What is the problem?

People using browsers with security-paranoid configurations have some unusual localStorage behavior. You either get:

  • Chrome on Android in private mode or with paranoid extensions = no localStorage object at all. localStorage is null.
  • Firefox in private mode: localStorage exists, but if you touch it, you get an error

So: the essence is that Blitz's auth system assumes that localStorage both exists and works, and that is not a safe assumption. In production, this leads to uncaught errors.

This was reported in https://github.com/blitz-js/legacy-framework/issues/111, but only touched one instance. There are several more localStorage accesses.

Paste all your error logs here:

TypeError: Cannot read properties of null (reading 'getItem')
  at getToken(./node_modules/next/dist/data-client/auth.js:97:33)
  at getData(./node_modules/next/dist/data-client/auth.js:84:38)
  at updateState(./node_modules/next/dist/data-client/auth.js:76:80)
  at new h(./node_modules/next/dist/data-client/auth.js:105:18)
  at getPublicDataStore(./node_modules/next/dist/data-client/auth.js:121:36)
  at c(./node_modules/next/dist/stdlib/blitz-app-root.js:109:43)
  at Nh(./node_modules/react-dom/cjs/react-dom.production.min.js:166:137)
  at Lk(./node_modules/react-dom/cjs/react-dom.production.min.js:289:386)
  at Kk(./node_modules/react-dom/cjs/react-dom.production.min.js:279:379)
  at Jk(./node_modules/react-dom/cjs/react-dom.production.min.js:279:310)
  at yk(./node_modules/react-dom/cjs/react-dom.production.min.js:279:170)
  at Dk(./node_modules/react-dom/cjs/react-dom.production.min.js:270:443)
  at wk(./node_modules/react-dom/cjs/react-dom.production.min.js:268:421)
  at J(./node_modules/react-dom/node_modules/scheduler/cjs/scheduler.production.min.js:13:203)
  at MessagePort.T(./node_modules/react-dom/node_modules/scheduler/cjs/scheduler.production.min.js:14:128)

Paste all relevant code snippets here:

(this is part of the default flow with Blitz)

What are detailed steps to reproduce this?

  1. Use Firefox in private mode or Chrome in private mode on Android

Run blitz -v and paste the output here:

blitz: 0.45.4 (local)

  Package manager: yarn
  System:
    OS: macOS 12.3.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 66.32 MB / 16.00 GB
    Shell: 5.8.1 - /usr/local/bin/zsh
  Binaries:
    Node: 16.13.1 - ~/n/bin/node
    Yarn: 1.22.4 - ~/.yarn/bin/yarn
    npm: 6.14.15 - /usr/local/bin/npm
    Watchman: Not Found
  npmPackages:
    @prisma/client: ~3.12 => 3.12.0
    blitz: 0.45.4 => 0.45.4
    prisma: ~3.12 => 3.12.0
    react: ^18.0.0 => 18.0.0
    react-dom: ^18.0.0 => 18.0.0
    typescript: ~4.6 => 4.6.3

Please include below any other applicable logs and screenshots that show your problem:

No response

tmcw avatar Apr 24 '22 14:04 tmcw

Thanks for the issue. What would be the desired behaviour if we get rid of the assumption that LS exists?

beerose avatar Jun 22 '22 11:06 beerose

Thanks! I think that desired behavior would be catching exceptions around localStorage access, and treating this crash the same way you'd treat the key as not existing in localStorage.

It's a sort of rare configuration and one that honestly blows up a lot of websites anyway, but crashes from this bubble up to the application layer, which is annoying.

tmcw avatar Jun 22 '22 13:06 tmcw