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

Bug; parse tries to parse float numbers as BigInt. #49

Open
Ayzrian opened this issue Mar 15, 2021 · 9 comments
Open

Bug; parse tries to parse float numbers as BigInt. #49

Ayzrian opened this issue Mar 15, 2021 · 9 comments

Comments

@Ayzrian
Copy link

Ayzrian commented Mar 15, 2021

Parse tries to convert float number as BigInt. It is a bug because it should not try to cast Float numbers to BigInt
SyntaxError: Cannot convert 0.8120414029545044 to a BigInt

@ryan-ju
Copy link

ryan-ju commented Jun 25, 2021

To reproduce, this is a simple program:

import JSONBig from 'json-bigint';

const input = '{"value": 42.144481999999}';

const big = JSONBig({useNativeBigInt: true, alwaysParseAsBig: true});

// Throws error:
//     RangeError: The number 42.144481999999 cannot be converted to a BigInt because it is not an integer
big.parse(input);

This is where the bug is

? BigInt(string)

BigInt cannot take numbers with decimal points

@airhorns
Copy link

airhorns commented Aug 16, 2021

Could the fix for this be as simple as looking for a . in the incoming string, and only using a bigint if the value doesn't have that character? Will hurt performance but this bug is pretty bad since most folks parsing JSON don't control the source and thus it'd be pretty easy to get sent floats.

@a-w-1806
Copy link

Just came across this today, and totally agree with @airhorns that this is easily triggered by the users rather than the developers.

@Targma
Copy link

Targma commented Feb 22, 2022

This is a quick fix in parser.sj.

image

@cjcandctr
Copy link

My solution is removing useNativeBigInt options.

I've read the readme quite a lot of times. Seems that it is trying to say floating number should consider as bigint. However, native bigint does not support floating number. Anyway, this should be fixed instead of throwing an error to user.

While most JSON parsers assume numeric values have same precision restrictions as IEEE 754 double, JSON specification does not say anything about number precision. Any floating point number in decimal (optionally scientific) notation is valid JSON value. It's a good idea to serialize values which might fall out of IEEE 754 integer precision as strings in your JSON api......

@vazkir
Copy link

vazkir commented Aug 11, 2022

There seems to be a fix already in master for this, only the package hasn't seen an official npm release since somewhere in 2020. At least for me it works now when I use the latest version from github, as explained in this issue

For example for yarn I ran:

yarn remove json-bigint

And then the version from gh:

yarn add 'json-bigint@https://github.com/sidorares/json-bigint'

ovaistariq added a commit to tigrisdata-archive/tigris-client-ts that referenced this issue Apr 7, 2023
where it tries to parse float numbers as BigInt
ovaistariq added a commit to tigrisdata-archive/tigris-client-ts that referenced this issue Apr 7, 2023
Fix for json-bigint bug sidorares/json-bigint#49
where it tries to parse float numbers as BigInt
@ktmud
Copy link

ktmud commented Jun 2, 2024

For anyone who wants to use only the native BigInt, here's a simple solution, without using any libraries (except some core-js polyfills if needed), utilizing the new JSON contex.source and JSON.rawJSON API:

import 'core-js/modules/esnext.json.parse';
import 'core-js/modules/esnext.json.raw-json';

export const ogirinalJSONParse = JSON.parse;

export interface JSONReviverContext {
  source: string;
}

const INTEGER_REGEX = /^-?\d+$/;

function isInteger(value: string) {
  return INTEGER_REGEX.test(value);
}

/**
 * Parse a JSON string with potential BigInt values.
 */
const parse: typeof ogirinalJSONParse = (text, reviver) => {
  return ogirinalJSONParse(
    text,
    // cannot use arrow function because we want to keep `this` context
    function reviveWithBigInt(key, value, context?: JSONReviverContext) {
      const obj = this;
      // @ts-expect-error Expected 3 arguments, but got 4.
      // eslint-disable-next-line @typescript-eslint/no-explicit-any
      const finalize = (val: any) => (reviver ? reviver.call(obj, key, val, context) : val);
      if (
        context?.source &&
        typeof value === 'number' &&
        typeof context?.source === 'string' &&
        (value < Number.MIN_SAFE_INTEGER || value > Number.MAX_SAFE_INTEGER) &&
        isInteger(context.source) &&
        BigInt(value) !== BigInt(context.source)
      ) {
        return finalize(BigInt(context.source));
      }
      return finalize(value);
    },
  );
};

// Patch the default JSON.parse to support BigInt values.
JSON.parse = parse;

// This PATCH makes `JSON.stringify` support BigInt values.
// @ts-expect-error Property 'toJSON' does not exist on type 'BigInt'.ts(2339)
// eslint-disable-next-line no-extend-native
BigInt.prototype.toJSON = function toJSON() {
  // @ts-expect-error Property 'rawJSON' does not exist on type 'JSON'.
  return JSON.rawJSON(this.toString());
};

// Named export makes auto-imports in IDE easier.
const JSONBigInt = {
  parse,
  stringify: JSON.stringify,
};
export default JSONBigInt;

@davidfaj
Copy link

davidfaj commented Jul 4, 2024

Thanks @vazkir for the solution, also linking to the other issue. I did something similar with npm.
Uninstalled current npm version:
npm uninstall json-bigint
Then installed from Github version:
npm i sidorares/json-bigint

@Darker
Copy link

Darker commented Oct 1, 2024

I also encountered this problem. Notably, it happens even for numbers that comfortably fit in IEE754 double, like this one:

SyntaxError: Cannot convert 2.2756800548799605 to a BigInt

But would work fine with native JSON:

node
Welcome to Node.js v20.2.0.
Type ".help" for more information.
> JSON.stringify(2.2756800548799605)*1 === 2.2756800548799605
true

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

No branches or pull requests

10 participants