[bacescuandrei]

XPM colours can use 1 to 4 hex digits per channel (#RGB, #RRGGBB, #RRRGGGBBB,
#RRRRGGGGBBBB): libXpm passes colour strings to Xlib's XParseColor, which accepts these four
forms. XpmImagePlugin parsed the value as one integer and used its lowest 24 bits, so only
#RRGGBB was right:

colour	before	after
#F00	(0, 15, 0)	(240, 0, 0)
#FFFF00000000	(0, 0, 0)	(255, 0, 0)
#C71BC30BC71B	(11, 199, 27)	(199, 195, 199)
#123456789	(69, 103, 137)	(18, 69, 120)

The 12-digit form is used in real files, for example wanderlust etc/icons/news.xpm
(#9E799A699E79) and openbabel src/GUI/babel.xpm (#FFFFF7F7E7E7).

Changes proposed in this pull request:

• Decode 1, 3 and 4 digit colours as XParseColor does: the digits are the most significant
  bits of each 16-bit channel ("#3a7 is the same as #3000a0007000", XParseColor(3)), and the
  top 8 bits are used, as an X server does for a 24-bit visual. #RRGGBB is unchanged.
• Add a parametrized test.

Tests (Ubuntu 24.04, Python 3.12): Tests/test_file_xpm.py 16 passed; the full suite before the
last commit: 5724 passed, 89 skipped, 3 xfailed. ruff check and black 26.10.0 are clean.

[akx]

Can you point out an XPM spec, or an XPM file in the wild, that shows these new color formats?

All I see in https://www.xfree86.org/current/xpm.pdf (though I don't know if there's a newer spec?) is

Colors can be specified by giving the colorname, a # followed by the RGB code in hexadecimal, or a % followed by the HSV code (not implemented).

[bacescuandrei]

The XPM manual doesn't define the hex syntax itself; libXpm hands the colour string to Xlib's XParseColor (src/create.c in libXpm). XParseColor(3) accepts #RGB, #RRGGBB, #RRRGGGBBB and #RRRRGGGGBBBB, and says: "When fewer than 16 bits each are specified, they represent the most significant bits of the value", e.g. #3a7 is the same as #3000a0007000. libX11's src/ParseCol.c accepts exactly 3, 6, 9 or 12 digits.

Files in the wild use the 12-digit form, for example:

• wanderlust etc/icons/news.xpm: c #9E799A699E79, c #CF3CD34CCF3C
• openbabel src/GUI/babel.xpm: c #FFFFF7F7E7E7, c #FFFFEFEFCECE

Pillow currently reads #9E799A699E79 as (105, 158, 121), the lowest 24 bits; libXpm gives (158, 154, 158).

Looking into this, I changed the conversion. Before, it scaled the values as ImageMagick does, so #F00 gave 255. Now it follows XParseColor: the digits become the most significant bits, so #F00 gives 240, and a 12-digit value keeps its top byte. That is also what an X server does on a 24-bit visual (miResolveColor keeps the top 8 bits). For the real colours above, and the test values, the result matches a port of ParseCol.c + miResolveColor; #RRGGBB is unchanged.

[Muguett-DBY]

Could this check that the hex length is exactly 3, 6, 9, or 12 before constructing the palette? A malformed colour can otherwise silently shift the next valid colour:

from io import BytesIO
from PIL import Image

data = b'''/* XPM */
static char *test[] = {
"2 1 2 1",
"a c #1234",
"b c #00ff00",
"ab"};
'''
with Image.open(BytesIO(data)) as im:
    rgb = im.convert("RGB")
    print(rgb.getpixel((1, 0)))

On the base this second pixel is green (0, 255, 0); with this patch it becomes (64, 0, 255). Four digits produce four bytes for the first palette entry, misaligning the following entry. Rejecting the invalid colour would avoid returning a corrupted image.

I exercised the exact base and PR _open methods using the existing Pillow 12.3 decoder; I did not run the full test suite.

[bacescuandrei]

Thanks, confirmed. Since e59890c every # colour goes through the per-channel decoding, so #1234 gives four palette bytes and the next colour is shifted.

I pushed a fix that only decodes 3, 6, 9 or 12 digits per channel (the lengths XParseColor accepts) and keeps the previous lowest-24-bits reading for other lengths. Your example now gives (0, 255, 0) for the second pixel again, and a new test covers #1234 and #12345678.

I didn't reject other lengths: test_undefined_color_key has a colour line without a trailing comma, so the plugin sees #00000 there, and rejecting it would change what main currently accepts.