sidorares / json-bigint

JSON.parse/stringify with bigints support
MIT License
790 stars 189 forks source link

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

Open Ayzrian opened 3 years ago

Ayzrian commented 3 years ago

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 commented 3 years ago

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 https://github.com/sidorares/json-bigint/blob/390482a8b6b460f98c61c3b65915dbd91fc8e7b2/lib/parse.js#L215

BigInt cannot take numbers with decimal points

airhorns commented 2 years ago

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 commented 2 years ago

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

Targma commented 2 years ago

This is a quick fix in parser.sj.

image

cjcandctr commented 1 year ago

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 commented 1 year ago

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'
ktmud commented 1 month ago

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 commented 1 day ago

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