Crash when parsing _lengthy_ beacon data fields

[gerardmundo-birket]

(First off, thank you for this fantastic library. I appreciate everyone's efforts into making this library better everyday. I am truly thankful for your time invested and for sharing all your expertise)

Expected behavior

Parsing of lengthy data fields succeeds

Actual behavior

Crash due to Java Long java.lang.NumberFormatException

Steps to reproduce this behavior

In lines 644 & 645 of BeaconParser.java, a hexadecimal string is created (because the data field is >= 5 bytes) but the function Long.decode expects a radix specifier ("0x", "0X" or "#" for hexadecimal) in front of the string.

This is due to the function byteArrayToFormattedString converting data fields longer than 4 bytes to hexadecimal. Additionally, if the length is 16, it even adds dashes which can also not be "decoded" by the Long function.

Finally, if the data fields are being inserted into a Long number, I expect there to be a length limitation, but I don't see that documented (it doesn't seem to be enforced in the code either, if I am not mistaken).

I can provide examples if desired.

Android Beacon Library version

Code from Master branch

Again... thank you!

[davidgyoung]

@gerardmundo-birket, the original design of the data field is to support two beacon formats, AltBeacon and Eddystone-UID each of which have a one byte data field. As you have seen, the data field is implemented internally as a signed Java long, which would in theory allow an 8 byte signed integer data field. But also as you have seen, the byteArrayToFormattedString function has its own limitations that impact this -- it is designed for formatting beacon identifiers 16 bytes are treated as UUIDs and formatted with dashes, > 4 bytes are formatted as hex, and <= 4 bytes are formatted as decimal.

Bottom line: the data fields were specifically built and tested to handle one byte, but designed to handle more bytes. But because of the above limitations, they really only work properly with four bytes or less. This is admittedly not documented.

Possible solutions in increasing order of complexity:

1. Document a four byte limitation on data fields
2. Add code to enforce the above limitation by throwing an exception in BeaconParser
3. Add code to expand support for data field lengths up to the 8 byte length of a Java long. This is a small fix that would require no API changes
4. Add code to expand support for longer data field lengths. This would require API changes to provide larger storage for the data fields internally and new accessors that operate on something other than a Java long.

It certainly makes sense to implement 1 and 2. If your use case requires a longer data field, I am open to you creating a pull request for 3 or 4 above. If your use case needs 4, however, we should discuss the details of the internal and API changes first to keep the impact as small as possible.

[gerardmundo-birket]

@davidgyoung , thank you for your prompt response.

For my use case, I am still experimenting with different options, but I am interested in code efficiency and after seeing the overhead of the parsing of data fields and the formatting, I think my best option is if I work with the raw packet data directly (I'm brave like that!).

I think option 1+2+3 will be the best experience for most users of the library, though. I'll update this thread in a few days with whatever I settle with, but I'm happy to keep the discussion going in the mean time.

[gerardmundo-birket]

David,

After testing for a while, I'll be using the raw bytes directly.

I think 1+2+3 will be the most reasonable solution, should you want to address this problem.

Thank you again for your promptness, and your time and dedication! It's a great library you've got here.

PS: are there other limits on other types of fields, for example the matching field?