JSON strings don't survive `setItem`/`getItem` roundtrip

[ms-tng]

Describe the bug When saving a string that is valid JSON with setItem, then loading it with getItem, the returned value isn't the original string, but the value obtained by parsing it as JSON.

To Reproduce Run the following test:

const storage = new EncryptStorage('verysecret');
const value = '{}';
storage.setItem('foo', value);
expect(storage.getItem('foo')).toEqual(value);

The test fails, also with other JSON values such as 42, "a string", true, [] and null.

Expected behavior getItem should reproduce the original string saved using setItem.

Desktop:

• OS: Windows
• Browser: Chrome
• Version: 124.0.6367.119

Additional context This behavior occurs because values with a type different from object (e.g. string) aren't JSON.stringify'd by setItem, but saved as is. getItem then tries to parse those values as JSON and returns the original string only if parsing fails. Now if the original string happens to be valid JSON, this roundtrip doesn't work. I guess it would make sense to always JSON.stringify in setItem, even for primitive values.

[michelonsouza]

Good morning @ms-tng, This happens because the library performs the data conversion internally.

For this test to pass effectively, its JSON.stringfy would have to be redone, as shown below.

const storage = new EncryptStorage('verysecret');
const value = '{}';
storage.setItem('foo', value);
const parsedValue = JSON.stringfy(storage.getItem('foo'));
expect(parsedValue).toEqual(value);

At first, it may seem like a bug.

But it is the nature of the library to return converted data, including typed data, as demonstrated in the documentation.

I'm going to add something to the options so that it doesn't parse the values and that it's up to the user to do JSON.stringfy and JSON.parse in the next version.

Hope this helps.

If you have any questions or problems, do not hesitate to respond to the issue.

And sorry for the delay.

[ms-tng]

Hi @michelonsouza, thanks for the explanation!

I understand that the library is supposed to deal with any kind of data in a typed way, and so it applies JSON.stringify when saving (except for strings, which is a problem - see below) and JSON.parse when loading. That's why saving and loading a non-string value such as {} works perfectly fine:

const storage = new EncryptStorage('verysecret');
const value = {};
storage.setItem('foo', value);
expect(storage.getItem('foo')).toEqual(value); // passes

(note that value is {} in this example, not '{}').

Now if we want the library to really be able to deal with any value, it must also be able to deal with strings, and indeed it does:

const storage = new EncryptStorage('verysecret');
const value = 'some string';
storage.setItem('foo', value);
expect(storage.getItem('foo')).toEqual(value); // passes

However, in this case it shouldn't matter what the content of the string is, so 'some string' and '{}' (note the quotes) should both work the same. But '{}' fails as demonstrated by my initial example above. The issue is not that I want to do the serialization and deserialization myself - the library does that for me just fine. The point is that I also want to be able to save and load arbitrary strings, regardless of whether they happen to be valid JSON.

Imagine the following use case, which should be quite common: You want to save a password (which is an arbitrary string) entered by the user in local storage in encrypted form. Now the if the user chooses abcd as their password, everything works fine, but if they choose "abcd" (just randomly including quotes to make it harder to guess), it fails because the quotes get lost along the way.

Your suggestion on how to make the test pass doesn't work in general, because I would either have to apply JSON.stringify or not depending on what the original value was, but I don't know that, because e.g. 'abcd' and '"abcd"' produce the same value when going through the save/load roundtrip. The problem is that the library saves string values verbatim, relying on the assumption that when loading them later, they will fail to parse and then be returned verbatim, which is just not true in general, because they could successfully parse as JSON even though they were originally just strings.

Wouldn't it be very easy to fix (and even make the code simpler) by just not treating strings in a special way, but instead just always applying JSON.stringify when saving and applying JSON.parse when loading (which would then always succeed)? Then,

• {} would be saved as '{}' and loaded back as {}, just as it is now
• '{}' would be saved as "'{}'" and loaded as '{}'
• "abcd" would be saved as "\"abcd\"" and loaded as "abcd" and so on.

Anyway, it doesn't block me because I can solve my password use case by just saving an object {password: 'abcd'} instead of just the password itself, which avoids the special handling of strings.

[michelonsouza]

In the new version that I have just published, 2.13.02, there is an option so that this does not happen and it is up to the developer to parse it. HERE

For your problem, I did not create type tests to assess these cases. Because if it is not possible to perform the conversion, it returns the data from the jet that was in storage.

I'm sorry for this complication. It's just that JSON.parse of '{}' is an object... Both numbers and '[]' are valid for Parse, that's why they are converted. Which complicates this comparison.

Sorry for the English, I'm Brazilian and I'm using Google translate.

[ms-tng]

Okay then, thanks for your efforts, that should be sufficient for now. And thanks for your work on this library!