On Mon, 12 Jan 2026 20:00:52 GMT, Damon Nguyen <[email protected]> wrote:

>> There were three concerns with the previous changes to `XbmImageDecoder`.
>> 1. Only the first line of the bit array was read.
>> 2. The regex was incorrect because it used `[]` instead of `()` for some 
>> grouping.
>> 3. The ordering of the width and height in the xbm file was too strict and 
>> had to be width first, then height.
>> 
>> To fix these issues, I have:
>> 1. Used a StringBuilder to append lines of the bit array to parse the entire 
>> array at once. This required changing the parsing loop and moving some code.
>> 2. Updated the regex to capture starting whitespace, optionally start with 
>> `0x`, include all chars/digits/punctuation (minus `,` and `};`) instead of 
>> just valid hex digits, and end with either `,` or `};`. This also allows for 
>> incorrect hex values such as `0x12345abcde` to be detected since the new 
>> regex allows for multiple chars after the `0x` until the next delimiter is 
>> found. This is accounted for in the test xbm files.
>> 3. Determined width or height based off of ending letters (`th` or `t`). 
>> This is similar to 
>> https://github.com/openjdk/jdk/blob/jdk-26+10/src/java.desktop/share/classes/sun/awt/image/XbmImageDecoder.java#L109-L112.
>> 
>> A new method is added to `XBMDecoderTest.java` to better check that the bit 
>> array parsing works. This method checks for non-empty pixel data.
>> 
>> A new xbm file is also added to check for the case where `0xAB+` exists in 
>> the bit array. This should be invalid, and the previous regex would allow 
>> this to pass. 
>> 
>> All tests pass with the new changes made here.
>
> Damon Nguyen has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Add new xbm file to test for multiple lines in bit array

LGTM. Nice to have better test coverage.

-------------

Marked as reviewed by jdv (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/29120#pullrequestreview-3653884345

Reply via email to