FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix for 0xc4 array length in unpacker by egv · Pull Request #130 · msgpack/msgpack-java · GitHub

fix for 0xc4 array length in unpacker - #130

Merged
frsyuki merged 1 commit into
msgpack:masterfrom
egv:master
Aug 12, 2014
Merged

fix for 0xc4 array length in unpacker#130
frsyuki merged 1 commit into
msgpack:masterfrom
egv:master

Conversation

egv commented Aug 12, 2014

Copy link
Copy Markdown

on android I had problems with negative-size arrays, this fixes them

on android I had problems with negative-size arrays, this fixes them

frsyuki commented Aug 12, 2014

Copy link
Copy Markdown
Member

I think it should throw SizeLimitException rather than faking the length because using wrong length may return broken value silently.
Changing count >= rawSizeLimit condition to count < 0 || count >= rawSizeLimit will work.

egv commented Aug 12, 2014

Copy link
Copy Markdown
Author

0xc4 is a bin8, which, according to spec stores a byte array whose length is upto (2^8)-1 bytes. Byte is signed, meaning that all values greater than 127 will be negative even when cast to int, so SizeLimitException in case of negative length is incorrect, that;s why I have introduced my change.

frsyuki commented Aug 12, 2014

Copy link
Copy Markdown
Member

Oh, I see. You're right. I miss read the change.

frsyuki added a commit that referenced this pull request Aug 12, 2014
fix for 0xc4 array length in unpacker
frsyuki merged commit bd988e3 into msgpack:master Aug 12, 2014

frsyuki commented Aug 12, 2014

Copy link
Copy Markdown
Member

I've merged. Thank you for the pull-req!

egv commented Aug 12, 2014

Copy link
Copy Markdown
Author

You are welcome!

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL