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

Fix int64 overflow issue to due precision issues in Typescript with large numbers #253

Conversation

ilanashapiro
Copy link
Contributor

@ilanashapiro ilanashapiro commented Sep 26, 2023

Currently, MAX_INT_64 is set to 2 ** 63 - 1, and MIN_INT_64 is set to 0 - 2 ** 63 in src/oas/transformers/schema/keywords/format.ts. The TypeScript compiler evaluates these, respectively, to 9223372036854776000 and -9223372036854776000.

Motivation and Context

These values are outside the range of int64 and result in bugs in clients such as Prism that use this library (e.g. stoplightio/prism#1992).

The reasoning behind this erroneous calculation is explained here: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER. Essentially, we are overflowing the capacity of JavaScript's Number type, and beyond Number.MAX_SAFE_INTEGER + 1 (9007199254740992), the IEEE-754 floating-point format can no longer represent every consecutive integer (see: https://stackoverflow.com/questions/1379934/large-numbers-erroneously-rounded-in-javascript).

Description

Thus, instead of setting MAX_INT_64 to 2 ** 63 - 1, and MIN_INT_64 to 0 - 2 ** 63, we set them to TypeScript's built-in Number.MAX_SAFE_INTEGER and Number.MIN_SAFE_INTEGER instead.

How Has This Been Tested?

Update local tests including screenshot tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

@@ -85,7 +85,7 @@
"lodash.pickby": "^4.6.0",
"openapi3-ts": "^2.0.2",
"postman-collection": "^4.1.2",
"tslib": "^2.3.1",
"tslib": "^2.6.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number.MAX_SAFE_INTEGER and Number.MIN_SAFE_INTEGER are not available on tslib v2.3.1 so we need to update

@ilanashapiro
Copy link
Contributor Author

@ilanashapiro
Copy link
Contributor Author

@daniel-white Hi! Could someone from your team please take a look at this PR? Thanks!

@P0lip
Copy link
Contributor

P0lip commented Oct 6, 2023

Heyo! I'll take a look at the PR soon

Copy link
Contributor

@P0lip P0lip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we'd switch these values over to bigints, but that'd be somewhat troublesome to implement and would require changes across many repos and libraries.
I'm fine with this change since at the very least the values are now well within the range.

@P0lip P0lip enabled auto-merge (squash) October 9, 2023 12:18
@P0lip P0lip merged commit 69d38ec into stoplightio:master Oct 9, 2023
2 checks passed
@stoplight-bot
Copy link
Collaborator

🎉 This PR is included in version 6.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants