-
Notifications
You must be signed in to change notification settings - Fork 17
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
Invalid results with values larger than MAX_SAFE_INTEGER #8
Comments
Hello, thank you very much for your contribution, you are correct, in that case, the decoding should be done using BigInt. I have prepared a solution for the problem that I will post between today and tomorrow. Thanks again, regards! |
I have updated the library (v.1.3.2) , and now your code should work correctly. If you detect any malfunctions, I would appreciate it if you could notify me. Regards. |
Hi, thanks for the superfast fix, appreciated! However, I'm afraid there's still some issues here with GIAI-96: > const t = tds.fromTagURI("urn:epc:tag:giai-96:0.1234567.30000000004214215")
undefined
> t.toTagURI()
'urn:epc:tag:giai-96:0.1234567.30000000004214215'
> t.toHexString()
'34144B5A1C6A94D74F834DC7'
> t.getAssetReference()
30000000004214216
> t.getGiai()
'123456730000000004214216' The |
😅 I apologize, it was overlooked. I have created a new version (v.1.3.3) with the problem solved, thank you for your contribution. |
Thanks! Those getAssetReference() and getGiai() seem to work now with GIAI-96, great. One observation though: the GIAI-96.getAssetReference() returns a BigInt but GIAI-202.getAssetReference() returns a string now. Not sure which one was your target, but would probably be good to have identical return values in these. On a related note, I guess these changes to the return types are likely to lead to issues if someone is using these functions and they blindly update the lib version to latest. Just stating this out loud as a heads-up, hopefully no major issues arise from these changes as a side effect. |
In the case of GIAI-96.getAssetReference(), a BigInt is returned because it is the only way to represent the values stored with that length. In the case of GIAI-202.getAssetReference(), a string is returned because in this case, alpha-numeric values are allowed. In this case, the user will need to be knowledgeable about the standards and handle the values correctly. Regards! |
Ah, of course, thanks for the clarification, the limits of my knowledge about the different formats were already met it seems. But thanks a lot for the quick support with this issue, cheers! |
Hi,
Thanks for the library, very useful.
After starting to experiment with this library, I ran into problems which seem to originate from how BigInts and Numbers are being handled. I played with GIAI-96 values, but I suspect this issue might apply to many more formats as well. Case example:
As far as I could investigate this, this seems to relate to the fact that
30000000004214215 > Number.MAX_SAFE_INTEGER
. In https://github.com/sergiss/epc-tds/blob/master/epc/utils/bit-array.js#L78 a BigInt is casted to aNumber
, where things fail:I'm afraid I don't understand all the corners of the library and EPC values well-enough to provide a proper fix, but I believe BigInts should be used everywhere in the library when dealing with the values. Then, when outputting the values, perhaps a bigger question from dev experience point of view is whether e.g.
getAssetReference()
should output BigInts or strings - I guess BigInt would be more correct.Here's a simple snippet which could be added to your
test.js
for validating this case with GIAI-96 values, might be useful:The text was updated successfully, but these errors were encountered: