digitalbazaar / digitalbazaar/oid4-client

Consider using `Number.isSafeInteger(i)`

Open
#80 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
2
Forks
2
Avg merge
16m
Merged PRs (30d)
1

Description

  • Does this need more edge case checks?
  • Might consider at least having a zero and negative int check. Probably Number.MIN/MAX_SAFE_INTEGER too.
  • Does this need the reverse checks of data with a plain int and query with an int string? I think the code would handle that?
  • Parsed JSON itself with native ints will (probably?) be limited to Number.MIN/MAX_SAFE_INTEGER. But if both sides had strings with non safe int values, I think this starts to lose precision and would cause matching failures. In any case, havoc will probably happen. Is that important to handle here?

Example:

> Number.MAX_SAFE_INTEGER
9007199254740991
> a = parseInt('9007199254740992')
9007199254740992
> b = parseInt('9007199254740993')
9007199254740992
> a.toString()
'9007199254740992'
> b.toString()
'9007199254740992'
> a.toString() === b.toString()
true
  • One solution might be to use BigInt(x), but that's introducing radix prefix issues and returning BigInts vs ints for comparison and no doubt has odd issues.
  • Another solution is to specifically not handle these issues and after the parseInt, check if Number.isSafeInteger(i) is false, then return x. Would need some tests to make sure that works and simply refuses to match those non-safe string values for now.

Originally posted by @davidlehn in https://github.com/digitalbazaar/oid4-client/pull/79#pullrequestreview-4386185039


We probably only want the Number.isSafeInteger(i) check, not any BigInt checks. We're only interested, I think, in converting integers for array indexing processing and interoperable JSON numbers.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the parseInt conversion used for array-indexing processing and review how JSON numbers and integer strings are compared. Add edge-case tests around Number.isSafeInteger(i), including safe and non-safe values, and verify that non-safe integer strings do not produce incorrect matches.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
api
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.