Skip to content

Commit 4f1c0e2

Browse files
ruiquelhasruiquelhas
committed
BUG#34728259: return unsafe numeric field values as strings
When retrieving records in a JSON column, if a field contains an unsafe numeric value, it is parsed as a JavaScript number from the corresponding result set. This means that applications are dealing with values that are loosing precision. The reason for this to happen is that the connector is parsing JSON strings as their corresponding plain JavaScript object counterparts for the sake of convenience. This creates a problem wherein the native "JSON.parse()" API automatically coerces a numeric value of a field to the a JavaScript number and there is no way to override that behavior, even using a "reviver", because the value is already coerced by that point. This patch addresses the issue by introducing a 3rd-party JSON parser which is capable of "reviving" unsafe big numbers (which loose precision when represented by JavaScript numbers), or even all numeric values as JavaScript strings. This is the strategy already in place to parse unsafe numeric values from explicit column data types such as BIGINT and DECIMAL. Change-Id: I30ad08d6088cf4d7388ec78d71c63119de5e34ad
1 parent db1b258 commit 4f1c0e2

File tree

8 files changed

+194
-19
lines changed

8 files changed

+194
-19
lines changed

‎lib/Protocol/Wrappers/Messages/Resultset/Row.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
'use strict';
3232

33+
const JSON = require('../../../../json');
3334
const ResultsetStub = require('../../../Stubs/mysqlx_resultset_pb');
3435
const ServerMessagesStub = require('../../../Stubs/mysqlx_pb').ServerMessages;
3536
const bytes = require('../../ScalarValues/bytes');
@@ -171,7 +172,7 @@ function decodeOpaqueByteString (decoder, options) {
171172
return data;
172173
}
173174

174-
return JSON.parse(data);
175+
return JSON(data).parse({ unsafeNumberAsString: true });
175176
}
176177

177178
/**

‎lib/json.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright (c) 2022, Oracle and/or its affiliates.
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License, version 2.0, as
6+
* published by the Free Software Foundation.
7+
*
8+
* This program is also distributed with certain software (including
9+
* but not limited to OpenSSL) that is licensed under separate terms,
10+
* as designated in a particular file or component or in included license
11+
* documentation. The authors of MySQL hereby grant you an
12+
* additional permission to link the program and your derivative works
13+
* with the separately licensed software that they have included with
14+
* MySQL.
15+
*
16+
* Without limiting anything contained in the foregoing, this file,
17+
* which is part of MySQL Connector/Node.js, is also subject to the
18+
* Universal FOSS Exception, version 1.0, a copy of which can be found at
19+
* http://oss.oracle.com/licenses/universal-foss-exception.
20+
*
21+
* This program is distributed in the hope that it will be useful, but
22+
* WITHOUT ANY WARRANTY; without even the implied warranty of
23+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
24+
* See the GNU General Public License, version 2.0, for more details.
25+
*
26+
* You should have received a copy of the GNU General Public License
27+
* along with this program; if not, write to the Free Software Foundation, Inc.,
28+
* 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
29+
*/
30+
31+
'use strict';
32+
33+
const { isNumber, isSafeNumber, parse } = require('lossless-json');
34+
35+
/**
36+
* Custom JSON module that extends the functionality provided by the native
37+
* JSON object.
38+
* @private
39+
* @param {string} json - A valid JSON string.
40+
* @returns
41+
*/
42+
module.exports = (json) => ({
43+
/**
44+
* Parse the JSON string as a plain JavaScript object given specific
45+
* transformation rules for large numeric values which loose precision
46+
* if represented as JavaScript numbers.
47+
* @private
48+
* @param {boolean} [unsafeNumberAsString] - An option used to instruct
49+
* the parser to return unsafe numeric values as JavaScript strings.
50+
* @returns A plain JavaScript object.
51+
*/
52+
parse ({ unsafeNumberAsString = false } = {}) {
53+
return parse(json, null, x => {
54+
if (isNumber(x) && isSafeNumber(x)) {
55+
return Number(x);
56+
}
57+
58+
if (isNumber(x) && !isSafeNumber(x) && !unsafeNumberAsString) {
59+
return Number(x);
60+
}
61+
62+
return x;
63+
});
64+
}
65+
});

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
},
6464
"dependencies": {
6565
"google-protobuf": "3.21.2",
66+
"lossless-json": "2.0.1",
6667
"parsimmon": "1.18.1"
6768
},
6869
"engines": {

‎test/functional/default/document-store/collection-as-table.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,24 @@ describe('finding documents in collections using CRUD with a table API', () => {
9090
});
9191
});
9292
});
93+
94+
context('BUG#34728259', () => {
95+
const id = '1';
96+
const signedBigInt = '-9223372036854775808';
97+
const unsignedBigInt = '18446744073709551615';
98+
99+
beforeEach('populate collection', () => {
100+
return session.sql(`INSERT INTO \`${schema.getName()}\`.\`${collection.getName()}\` (doc) VALUES ('{ "_id": "${id}", "signedBigInt": ${signedBigInt}, "unsignedBigInt": ${unsignedBigInt} }')`)
101+
.execute();
102+
});
103+
104+
it('returns unsafe numeric field values as strings', () => {
105+
return schema.getCollectionAsTable(collection.getName())
106+
.select('doc')
107+
.execute()
108+
.then(res => {
109+
return expect(res.fetchOne()).to.deep.equal([{ _id: id, signedBigInt, unsignedBigInt }]);
110+
});
111+
});
112+
});
93113
});

‎test/functional/default/document-store/collection-find.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,25 @@ describe('finding documents in collections using CRUD', () => {
608608
});
609609
});
610610

611+
context('BUG#34728259', () => {
612+
const signedBigInt = '-9223372036854775808';
613+
const unsignedBigInt = '18446744073709551615';
614+
615+
beforeEach('populate collection', () => {
616+
return session.sql(`INSERT INTO \`${schema.getName()}\`.\`${collection.getName()}\` (doc) VALUES ('{ "_id": "1", "signedBigInt": ${signedBigInt}, "unsignedBigInt": ${unsignedBigInt} }')`)
617+
.execute();
618+
});
619+
620+
it('returns unsafe numeric field values as strings', () => {
621+
return collection.find()
622+
.fields('signedBigInt', 'unsignedBigInt')
623+
.execute()
624+
.then(res => {
625+
return expect(res.fetchOne()).to.deep.equal({ signedBigInt, unsignedBigInt });
626+
});
627+
});
628+
});
629+
611630
context('when debug mode is enabled', () => {
612631
beforeEach('populate collection', () => {
613632
return collection.add({ name: 'foo', count: 2 })

‎test/functional/default/relational-tables/misc.js‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -560,23 +560,24 @@ describe('relational miscellaneous tests', () => {
560560

561561
context('BUG#34016587 values encoded as DECIMAL with precision loss', () => {
562562
beforeEach('create table', () => {
563-
// A JavaScript number does not have enough precision to encode
564-
// more than 17 corresponding DECIMAL digits in MySQL.
563+
// A JavaScript number does not have enough precision to safely
564+
// represent a decimal value with more than 16 decimal scale
565+
// digits (sometimes even less depending on which digits).
565566
return session.sql(`CREATE TABLE test (
566-
decimal_1 DECIMAL(18, 1),
567-
decimal_2 DECIMAL(18, 17),
568-
decimal_3 DECIMAL(18, 17),
569-
decimal_4 DECIMAL(18, 1))`)
567+
decimal_1 DECIMAL(17, 1),
568+
decimal_2 DECIMAL(17, 16),
569+
decimal_3 DECIMAL(17, 16),
570+
decimal_4 DECIMAL(17, 1))`)
570571
.execute();
571572
});
572573

573574
beforeEach('add fixtures', () => {
574-
return session.sql('INSERT INTO test VALUES (-99999999999999999.9, -9.99999999999999999, 9.99999999999999999, 99999999999999999.9)')
575+
return session.sql('INSERT INTO test VALUES (-9999999999999999.9, -9.9999999999999999, 9.9999999999999999, 9999999999999999.9)')
575576
.execute();
576577
});
577578

578579
it('decodes values using a string to avoid precision loss', () => {
579-
const expected = [['-99999999999999999.9', '-9.99999999999999999', '9.99999999999999999', '99999999999999999.9']];
580+
const expected = [['-9999999999999999.9', '-9.9999999999999999', '9.9999999999999999', '9999999999999999.9']];
580581

581582
return schema.getTable('test').select()
582583
.execute()

‎test/functional/default/sql.js‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -902,13 +902,16 @@ describe('raw SQL', () => {
902902
});
903903

904904
context('JSON', () => {
905+
const signedBigInt = '-9223372036854775808';
906+
const unsignedBigInt = '18446744073709551615';
907+
905908
beforeEach('create table', () => {
906909
return session.sql('CREATE TABLE test (value JSON)')
907910
.execute();
908911
});
909912

910-
beforeEach('add fixtures', () => {
911-
return session.sql('INSERT INTO test VALUES (\'{"foo":"bar"}\')')
913+
beforeEach('populate column', () => {
914+
return session.sql(`INSERT INTO test (value) VALUES ('{ "signedBigInt": ${signedBigInt}, "unsignedBigInt": ${unsignedBigInt} }')`)
912915
.execute();
913916
});
914917

@@ -917,6 +920,16 @@ describe('raw SQL', () => {
917920
.execute()
918921
.then(res => expect(res.getColumns()[0].getType()).to.equal('JSON'));
919922
});
923+
924+
context('BUG#34728259', () => {
925+
it('returns unsafe numeric field values as strings', () => {
926+
return session.sql('SELECT value FROM test')
927+
.execute()
928+
.then(res => {
929+
return expect(res.fetchOne()).to.deep.equal([{ signedBigInt, unsignedBigInt }]);
930+
});
931+
});
932+
});
920933
});
921934

922935
context('STRING', () => {
@@ -1156,23 +1169,24 @@ describe('raw SQL', () => {
11561169

11571170
context('BUG#34016587 values encoded as DECIMAL with precision loss', () => {
11581171
beforeEach('create table', () => {
1159-
// A JavaScript number does not have enough precision to encode
1160-
// more than 17 corresponding DECIMAL digits in MySQL.
1172+
// A JavaScript number does not have enough precision to safely
1173+
// represent a decimal value with more than 16 decimal scale
1174+
// digits (sometimes even less depending on which digits).
11611175
return session.sql(`CREATE TABLE test (
1162-
decimal_1 DECIMAL(18, 1),
1163-
decimal_2 DECIMAL(18, 17),
1164-
decimal_3 DECIMAL(18, 17),
1165-
decimal_4 DECIMAL(18, 1))`)
1176+
decimal_1 DECIMAL(17, 1),
1177+
decimal_2 DECIMAL(17, 16),
1178+
decimal_3 DECIMAL(17, 16),
1179+
decimal_4 DECIMAL(17, 1))`)
11661180
.execute();
11671181
});
11681182

11691183
beforeEach('add fixtures', () => {
1170-
return session.sql('INSERT INTO test VALUES (-99999999999999999.9, -9.99999999999999999, 9.99999999999999999, 99999999999999999.9)')
1184+
return session.sql('INSERT INTO test VALUES (-9999999999999999.9, -9.9999999999999999, 9.9999999999999999, 9999999999999999.9)')
11711185
.execute();
11721186
});
11731187

11741188
it('decodes values using a string to avoid precision loss', () => {
1175-
const expected = [['-99999999999999999.9', '-9.99999999999999999', '9.99999999999999999', '99999999999999999.9']];
1189+
const expected = [['-9999999999999999.9', '-9.9999999999999999', '9.9999999999999999', '9999999999999999.9']];
11761190

11771191
return session.sql('select * from test')
11781192
.execute()

‎test/unit/json.js‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Copyright (c) 2022, Oracle and/or its affiliates.
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License, version 2.0, as
6+
* published by the Free Software Foundation.
7+
*
8+
* This program is also distributed with certain software (including
9+
* but not limited to OpenSSL) that is licensed under separate terms,
10+
* as designated in a particular file or component or in included license
11+
* documentation. The authors of MySQL hereby grant you an
12+
* additional permission to link the program and your derivative works
13+
* with the separately licensed software that they have included with
14+
* MySQL.
15+
*
16+
* Without limiting anything contained in the foregoing, this file,
17+
* which is part of MySQL Connector/Node.js, is also subject to the
18+
* Universal FOSS Exception, version 1.0, a copy of which can be found at
19+
* http://oss.oracle.com/licenses/universal-foss-exception.
20+
*
21+
* This program is distributed in the hope that it will be useful, but
22+
* WITHOUT ANY WARRANTY; without even the implied warranty of
23+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
24+
* See the GNU General Public License, version 2.0, for more details.
25+
*
26+
* You should have received a copy of the GNU General Public License
27+
* along with this program; if not, write to the Free Software Foundation, Inc.,
28+
* 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
29+
*/
30+
31+
'use strict';
32+
33+
/* eslint-env node, mocha */
34+
35+
const expect = require('chai').expect;
36+
const MyJSON = require('../../lib/json');
37+
38+
describe('JSON', () => {
39+
const signedBigInt = '-9223372036854775808';
40+
const unsignedBigInt = '18446744073709551615';
41+
const unsafeDecimal = '9.9999999999999999';
42+
const json = `{ "a": "foo", "b": true, "c": false, "d": null, "e": 1234, "f": 1.234, "g": ${signedBigInt}, "h": ${unsignedBigInt}, "i": ${unsafeDecimal} }`;
43+
44+
context('parse()', () => {
45+
it('returns the same output as the native JSON.parse() when the "unsafeNumberAsString" option is not enabled (default)', () => {
46+
expect(MyJSON(json).parse()).to.deep.equal(JSON.parse(json));
47+
});
48+
49+
it('returns unsafe values as strings when the "unsafeNumberAsString" option is enabled', () => {
50+
const expected = { a: 'foo', b: true, c: false, d: null, e: 1234, f: 1.234, g: signedBigInt, h: unsignedBigInt, i: unsafeDecimal };
51+
expect(MyJSON(json).parse({ unsafeNumberAsString: true })).to.deep.equal(expected);
52+
});
53+
});
54+
});

0 commit comments

Comments
 (0)