sidorares / json-bigint

JSON.parse/stringify with bigints support
MIT License
808 stars 192 forks source link

missing hasOwnProperty function after Parse #38

Closed runerback closed 4 years ago

runerback commented 4 years ago

I got big number serializing problem in JSON, so I decide to override JSON.parse and JSON.stringify with this package globally, then I encountered some errors in other js codes,

it seems Object parsed by JSONbig lost hasOwnProperty function:

import * as JSONbig from "json-bigint";

var json = '{ "value" : 9223372036854775807, "v2": 123 }';

console.log(
    JSON.parse(json).hasOwnProperty("value")
);

console.log(
    JSONbig.parse(json).hasOwnProperty("value")
);

module def for typescript:

declare module "json-bigint" {
    function parse(text: string, reviver?: (this: any, key: string, value: any) => any): any;
    function stringify(value: any, replacer?: (this: any, key: string, value: any) => any, space?: string | number): string;
    function stringify(value: any, replacer?: (number | string)[] | null, space?: string | number): string;
}

got output:

true
console.log(JSONbig.parse(json).hasOwnProperty("value"));
                                ^

TypeError: JSONbig.parse(...).hasOwnProperty is not a function
    at Object.<anonymous> (\resources\json-bigint\dist\index.js:27:33)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47
error Command failed with exit code 1.
sidorares commented 4 years ago

top level export returns a function, we just set .parse and .stringify methods on it to make it easier to work with default option.

Change console.log(JSONbig.parse(json).hasOwnProperty("value")); to something like console.log(JSONbig({}).parse(json).hasOwnProperty("value")); ( or better, save it earlier as something like const JSON = JSONBig({/* see options in readme*/}); )

runerback commented 4 years ago

it works, thanks.

goxr3plus commented 4 years ago

@runerback @sidorares

This doesn't work for me

var JSONbig = require('json-bigint');
var JSONBigInt = JSONbig({/* see options in readme*/});
var json = '{ "value" : 9223372036854775807, "v2": 123 }';
console.log(
    JSON.parse(json).hasOwnProperty("value")
);
console.log(JSONBigInt.parse(json).hasOwnProperty("value")); 

I ended up using this :

const jsonWithString = JSONbigString.parse(json)
const toString = JSONbigint.stringify(jsonWithString)
console.log(JSON.parse(toString).hasOwnProperty('value'))
sidorares commented 4 years ago

@goxr3plus this is a recent change, to add better protection from prototype pollution I changed the way objects are created from {} to Object.create(null), unfortunately this has a side effect you posted. Not sure if there is a better way to make it to turn '{"__proto___": { "test": true }}' string into {__proto___: {test: true }} object rather than {} with {test: true} prototype

goxr3plus commented 4 years ago

@sidorares

so how should I write my above example in order to work properly?

sidorares commented 4 years ago

Objects can be created with null prototype, in that case they won't have .hasOwnProperty method.

Safer way to call:

const example = Object.create(null)
Object.hasOwnProperty.call(example, 'value')
// > false
example.value = 111
Object.hasOwnProperty.call(example, 'value')
// > true

const hasOwnProperty = (object, propertyName) => Object.hasOwnProperty.call(object, propertyName)
hasOwnProperty(example, 'value2')
// > false
hasOwnProperty(example, 'value')
// > true
goxr3plus commented 4 years ago

Thank you :)

klesun commented 3 years ago

But that, like, totally breaks compatibility with JSON.parse. Changing whole codebase from parsed.hasOwnProperty('value') to Object.hasOwnProperty(obj, 'value') just to use this lib is a really big setback.

Is there no option to specify that the proto class should be Object, not null?

Upd.: yep, there is no such option. But rejoice, I filed a PR to add it: objectBaseClass to take Object so that it was used instead of null in Object.create(...): https://github.com/sidorares/json-bigint/pull/47

P.S. I did not look too deep, but can't you simply mimick JSON.parse()'s behaviour for __proto__ using Object.defineProperty? And why does the restriction of writing to __proto__ require basing objects on null anyway?

klesun commented 3 years ago

If this PR does not get accepted anytime soon, can just follow the solution from https://github.com/sidorares/json-bigint/issues/39#issuecomment-692533573 and downgrade to v0.4.0:

npm install --save json-bigint@0.4.0
MasterOdin commented 1 year ago

Adding onto this issue, this library is billed as:

JSON.parse/stringify with bigints support.

and we ended up getting tripped up on this difference of behavior between JSON.parse and JSONbig.parse and should be prominently documented as such.