2 ms·

*edit, I think this issue was specific to the Android client, the desktop client has a totally different sqlite schema. The child comment to your comment is de

*edit, I think this issue was specific to the Android client, the desktop client has a totally different sqlite schema. The child comment to your comment is deleted, but I think autoincrement IDs shouldn't be used under an ambient authority context. It would make more sense to have IDs based on an LSF or Feistel sequence, perhaps split into a master ID and a conversation sequence. Autoincrement on this field makes it easy for off by one errors. Even just moving to guids and maintaining a proper parent child relationship would have prevented this. Or maybe there should be a database per conversation (set of all parties). https://github.com/signalapp/Signal-Android/commit/83086a5a2b8cae3ac40dea75d8c9533457a31858 https://github.com/signalapp/Signal-Android/commit/83086a5a2... https://github.com/signalapp/Signal-Android/commit/b9657208fea1c7bcfb90b7400037a7858ba56516 https://github.com/signalapp/Signal-Android/commit/b9657208f... Row IDs shouldn't have so much power.

WesolyKubeczek 5y ago
The good thing about autoincrementing unsigned 64-bit integers is that 1) it's insanely fast, 2) SQLite is doing it automatically, 3) seriously, why don't they have it on all tables yet? SQLite guarantees no collisions within a table. Doing homegrown ID generation is how such bugs are being introduced in the first place. Your application level trigger got bypassed, oops. Your check was experimentally disabled and left like that for a year until people started noticing, oops. If you make an SQLite-backed application and it has a bug like this, I can safely bet $500 it's not going to be SQLite that has the bug. Just don't expose the numeric IDs in links, and generate your UUIDs in tables where you need to point to rows in an outside-accessible link. This is database 101, for crying out loud.

2 ms·

*edit, I think this issue was specific to the Android client, the desktop client has a totally different sqlite schema. The child comment to your comment is de

by seg_lol 5y ago

WesolyKubeczek 5y ago
The good thing about autoincrementing unsigned 64-bit integers is that 1) it's insanely fast, 2) SQLite is doing it automatically, 3) seriously, why don't they have it on all tables yet? SQLite guarantees no collisions within a table. Doing homegrown ID generation is how such bugs are being introduced in the first place. Your application level trigger got bypassed, oops. Your check was experimentally disabled and left like that for a year until people started noticing, oops. If you make an SQLite-backed application and it has a bug like this, I can safely bet $500 it's not going to be SQLite that has the bug. Just don't expose the numeric IDs in links, and generate your UUIDs in tables where you need to point to rows in an outside-accessible link. This is database 101, for crying out loud.