lewisl9029 / toc

Toc Messenger - A distributed messaging app that can sync across all of your devices
http://toc.im
GNU Affero General Public License v3.0
56 stars 1 forks source link

Removing objects through state service sometimes results in non-intuitive behavior #233

Open lewisl9029 opened 9 years ago

lewisl9029 commented 9 years ago

This is because the abstraction between the in-memory baobab tree and the remotestorage module is a leaky one: The baobab tree is hierarchical, while we're using remotestorage as a flat key-value store (probably against best practices, but this was necessary to enable encryption of storage keys in addition to values).

If we save using

state.save(state.cloud.notifications, [notificationId, 'notificationInfo'], {})

We cannot remove it by calling remove on the parent key using

state.remove(state.cloud.notifications, [notificationId])

because there is no such key in remotestorage, as only the path storage.getStorageKey(state.cloud.notifications, [notificationId, 'notificationInfo']) exists, so the removal will fail.

Instead we must call

state.remove(state.cloud.notifications, [notificationId, 'notificationInfo'])

This could become more problematic later on as we make more use of state.remove and when we need the ability to remove entire subpaths at once. Consider switching to non-encrypted, hierarchical storage keys that map 1:1 with the underlying baobab tree.

lewisl9029 commented 9 years ago

Actually they can't map 1:1 yet unless we refactor state.save to break down objects into individual properties and save each separately.

This would make state service more robust but increase storage requirements. Might possibly reduce the effectiveness of the crypto since some values can be very short and might make brute forcing easier if they were split up into separate properties (possibly not due to the length of the IV)?