Skip to content

Fix ArrayIndexOutOfBoundException when check bytes in a range [0x80; 0xFF] to be a hex digit #11

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

Closed
wants to merge 1 commit into from

Conversation

Mingun
Copy link

@Mingun Mingun commented Dec 8, 2023

byte is a signed type and the [] operator takes an int as an index.
When you index array using byte index it is promoted to the int type. The most significant bit of a byte (which is a sign bit) promoted to the int resulting in a negative value. As a result ArrayIndexOutOfBoundException can be thrown.

Using a & 0xFF trick is a standard way to prevent sign promotion and treat byte as an unsigned value.

@pjfanning
Copy link
Contributor

please add some evidence to back up your claim - like a test case

xmlbeans is a widely used liband is over 20 years old and we have not received any issues like this before

@Mingun
Copy link
Author

Mingun commented Dec 8, 2023

I'll add the test if you say me where I should add it. I just wonder why this bug is still exist when I noticed it in 2017.

I think, that really this bug is not very noticable, because everything usually pass the actually HEX strings to decode methods. The first time when I encountered this problem was in 2017 and the second is today.

The test is simple:

final byte[] bug = {(byte)0xEF, (byte)0xBF};
org.apache.xmlbeans.impl.util.HexBin.decode(bug);

Result:

2023-12-08 16:10:37.61: Test execution failed: java.lang.ArrayIndexOutOfBoundsException: Index -17 out of bounds for length 255
	at org.apache.xmlbeans.impl.util.HexBin.isHex(HexBin.java:61)
	at org.apache.xmlbeans.impl.util.HexBin.decode(HexBin.java:113)
	at (...irrelevant...)

The project from which this stack uses slightly outdated xmlbeans, but this method was unchanges over 20 years.

@pjfanning
Copy link
Contributor

If you just want hex decoding there are better libs. Hex support in xmlbeans is an afterthought and may not really be used by anyone.

@Mingun
Copy link
Author

Mingun commented Dec 8, 2023

You want to say, that this class is not used in XmlBeans at all? That stack is from our extensions to XmlBeans that introduces special types to story binary data hex-encoded in XML.

@pjfanning
Copy link
Contributor

Your stack trace above hid most of the stack trace. If you are using xmlbeans because you need xmlbeans features, that's fine but we do get people using the internal methods of libraries all the time. I will commit something of my own since you have provided no tests.

@pjfanning
Copy link
Contributor

@pjfanning
Copy link
Contributor

I added f9a0ed8

@Mingun
Copy link
Author

Mingun commented Dec 8, 2023

Your stack trace above hid most of the stack trace.

The other lines is from our code, there is no xmlbeans-specific things. Yes, we're directly using this class. I'm not a maintainer of that code so I do not know how exactly it's working.

@Mingun
Copy link
Author

Mingun commented Dec 8, 2023

Thanks!

@Mingun Mingun closed this Dec 8, 2023
@pjfanning
Copy link
Contributor

I reiterate - please do not use HexBin class directly. Report it to the maintainers. It is an internal class. Find another Hex lib.

@Mingun Mingun deleted the patch-1 branch December 8, 2023 13:27
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