This PR fixes a bug with int32 encoding. Bug was triggered with negative int32 values, when we insert them and then select with text encoding.
There were two ways of fixing this bug:
Fix insertion (which is implemented here)
Acra normalizes integers into int64 text before encrypting and sending into the database. The normalization happens here.
For an int32 the whole process looked like:
[4 bytes] -> uint32 -> int64 -> text
But the issue is, expanding from uint32 into int64 happens signless. So, if the integer is -2061435539 the whole process would looks like:
But, this will not work with the values encrypted before this change, the bug can still remain in the database of some users
Fix selection
This is the fix I was first thinking of, and even implemented that, so if you believe this way should be used instead/with the previous one, it can be easily applied.
When we encode integer values in postgres, we create IntValue object, defined here. In consists of an int value and a string one. String is used as an optimization, because the whole int value is produced from string, so in case of text encoding, we can reuse that string.
But because strings are signless for int32 (see reasons above), this doesn't work. The way to correct is to explicitly calculate string representation based on an integer value.
I don't know which variant is better, so I'm here for your opinion.
This PR fixes a bug with int32 encoding. Bug was triggered with negative int32 values, when we insert them and then select with text encoding.
There were two ways of fixing this bug:
Fix insertion (which is implemented here)
Acra normalizes integers into int64 text before encrypting and sending into the database. The normalization happens here. For an int32 the whole process looked like:
But the issue is, expanding from uint32 into int64 happens signless. So, if the integer is
-2061435539the whole process would looks like:This PR adds explicit conversion from uint32 into int32, so the compiler can omit correct instruction for expansions with a negative sign:
But, this will not work with the values encrypted before this change, the bug can still remain in the database of some users
Fix selection
This is the fix I was first thinking of, and even implemented that, so if you believe this way should be used instead/with the previous one, it can be easily applied.
When we encode integer values in postgres, we create
IntValueobject, defined here. In consists of an int value and a string one. String is used as an optimization, because the whole int value is produced from string, so in case of text encoding, we can reuse that string.But because strings are signless for int32 (see reasons above), this doesn't work. The way to correct is to explicitly calculate string representation based on an integer value.
I don't know which variant is better, so I'm here for your opinion.
Checklist