fix: use BigInteger for OAuthAccount.expires_at to avoid 32-bit overflow - #27
fix: use BigInteger for OAuthAccount.expires_at to avoid 32-bit overflow#27SAY-5 wants to merge 2 commits into
Conversation
tapetersen
left a comment
tapetersen
left a comment
There was a problem hiding this comment.
I'm not a maintainer but while the change seems reasonable I'm not sure the test is.
| assert unknown_oauth_user is None | ||
|
|
||
|
|
||
| def test_oauth_account_expires_at_is_big_integer(): |
There was a problem hiding this comment.
This test does nothing more than double check a declaration. If a test is warranted it should verify it corrects the original issue, that it accepts dates beyond 2038
There was a problem hiding this comment.
Fair point, replaced it with a round trip that writes 2147483648 and reads it back, so it actually exercises the overflow instead of restating the column type.
There was a problem hiding this comment.
Looks good to me but again this was just a drive by suggestion and I have no idea if the original maintainer will do any further updates or releases.
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
|
Fair point, swapped it for a round trip that stores expires_at = 2147483648 and reads it back. That one actually fails on the postgres and mysql legs of the matrix without the BigInteger change, sqlite would pass either way since its INTEGER is already 64 bit. |
2 participants
Some OAuth providers return
expires_atvalues beyond 2038 (e.g. ORCID sandbox returns 2043), which overflow a 32-bitINTEGERcolumn and cause the insert to fail. Switching the column toBigIntegerfixes this on PostgreSQL, MySQL/MariaDB, etc. while remaining a no-op for SQLite.Closes #1296