Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[shared_preferences] On Web, exception on initialization when a stored value is not valid JSON #156574

Open
gdurandrexel opened this issue Oct 11, 2024 · 11 comments · May be fixed by flutter/packages#8211
Labels
p: shared_preferences Plugin to read and write Shared Preferences P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-web Web applications specifically team-web Owned by Web platform team triaged-web Triaged by Web platform team

Comments

@gdurandrexel
Copy link

Steps to reproduce

Create a web project.
In my case, use secure_storage to store some value. It is stored as plain text.

Expected results

The value that cannot be decoded should be ignored.

Actual results

When trying to initialize from this local storage:

image

I get this exception:

[2024-10-11 11:01:16.394 | Catcher 2 | INFO] ---------- ERROR ----------
[2024-10-11 11:01:16.396 | Catcher 2 | INFO] FormatException: SyntaxError: Unexpected token 'k', "k6IlhTmQIL"... is not valid JSON
[2024-10-11 11:01:16.396 | Catcher 2 | INFO]
[2024-10-11 11:01:16.397 | Catcher 2 | INFO] ------- STACK TRACE -------
[2024-10-11 11:01:16.737 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/errors.dart 296:3  throw_
[2024-10-11 11:01:16.737 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/convert_patch.dart 36:5          _parseJson
[2024-10-11 11:01:16.738 | Catcher 2 | INFO] dart-sdk/lib/convert/json.dart 610:36                                        convert
[2024-10-11 11:01:16.738 | Catcher 2 | INFO] dart-sdk/lib/convert/json.dart 216:41                                        decode
[2024-10-11 11:01:16.738 | Catcher 2 | INFO] packages/shared_preferences_web/shared_preferences_web.dart 262:37           _decodeValue
[2024-10-11 11:01:16.739 | Catcher 2 | INFO] packages/shared_preferences_web/shared_preferences_web.dart 135:22           _readAllFromLocalStorage
[2024-10-11 11:01:16.739 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54           runBody
[2024-10-11 11:01:16.739 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 127:5           _async
[2024-10-11 11:01:16.739 | Catcher 2 | INFO] packages/shared_preferences_web/shared_preferences_web.dart 129:55           [_readAllFromLocalStorage]
[2024-10-11 11:01:16.740 | Catcher 2 | INFO] packages/shared_preferences_web/shared_preferences_web.dart 126:12           getPreferences
[2024-10-11 11:01:16.740 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54           runBody
[2024-10-11 11:01:16.740 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 127:5           _async
[2024-10-11 11:01:16.740 | Catcher 2 | INFO] packages/shared_preferences_web/shared_preferences_web.dart 122:45           getPreferences
[2024-10-11 11:01:16.741 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 55:22          getAll
[2024-10-11 11:01:16.741 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54           runBody
[2024-10-11 11:01:16.741 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 127:5           _async
[2024-10-11 11:01:16.742 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 52:38          getAll
[2024-10-11 11:01:16.742 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 225:32         reloadCache
[2024-10-11 11:01:16.742 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54           runBody
[2024-10-11 11:01:16.743 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 127:5           _async
[2024-10-11 11:01:16.743 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 222:27         reloadCache
[2024-10-11 11:01:16.743 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 200:22         create
[2024-10-11 11:01:16.744 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 84:54           runBody
[2024-10-11 11:01:16.744 | Catcher 2 | INFO] dart-sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart 127:5           _async
[2024-10-11 11:01:16.744 | Catcher 2 | INFO] packages/shared_preferences/src/shared_preferences_async.dart 187:51         create

Code sample

Code sample
final sharedPreferences = await SharedPreferencesWithCache.create(
      cacheOptions: const SharedPreferencesWithCacheOptions());

Screenshots or Video

Screenshots / Video demonstration

[Upload media here]

Logs

Logs
[Paste your logs here]

Flutter Doctor output

Doctor output
[√] Flutter (Channel stable, 3.24.3, on Microsoft Windows [version 10.0.22631.4169], locale fr-FR)
    • Flutter version 3.24.3 on channel stable at c:\Vrac\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 2663184aa7 (4 weeks ago), 2024-09-11 16:27:48 -0500
    • Engine revision 36335019a8
    • Dart version 3.5.3
    • DevTools version 2.37.3

[√] Windows Version (Installed version of Windows is version 10 or higher)

[√] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at C:\Users\gdurand\AppData\Local\Android\Sdk
    • Platform android-34, build-tools 34.0.0
    • Java binary at: C:\Program Files\Android\Android Studio\jbr\bin\java
    • Java version OpenJDK Runtime Environment (build 17.0.10+0--11609105)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop Windows apps (Visual Studio Community 2022 17.11.5)
    • Visual Studio at C:\Program Files\Microsoft Visual Studio\2022\Community
    • Visual Studio Community 2022 version 17.11.35327.3
    • Windows 10 SDK version 10.0.22621.0

[√] Android Studio (version 2024.1)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.10+0--11609105)

[√] IntelliJ IDEA Community Edition (version 2020.3)
    • IntelliJ at C:\Program Files\JetBrains\IntelliJ IDEA Community Edition 2020.1.1
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart

[√] VS Code (version 1.94.1)
    • VS Code at C:\Users\gdurand\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.98.0

[√] Connected device (4 available)
    • SM G930U (mobile) • b86bd847 • android-arm64  • Android 8.0.0 (API 26)
    • Windows (desktop) • windows  • windows-x64    • Microsoft Windows [version 10.0.22631.4169]
    • Chrome (web)      • chrome   • web-javascript • Google Chrome 129.0.6668.90
    • Edge (web)        • edge     • web-javascript • Microsoft Edge 129.0.2792.65

[√] Network resources
    • All expected network resources are available.

• No issues found!
@gdurandrexel gdurandrexel changed the title On Web, exception on initialization when a stored value is not valid JSON [shared_preferences] On Web, exception on initialization when a stored value is not valid JSON Oct 11, 2024
@danagbemava-nc danagbemava-nc added the in triage Presently being triaged by the triage team label Oct 11, 2024
@danagbemava-nc
Copy link
Member

Hi @gdurandrexel, please provide a complete minimal reproducible code sample so that we can properly investigate this.

Please also provide the version of shared_preferences & shared_preferences_web you're using.

Thank you

@danagbemava-nc danagbemava-nc added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 11, 2024
@gdurandrexel
Copy link
Author

I use version 2.2.2.

As for a reproducer:

  • create a web project,
  • put the code above in main(),
  • create a plain text key in the local storage from Chrome
  • launch

The problem is obvious in the code of packages/shared_preferences_web/shared_preferences_web.dart 262:37: json.decode() is called on the retrieved value, which fails because it is not a json value

@github-actions github-actions bot removed the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Oct 11, 2024
@stuartmorgan
Copy link
Contributor

stuartmorgan commented Oct 11, 2024

Is there a reason you are not passing an allowList as part of your options? Not doing so means you are explicitly asking for SharedPreferencesWithCache to cache every value. Dropping some values silently in that scenario is actually not unambiguously correct.

That said, we do filter unsupported types on iOS/macOS, so silent filtering would be consistent, and from discussion in flutter/packages#5813 I think there's consensus that it's the better option. Anyone interested in completing flutter/packages#5813 is welcome to contribute a new PR.

@stuartmorgan stuartmorgan added platform-web Web applications specifically p: shared_preferences Plugin to read and write Shared Preferences package flutter/packages repository. See also p: labels. team-web Owned by Web platform team and removed in triage Presently being triaged by the triage team labels Oct 11, 2024
@gdurandrexel
Copy link
Author

Using allowList is cumbersome: I have to track all the keys I use with no real benefit for me.
The prefix system used by the previous version was much less effort. I would prefer to have that in-lieu or in addition to allowList.

A more general thought: if the package uses a shared storage and does not support all the data types of the storage, it should probably handle gracefully the unsupported data types by default. Asking the user to setup this mechanism explicitly with allowList or prefix or try/catch does not seem right.

Also, if for whatever reason a value is not in the expected format, then the whole initialisation fails. In my case I prefer having some of the values (which should be all the ones I created with this package) than none at all.

@yjbanov yjbanov added P2 Important issues not at the top of the work list triaged-web Triaged by Web platform team labels Oct 24, 2024
@Dan-Crane
Copy link
Contributor

@gdurandrexel you found a workaround?

@gdurandrexel
Copy link
Author

In one project I just used SharedPreferences instead of SecureStorage (which was creating a plain text entry) to avoid having plain text entries.
In another I used allowedList. This is what should be used for now as it is the safest solution.

@gdurandrexel
Copy link
Author

To go futher with this, the meaning of a null allowList should be clearly defined (if allowed at all):

  • option 1: read all the values in the storage. I don't understand why this would be desirable. In this case the package should make an effort to read all the values, whatever their format.
  • option 2: read all the values managed by SharedPreferences. In this case, store the values with a discriminant allowing to know what values are managed by SharedPreferences (using a prefix for instance), and read only these values. Personnally I would be OK with reading all readable values whatever their source, but that should be documented as it is not obvious.

In any case, the initialization must not fail.

@stuartmorgan
Copy link
Contributor

  • option 2: read all the values managed by SharedPreferences. In this case, store the values with a discriminant allowing to know what values are managed by SharedPreferences (using a prefix for instance), and read only these values.

The reason the new APIs don't do that is that there was significant feedback over the years with the old API that people wanted to be able to access values that were not stored by the plugin, especially in add-to-app or migration-from-non-Flutter use cases. The lack of automatic prefixing is an intentional change.

@stuartmorgan
Copy link
Contributor

In any case, the initialization must not fail.

As I said in my earlier comment:

Anyone interested in completing flutter/packages#5813 is welcome to contribute a new PR.

@Dan-Crane
Copy link
Contributor

@stuartmorgan I plan to create a PR in the near future. To do this, I need to figure out how to write integration tests.

@Dan-Crane
Copy link
Contributor

@stuartmorgan I did it. If you don't mind, could you take a look, please?

flutter/packages#8211

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: shared_preferences Plugin to read and write Shared Preferences P2 Important issues not at the top of the work list package flutter/packages repository. See also p: labels. platform-web Web applications specifically team-web Owned by Web platform team triaged-web Triaged by Web platform team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants