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: parsing big integers in JSON data loses precision #489

Closed
lance opened this issue May 13, 2022 · 4 comments · Fixed by #495
Closed

bug: parsing big integers in JSON data loses precision #489

lance opened this issue May 13, 2022 · 4 comments · Fixed by #495
Labels

Comments

@lance
Copy link
Member

lance commented May 13, 2022

Describe the Bug
When parsing JSON data, if a JSON field value is a number, and that number is really big, JavaScript loses precision. For example, the Twitter API exposes the Tweet ID. This is a large number that exceeds the integer space of Number. You can see this by simply assigning a large number to a simple variable (it's not actually JSON that is the problem, it's JavaScript itself).

> let id = 1524831183200260097
undefined
> id
1524831183200260000

Steps to Reproduce

❯ npm install cloudevents
❯ cat > index.js
const { CloudEvent } = require('cloudevents')

let e = new CloudEvent({ source: 'example', type: 'example', data: 1524831183200260097 })
console.log(e.data)
^D
❯ node index.js
1524831183200260000

Expected Behavior
I expect the data to not lose precision.

Additional context
This can be resolved by using json-bigint

> let JSON = require('json-bigint')({useNativeBigInt: true})
> let j = JSON.parse('{ "key": 993143214321423154315154321 }')
> j.key
993143214321423154315154321
@lance lance added the type/bug Something isn't working label May 13, 2022
@cardil
Copy link

cardil commented May 13, 2022

I think your Steps to Reproduce are a little misleading. You type there in verbatim a long number. JavaScript engine, parse a file, and use IEEE 754 Standard to parse a Number. You could avoid that by using BigInt notation:

let e = new CloudEvent({ source: 'example', type: 'example', data: 1524831183200260097n })

The problem exists, though. It lies in parsers implemented in this library. Here's simple jest for HTTP:

const { HTTP } = require('cloudevents')

test('Parse big numbers from HTTP', () => {
  let ce = HTTP.toEvent({
    headers: { 'content-type': 'application/cloudevents+json' },
    body: `{"id": 1524831183200260097}`
  })
  expect(ce.id).toBe(1524831183200260097n);
})

This test fails:

  ● Parse big numbers from HTTP

    expect(received).toBe(expected) // Object.is equality

    Expected: 1524831183200260097n
    Received: 1524831183200260000

       6 |     body: `{"id": 1524831183200260097}`
       7 |   })
    >  8 |   expect(ce.id).toBe(1524831183200260097n)
         |                 ^
       9 | })
      10 |
      11 |

      at Object.toBe (bigint.test.js:8:17)

@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

@lance
Copy link
Member Author

lance commented Jun 14, 2022

@cardil you are correct to point out the error in my example. But your example would not produce a valid CE even if we could handle big integers, because the required type for the id field is String. https://github.com/cloudevents/spec/blob/main/cloudevents/spec.md#id

I think this is still a problem, but if something is sending a big int for the id that's not going to fly.

lance added a commit to lance/sdk-javascript that referenced this issue Jun 14, 2022
An event may have data that contains a BigInt. The builtin `JSON` parser
for JavaScript does not handle the `BigInt` types. The introduced
`json-bigint` dependency (34k) does.

Fixes: cloudevents#489

Signed-off-by: Lance Ball <lball@redhat.com>
lance added a commit to lance/sdk-javascript that referenced this issue Jun 15, 2022
An event may have data that contains a BigInt. The builtin `JSON` parser
for JavaScript does not handle the `BigInt` types. The introduced
`json-bigint` dependency (34k) does.

Fixes: cloudevents#489

Signed-off-by: Lance Ball <lball@redhat.com>
@github-actions
Copy link
Contributor

This issue is stale because it has been open 30 days with no activity.

lance added a commit to lance/sdk-javascript that referenced this issue May 8, 2023
An event may have data that contains a BigInt. The builtin `JSON` parser
for JavaScript does not handle the `BigInt` types. The introduced
`json-bigint` dependency (34k) does.

Fixes: cloudevents#489

Signed-off-by: Lance Ball <lball@redhat.com>
lance added a commit that referenced this issue May 10, 2023
* fix: handle big integers in incoming events

An event may have data that contains a BigInt. The builtin `JSON` parser
for JavaScript does not handle the `BigInt` types. The introduced
`json-bigint` dependency (34k) does.

Fixes: #489

Signed-off-by: Lance Ball <lball@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants