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

Negative int32 values are not decoded properly. #62

Open
duaneellissd opened this issue Mar 14, 2016 · 4 comments
Open

Negative int32 values are not decoded properly. #62

duaneellissd opened this issue Mar 14, 2016 · 4 comments

Comments

@duaneellissd
Copy link

Module: encodings.js - function int32 - is wrong.
It does not properly decode negative numbers.

For example "-15" encodes as 10 bytes
0xf1 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0xff 0x01

The return value from "varint" is a giant number
As each byte is decode, the result value becomes:
BYTE is: 241 res: 0
BYTE is: 255 res: 113
BYTE is: 255 res: 16369
BYTE is: 255 res: 2097137
BYTE is: 255 res: 268435441
BYTE is: 255 res: 34359738353
BYTE is: 255 res: 4398046511089
BYTE is: 255 res: 562949953421297
BYTE is: 255 res: 72057594037927920
BYTE is: 1 res: 9223372036854776000
Result: 18446744073709552000
int32, value = 18446744073709552000 (Which is the unsigned value)

Above is the proper return value from "varint" the problem is in the encoding.js file

The 'pseudo-negative' test is correct, ie: if the value is > 2147483647 [then the value is negative]

however , the code that should adjust for a negative number is just wrong.

This is with "node.exe" windows, v4.2.6, on Win7 64bit, I have not tried other platforms.

@mafintosh
Copy link
Owner

Can you share a full runnable example that reproduces your above issue? If I encode -15 using the int32 function i get 5 bytes, [241, 255, 255, 255, 15] which, when decoded using int32, yields -15

@duaneellissd
Copy link
Author

Can't easily give you a 'working example', two reasons (1) the example comes from "protobuf-c" - a C implementation that talks to the NodeJS solution.

using the int32 function i get 5 bytes

Reason (2) The encoding scheme in your implementation is wrong, this should encode as 10 bytes. See the protocol buffers specification here: https://developers.google.com/protocol-buffers/docs/encoding#structure

The quote in the specification you are looking for is:

If you use int32 or int64 as the type for a negative number, the resulting varint is always ten bytes long – it is, effectively, treated like a very large unsigned integer.

This is the basic cause of the inefficient encoding the other pages refer to.

@duaneellissd
Copy link
Author

I presume the reference to the specification (i.e.: negative numbers encode as 10 bytes, not 5) its sufficient to explain the problem?

Do you need anything else?

Thanks.

@mafintosh
Copy link
Owner

Yep I understand it. You wanna take a stab at fixing this?

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

No branches or pull requests

2 participants