Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integer overflow fixes #120

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Integer overflow fixes #120

merged 4 commits into from
Jul 30, 2020

Conversation

tanner0101
Copy link
Member

@tanner0101 tanner0101 commented Jul 29, 2020

Deprecates PostgresData integer conversion methods that could lead to overflow errors (#120, fixes #119).

Using the following types in release-mode should no longer be susceptible to overflow (or underflow) crashes:

  • UInt
  • Int8
  • UInt16
  • UInt32
  • UInt64

⚠️ However, these types will still be interpreted incorrectly by Postgres since it doesn't have native support for them. This could create problems with other clients connecting to the database. To prevent such issues, using these types will now result in a debug-mode only assertion.

To migrate away from these types, there are two options:

1: Use a wider integer (or type) that does not overflow or underflow.

For example:

  • Int8 -> Int16
  • UInt16 -> Int32
  • UInt32 -> Int (Int64)
  • UInt / UInt64 -> String

The caveats with this method are increased storage size and a database migration is required.

2: Do an explicit bitPattern conversion to the inversely signed type with same bit width.

For example, using Fluent:

// Store the value as `Int32` in Postgres
@Field(key: "foo")
var _foo: Int32

// Access the value as if it were a `UInt32`. 
var foo: UInt32 {
    get {
        .init(bitPattern: self._foo)
    }
    set {
        self._foo = .init(bitPattern: newValue)
    }
}

The caveat with this method is that other clients may misinterpret the stored value.

@tanner0101 tanner0101 added enhancement New feature or request semver-minor labels Jul 29, 2020
@tanner0101 tanner0101 added this to Awaiting Review in Vapor 4 via automation Jul 29, 2020
@tanner0101
Copy link
Member Author

Idea for making these APIs more obvious in the next major version: #121

@tanner0101 tanner0101 marked this pull request as ready for review July 30, 2020 01:26
@tanner0101 tanner0101 merged commit 70d63c5 into master Jul 30, 2020
Vapor 4 automation moved this from Awaiting Review to Done Jul 30, 2020
@tanner0101 tanner0101 deleted the tn-int-fix branch July 30, 2020 15:36
@tanner0101
Copy link
Member Author

These changes are now available in 1.3.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

Unsigned integer read as signed
1 participant