cloudevents/sdk-javascript

bug: parsing big integers in JSON `data` loses precision

lance opened this issue · 4 comments

lance commented

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

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)

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

lance commented

@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.

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