mogol / flutter_secure_storage

A Flutter plugin to store data in secure storage
https://pub.dartlang.org/packages/flutter_secure_storage
BSD 3-Clause "New" or "Revised" License
1.09k stars 340 forks source link

[bug] Race condition on `write` and `read`. #716

Closed PROGrand closed 1 month ago

PROGrand commented 1 month ago

Problem

Race condition on write and read.

There is FlutterSecureStorage operations async bug:

Future<void> main() async {
  WidgetsFlutterBinding.ensureInitialized();
  const storage = FlutterSecureStorage();
  await storage.write(key: 'key', value: 'value');
  assert(await storage.read(key: 'key') == 'value'); // assertion: platform [read] finishes before platform implementation [write].
}
Daniel-Cong commented 1 month ago

This is a feature / good thing, not a bug. By making it synchronous by default, we can go back to responding to user, while the write operation is continuing in the background. Also you can always add await on your own code if you want it to be blocking. Why are you reading said value right away anyway after writing it? I have never met such scenario in my time writing flutter app.

hantrungkien commented 1 month ago

@Daniel-Cong what do you think about this case:

  1. When user want to logout
  Future<void> logout() async {
    await _secureStorage.delete(key: _isLoggedInKey);
    _isLoggedInSubject.add(false);
  }
  1. _isLoggedInSubject will notify to GoRouter.refreshListenable to invoke GoRouter.redirect

  2. In GoRouter.redirect need to directly call _secureStorage.read to determine logged in or not. However, the result from the read function may be true even though it was deleted in step 1. This causes the router to not redirect as expected.

redirect: (context, state) async {
      final path = state.uri.path;
      final isLoggedIn = (await _secureStorage.read(key: _isLoggedInKey)) != null; // for example
      if (isLoggedIn) {

      } else {

      }
    },