-
Notifications
You must be signed in to change notification settings - Fork 32
Bigint silent truncation #135
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
6bac41a
82e1bab
89a19ee
003d162
b46359a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,11 @@ project adheres to [Semantic Versioning](http://semver.org/). | |
|
|
||
| * Decoding Array and VarArray now fast fails when the array length exceeds remaining bytes to decode ([#132](https://github.com/stellar/js-xdr/pull/132)) | ||
|
|
||
| * Fixed silent truncation of bigint values exceeding the range of sized integers (`Hyper`, `UnsignedHyper`, and other `LargeInt` subtypes). Construction, encoding, and multi-part assembly now throw on overflow/underflow instead of silently clamping. `isValid` also validates value range. `sliceBigInt` now returns unsigned slice values for consistency ([#133](https://github.com/stellar/js-xdr/pull/133)). | ||
|
|
||
|
|
||
|
|
||
|
|
||
|
Comment on lines
+18
to
+20
|
||
| ## [v3.1.2](https://github.com/stellar/js-xdr/compare/v3.1.1...v3.1.2) | ||
|
|
||
| ### Fixed | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,20 +43,32 @@ export function encodeBigIntFromBits(parts, size, unsigned) { | |
| throw new TypeError(`expected bigint-like values, got: ${parts} (${e})`); | ||
| } | ||
|
|
||
| // check for sign mismatches for single inputs (this is a special case to | ||
| // handle one parameter passed to e.g. UnsignedHyper et al.) | ||
| // see https://github.com/stellar/js-xdr/pull/100#discussion_r1228770845 | ||
| if (unsigned && parts.length === 1 && parts[0] < 0n) { | ||
| throw new RangeError(`expected a positive value, got: ${parts}`); | ||
| // fast path: single value — validate and return directly without assembly | ||
| if (parts.length === 1) { | ||
| const value = parts[0]; | ||
| if (unsigned && value < 0n) { | ||
| throw new RangeError(`expected a positive value, got: ${parts}`); | ||
| } | ||
| const [min, max] = calculateBigIntBoundaries(size, unsigned); | ||
| if (value < min || value > max) { | ||
| throw new TypeError( | ||
| `bigint value ${value} for ${formatIntName( | ||
| size, | ||
| unsigned | ||
| )} out of range [${min}, ${max}]` | ||
| ); | ||
| } | ||
|
Comment on lines
+54
to
+60
|
||
| return value; | ||
| } | ||
|
|
||
| // encode in big-endian fashion, shifting each slice by the slice size | ||
| let result = BigInt.asUintN(sliceSize, parts[0]); // safe: len >= 1 | ||
| for (let i = 1; i < parts.length; i++) { | ||
| // multi-part assembly: encode in big-endian fashion, shifting each slice | ||
| let result = 0n; | ||
|
|
||
| for (let i = 0; i < parts.length; i++) { | ||
| assertSliceFits(parts[i], sliceSize); | ||
| result |= BigInt.asUintN(sliceSize, parts[i]) << BigInt(i * sliceSize); | ||
| } | ||
|
|
||
| // interpret value as signed if necessary and clamp it | ||
| if (!unsigned) { | ||
| result = BigInt.asIntN(size, result); | ||
| } | ||
|
|
@@ -106,15 +118,11 @@ export function sliceBigInt(value, iSize, sliceSize) { | |
| } | ||
|
|
||
| const shift = BigInt(sliceSize); | ||
| const mask = (1n << shift) - 1n; | ||
|
|
||
| // iterate shift and mask application | ||
| const result = new Array(total); | ||
| for (let i = 0; i < total; i++) { | ||
| // we force a signed interpretation to preserve sign in each slice value, | ||
| // but downstream can convert to unsigned if it's appropriate | ||
| result[i] = BigInt.asIntN(sliceSize, value); // clamps to size | ||
|
|
||
| // move on to the next chunk | ||
| result[i] = value & mask; | ||
| value >>= shift; | ||
| } | ||
|
|
||
|
|
@@ -139,3 +147,21 @@ export function calculateBigIntBoundaries(size, unsigned) { | |
| const boundary = 1n << BigInt(size - 1); | ||
| return [0n - boundary, boundary - 1n]; | ||
| } | ||
|
|
||
| /** | ||
| * Asserts that a given part fits within the specified slice size. | ||
| * @param {bigint | number | string} part - The part to check. | ||
| * @param {number} sliceSize - The size of the slice in bits (e.g., 32, 64, 128) | ||
| * @returns {void} | ||
| * @throws {RangeError} If the part does not fit within the slice size. | ||
| */ | ||
| function assertSliceFits(part, sliceSize) { | ||
| const fitsSigned = BigInt.asIntN(sliceSize, part) === part; | ||
| const fitsUnsigned = BigInt.asUintN(sliceSize, part) === part; | ||
|
|
||
| if (!fitsSigned && !fitsUnsigned) { | ||
| throw new RangeError( | ||
| `slice value ${part} does not fit in ${sliceSize} bits` | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changelog entry claims that
sliceBigIntnow returns unsigned slice values, but the current implementation ofsliceBigIntstill usesBigInt.asIntN(...)for each slice (i.e., signed slices). Either update the changelog text to match the actual behavior or adjustsliceBigIntaccordingly.