RFR: 8373727: New XBM images parser regression: only the first line of the bitmap array is parsed [v3]

[Alexey Ivanov]

On Mon, 12 Jan 2026 20:00:52 GMT, Damon Nguyen <dnguyen at openjdk.org> 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

src/java.desktop/share/classes/sun/awt/image/XbmImageDecoder.java line 118:

> 116:                             } else if (token[1].endsWith("t")) {
> 117:                                 H = Integer.parseInt(token[2]);
> 118:                             }

~~Wouldn't it be better to use `width` and `height` in `endsWith`? The code would be clearer.~~ But it will break backwards compatibility.

I submitted [JDK-8377924](https://bugs.openjdk.org/browse/JDK-8377924 "Inconsistent parsing of XBM files after JDK-8361748") to address inconsistency with parsing `.xbm` files after [JDK-8361748](https://bugs.openjdk.org/browse/JDK-8361748) and [JDK-8373727](https://bugs.openjdk.org/browse/JDK-8373727): the old code used `-h` and `-ht` as the markers for width and height correspondingly.

src/java.desktop/share/classes/sun/awt/image/XbmImageDecoder.java line 127:

> 125:                             }
> 126:                             state = 2; // after second dimension is set
> 127:                         }

~~Isn't a problem that state 2 can be reached where only `W` or `H` are set?~~

I found the answer myself: `state == 2 && (W <= 0 || H <= 0)` will throw an error.

test/jdk/java/awt/image/XBMDecoder/XBMDecoderTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2026, Oracle and/or its affiliates. All rights reserved.

It should've become `(c) 2025, 2026, Oracle`.

Addressed in #29769.

test/jdk/java/awt/image/XBMDecoder/invalid_empty.xbm line 1:

> 1: #define test_width 16

Why is `invalid_empty.xbm` invalid?

If it is, then an exception should be thrown while decoding the image, but no exception is thrown.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29120#discussion_r2747895862
PR Review Comment: https://git.openjdk.org/jdk/pull/29120#discussion_r2732166730
PR Review Comment: https://git.openjdk.org/jdk/pull/29120#discussion_r2819266987
PR Review Comment: https://git.openjdk.org/jdk/pull/29120#discussion_r2818621385