Skip to content

Conversation

@radarhere
Copy link
Member

@radarhere radarhere commented Mar 20, 2025

Resolves #6639

#6640 has concerns about Pillow's limits when reading a Gimp palette file.

for _ in range(256):
s = fp.readline()

  1. Pillow reads at most 256 lines after the first header line. This means that Pillow can't read more than 256 colours, which apparently might be the case in Gimp palette files and the spec doesn't say otherwise. I'm concerned about the idea of creating a "palette" that is larger than what our ImagePalette can handle.
  2. The fact that only 256 lines are read after the first header line also means that any additional lines that are present ('Name:' header, 'Columns:' header, comments) actually subtract from the 256 colour limit.

if len(s) > 100:
msg = "bad palette file"
raise SyntaxError(msg)

3. A colour's name can't be long enough to make the line more than 100 characters.

My general concern is that the spirit of #515 is against reading an unlimited amount of data from a file.

The test files that we have do include two additional headers and a comment.

GIMP Palette
Name: custompalette
Columns: 4
#
0 0 0 Index 3

So I think extending the number of lines read by three is reasonable. That is my first commit, partly addressing point 2.

Beyond that, I think there are three options.
A. Do nothing. Palettes with more than 256 colours and long colour names are an edge case, and this is a lesser used format.
B. #6640's solution is to add class variables to allow configuration - max_colors, _max_line_size and _max_file_size. I think that makes for a bit of a complex API.
C. My idea is to add a frombytes() method that accepts bytes. That way, if the user doesn't like our limits, they can take on the responsibility of reading everything from a file, and we can just parse the result.

@aclark4life
Copy link
Member

Yeah I like frombytes too and I agree with you that adding class variables is not great.

@hugovk hugovk merged commit 095f599 into python-pillow:main Mar 27, 2025
55 checks passed
@radarhere radarhere deleted the gimp branch March 27, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Indexes of colors when reading a GIMP Palette file are wrong

3 participants