64-bit integers overflow #1

Closed
sjmulder opened this Issue Feb 17, 2011 · 6 comments

Comments

Projects
None yet
2 participants
Owner

sjmulder commented Feb 17, 2011

Values from TAG_Long tags may overflow if the value is negative or if it does not fit in 32 bits. This is a limitation of the library used for unpacking binary values.

Contributor

deathcap commented Mar 15, 2014

Found this other NBT parsing library: https://github.com/maxogden/minecraft-nbt - it also has (about to fix) problems with TAG_Long, but the jdataview https://github.com/jdataview/jdataview library it uses supports 64-bit integers by splitting them into hi and lo fields (32-bit each, allows representing integers beyond 53-bits).

Owner

sjmulder commented Mar 16, 2014

That looks like a good solution. You can submit a pull request if you feel like it, otherwise I’ll have a look soon.

sjmulder added a commit that referenced this issue Mar 16, 2014

Owner

sjmulder commented Mar 16, 2014

Actually, maybe this should be configurable in some way. It really depends on the consumer what the desired output his. hi and lo fields are one good options, but other code may want to have a clamped value or even a string.

Contributor

deathcap commented Mar 28, 2014

Hm, are there really situations where clamped values would be useful? Most 64-bit integers in NBT seem to be some kind of timestamp or identifier, where clamping could be problematic. Rounding would be problematic for identifiers, but possibly useful for timestamps.

Maybe valueOf could be implemented to return the approximate value (double, if coerced numerically), toString the precise decimal value as a string, and properties for the high and low halves.

deathcap added a commit to deathcap/nbt-js that referenced this issue Mar 29, 2014

deathcap added a commit to deathcap/nbt-js that referenced this issue Mar 29, 2014

Contributor

deathcap commented Mar 29, 2014

@sjmulder Found this nifty module https://www.npmjs.org/package/node-int64 - looks like it solves all these problems, let me know what you think. Submitted PR to use this module in #9

Owner

sjmulder commented Mar 31, 2014

That looks like a good solution, I’m surprised that the big values can be coerced to numbers. Thanks so much!

@sjmulder sjmulder closed this in #9 Mar 31, 2014

sjmulder added a commit that referenced this issue Mar 31, 2014

Merge pull request #9 from deathcap/int64
Use node-int64 for 64-bit long integers. Closes GH-1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment