From 5d618ec02a4b81d6c418132e97c45bf02979b3ed Mon Sep 17 00:00:00 2001 From: chuenchen309 <48723787+chuenchen309@users.noreply.github.com> Date: Sun, 19 Jul 2026 09:45:17 +0800 Subject: [PATCH 1/2] Derive PCX plane count from the image, not the line width When decoding a multi-plane (e.g. RGB) PCX scanline, the decoder recovered the per-plane stride by guessing the band count as state->bytes / xsize. state->bytes is planes * stride, so when the padded stride differs from the width (an odd-width line padded to an even stride) that quotient is wrong: for a width-3 RGB line it gives 4 bands / stride 3, the plane-compaction step is skipped, and the R/G/B planes stay interleaved. The pixels decode to the wrong colours with no error raised. Use the known band count (im->bands) to split the line instead. Added an RGB round-trip regression at a width that triggers the padding. Co-Authored-By: Claude Opus 4.8 (1M context) --- Tests/test_file_pcx.py | 8 ++++++++ src/libImaging/PcxDecode.c | 10 ++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/Tests/test_file_pcx.py b/Tests/test_file_pcx.py index 76fd09dac9b..4cba9ac857b 100644 --- a/Tests/test_file_pcx.py +++ b/Tests/test_file_pcx.py @@ -118,6 +118,14 @@ def test_1px_width(tmp_path: Path) -> None: _roundtrip(tmp_path, im) +def test_rgb_odd_width(tmp_path: Path) -> None: + # An RGB (multi-plane) width whose even-padded stride differs from the + # width must still separate the colour planes correctly. Distinct channel + # values expose a misaligned split. + im = Image.new("RGB", (3, 3), (0x11, 0x22, 0x33)) + _roundtrip(tmp_path, im) + + def test_large_count(tmp_path: Path) -> None: im = Image.new("L", (256, 1)) px = im.load() diff --git a/src/libImaging/PcxDecode.c b/src/libImaging/PcxDecode.c index a65952fb1da..f29feaa701c 100644 --- a/src/libImaging/PcxDecode.c +++ b/src/libImaging/PcxDecode.c @@ -69,10 +69,12 @@ ImagingPcxDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t byt stride = state->bytes / state->bits; } else { xsize = state->xsize; - bands = state->bytes / state->xsize; - if (bands != 0) { - stride = state->bytes / bands; - } + // state->bytes is planes * stride; derive the stride from the + // known band count rather than guessing it from state->xsize, + // which picks the wrong split when xsize != stride (e.g. an + // odd-width RGB line padded to an even stride). + bands = im->bands; + stride = state->bytes / bands; } if (stride > xsize) { int i; From 2f3f7ff6bcd36e83b8fbc6b064611b4085af666d Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 28 Jul 2026 22:39:53 +1000 Subject: [PATCH 2/2] Simplified code --- Tests/test_file_pcx.py | 13 +++---------- src/libImaging/PcxDecode.c | 8 +------- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/Tests/test_file_pcx.py b/Tests/test_file_pcx.py index 4cba9ac857b..c3573bbfccc 100644 --- a/Tests/test_file_pcx.py +++ b/Tests/test_file_pcx.py @@ -80,13 +80,14 @@ def test_invalid_file() -> None: @pytest.mark.parametrize("mode", ("1", "L", "P", "RGB")) -def test_odd(tmp_path: Path, mode: str) -> None: +@pytest.mark.parametrize("size", (3, 511)) +def test_odd(tmp_path: Path, mode: str, size: int) -> None: # See issue #523, odd sized images should have a stride that's even. # Not that ImageMagick or GIMP write PCX that way. # We were not handling properly. # larger, odd sized images are better here to ensure that # we handle interrupted scan lines properly. - _roundtrip(tmp_path, hopper(mode).resize((511, 511))) + _roundtrip(tmp_path, hopper(mode).resize((size, size))) def test_odd_read() -> None: @@ -118,14 +119,6 @@ def test_1px_width(tmp_path: Path) -> None: _roundtrip(tmp_path, im) -def test_rgb_odd_width(tmp_path: Path) -> None: - # An RGB (multi-plane) width whose even-padded stride differs from the - # width must still separate the colour planes correctly. Distinct channel - # values expose a misaligned split. - im = Image.new("RGB", (3, 3), (0x11, 0x22, 0x33)) - _roundtrip(tmp_path, im) - - def test_large_count(tmp_path: Path) -> None: im = Image.new("L", (256, 1)) px = im.load() diff --git a/src/libImaging/PcxDecode.c b/src/libImaging/PcxDecode.c index f29feaa701c..d060be3e7bc 100644 --- a/src/libImaging/PcxDecode.c +++ b/src/libImaging/PcxDecode.c @@ -62,20 +62,14 @@ ImagingPcxDecode(Imaging im, ImagingCodecState state, UINT8 *buf, Py_ssize_t byt if (state->x >= state->bytes) { int bands; int xsize = 0; - int stride = 0; if (state->bits == 2 || state->bits == 4) { xsize = (state->xsize + 7) / 8; bands = state->bits; - stride = state->bytes / state->bits; } else { xsize = state->xsize; - // state->bytes is planes * stride; derive the stride from the - // known band count rather than guessing it from state->xsize, - // which picks the wrong split when xsize != stride (e.g. an - // odd-width RGB line padded to an even stride). bands = im->bands; - stride = state->bytes / bands; } + int stride = state->bytes / bands; if (stride > xsize) { int i; for (i = 1; i < bands; i++) { // note -- skipping first band