From 48fa0c5f40f578453d83ae36ea86914c40deba58 Mon Sep 17 00:00:00 2001 From: Andrew Chen <48723787+chuenchen309@users.noreply.github.com> Date: Sun, 19 Jul 2026 14:53:22 +0800 Subject: [PATCH 1/3] Store the color at the returned index in ImagePalette.getcolor When getcolor allocates a new entry, _new_color_index advances the index past the palette tail to skip a slot reserved for the background or transparency index. The append branch then wrote the color at the current tail but returned the advanced index, so the returned index addressed a slot that either did not exist or held a different (reserved) color, and the requested color was silently lost. Pad the skipped reserved slots so the color is stored at the index that is returned. This is a no-op when no slot was skipped. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com> --- Tests/test_imagepalette.py | 25 +++++++++++++++++++++++++ src/PIL/ImagePalette.py | 4 ++++ 2 files changed, 29 insertions(+) diff --git a/Tests/test_imagepalette.py b/Tests/test_imagepalette.py index 526beb656e8..09f588fcc5f 100644 --- a/Tests/test_imagepalette.py +++ b/Tests/test_imagepalette.py @@ -96,6 +96,31 @@ def test_getcolor_not_special(index: int, palette: ImagePalette.ImagePalette) -> assert index2 not in (index, index1) +@pytest.mark.parametrize( + "index, palette", + [ + (0, ImagePalette.ImagePalette()), + (255, ImagePalette.ImagePalette("RGB", list(range(256)) * 3)), + ], +) +def test_getcolor_not_special_stores_color( + index: int, palette: ImagePalette.ImagePalette +) -> None: + im = Image.new("P", (1, 1)) + im.info["transparency"] = index + + color = (1, 2, 3) + returned = palette.getcolor(color, im) + assert returned != index + + # The returned index must actually address the stored color, not a slot + # that was skipped to avoid the transparency/background index. + mode_len = len(palette.mode) + palette_bytes = bytes(palette.palette) + stored = tuple(palette_bytes[returned * mode_len : returned * mode_len + mode_len]) + assert stored == color + + def test_file(tmp_path: Path) -> None: palette = ImagePalette.ImagePalette("RGB", list(range(256)) * 3) diff --git a/src/PIL/ImagePalette.py b/src/PIL/ImagePalette.py index 2abbd46eaf1..b9ed95fe8c2 100644 --- a/src/PIL/ImagePalette.py +++ b/src/PIL/ImagePalette.py @@ -176,6 +176,10 @@ def getcolor( + self._palette[index * mode_len + mode_len :] ) else: + # ``index`` may have been advanced past the tail to skip a + # reserved background/transparency slot; pad those slots so + # the color is stored at the index that is returned. + self._palette += bytes(index * mode_len - len(self._palette)) self._palette += bytes(color) self.dirty = 1 return index From d809d2b197a47369ae85015106c500fa1170146b Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 19 Jul 2026 22:05:41 +1000 Subject: [PATCH 2/3] Extend palette at the same time as increasing the index --- src/PIL/ImagePalette.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/PIL/ImagePalette.py b/src/PIL/ImagePalette.py index b9ed95fe8c2..03fb4d8c4a4 100644 --- a/src/PIL/ImagePalette.py +++ b/src/PIL/ImagePalette.py @@ -125,7 +125,11 @@ def _new_color_index( image.info.get("background"), image.info.get("transparency"), ) + assert isinstance(self._palette, bytearray) while index in special_colors: + # Background or transparency index points past the end of the palette. + # Set it to black, so that the new color can be written afterwards. + self._palette += bytearray(len(self.mode)) index += 1 if index >= 256: if image: @@ -176,10 +180,6 @@ def getcolor( + self._palette[index * mode_len + mode_len :] ) else: - # ``index`` may have been advanced past the tail to skip a - # reserved background/transparency slot; pad those slots so - # the color is stored at the index that is returned. - self._palette += bytes(index * mode_len - len(self._palette)) self._palette += bytes(color) self.dirty = 1 return index From 71df484ba14c9887d8f87064212f223c98f2392c Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 19 Jul 2026 22:24:36 +1000 Subject: [PATCH 3/3] Combine new test into existing test --- Tests/test_imagepalette.py | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/Tests/test_imagepalette.py b/Tests/test_imagepalette.py index 09f588fcc5f..3f37c23c5ae 100644 --- a/Tests/test_imagepalette.py +++ b/Tests/test_imagepalette.py @@ -87,38 +87,17 @@ def test_getcolor_not_special(index: int, palette: ImagePalette.ImagePalette) -> # Do not use transparency index as a new color im.info["transparency"] = index - index1 = palette.getcolor((0, 0, 0), im) + index1 = palette.getcolor((0, 0, 1), im) assert index1 != index + roundtripped_palette = ImagePalette.ImagePalette(palette=palette.palette) + assert roundtripped_palette.colors[(0, 0, 1)] == index1 # Do not use background index as a new color im.info["background"] = index1 - index2 = palette.getcolor((0, 0, 1), im) + index2 = palette.getcolor((0, 0, 2), im) assert index2 not in (index, index1) - - -@pytest.mark.parametrize( - "index, palette", - [ - (0, ImagePalette.ImagePalette()), - (255, ImagePalette.ImagePalette("RGB", list(range(256)) * 3)), - ], -) -def test_getcolor_not_special_stores_color( - index: int, palette: ImagePalette.ImagePalette -) -> None: - im = Image.new("P", (1, 1)) - im.info["transparency"] = index - - color = (1, 2, 3) - returned = palette.getcolor(color, im) - assert returned != index - - # The returned index must actually address the stored color, not a slot - # that was skipped to avoid the transparency/background index. - mode_len = len(palette.mode) - palette_bytes = bytes(palette.palette) - stored = tuple(palette_bytes[returned * mode_len : returned * mode_len + mode_len]) - assert stored == color + roundtripped_palette = ImagePalette.ImagePalette(palette=palette.palette) + assert roundtripped_palette.colors[(0, 0, 2)] == index2 def test_file(tmp_path: Path) -> None: