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

Conversion error on large number (> 32bit) #221

Open
pauldeng opened this issue Mar 23, 2023 · 3 comments
Open

Conversion error on large number (> 32bit) #221

pauldeng opened this issue Mar 23, 2023 · 3 comments
Labels

Comments

@pauldeng
Copy link

Hi all,

Firstly, I like to appreciate the author for this lib. Understand it is the most downloaded lib for this function and has been stable for years.

Maybe I missed out something obvious, can someone in the community tell me what happens here:

// tested on nodejs v14, v18
const sprintf = require('sprintf-js');

let num = 150460469257; // 0x2308249009

// simpliy convert number to hex string

const hexStr_native = num.toString(16);
const hexStr_sprintfjs = sprintf.sprintf("%x", num);

console.log(hexStr_native);    // 2308249009 as expected
console.log(hexStr_sprintfjs); // expecting 2308249009, but get 8249009. Seems only max 4 bytes (32 bit) number allowed, larger than that will be trimmed. I am not aware this lib has this restriction.

Thanks.

Cheers,
Paul

@alexei
Copy link
Owner

alexei commented Mar 23, 2023

I believe that happens because the library uses the right shift operator to ensure the number is positive:

sprintf.js/src/sprintf.js

Lines 114 to 116 in 8a3a4bb

case 'x':
arg = (parseInt(arg, 10) >>> 0).toString(16)
break

In doing so, the value is converted to a 32 bits integer. But 150460469257 is greater than the largest 32-bits integer, signed or unsigned, so I guess it overflows.

I don't think I've ever thought of a use for hexadecimal notation beyond representing colors. In such cases 32 bits are sufficient. However, this limitation of the library seems like an unfortunate accident and a bug.

@alexei alexei added the bug label Mar 23, 2023
@pauldeng
Copy link
Author

pauldeng commented Mar 24, 2023

Thanks for your reply.

Perhaps %x, %lX can be introduced, just like how C handles long type.

Cheers,
Paul

@alexei
Copy link
Owner

alexei commented Mar 24, 2023

Wouldn't you say it would be better to use Math.abs to ensure the value is positive, instead of introducing features which are particular to C but strange to JS?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants