Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved handling of PPM header #5121

Merged
merged 23 commits into from Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
51 changes: 50 additions & 1 deletion Tests/test_file_ppm.py
@@ -1,6 +1,6 @@
import pytest

from PIL import Image
from PIL import Image, UnidentifiedImageError

from .helper import assert_image_equal, assert_image_similar, hopper

Expand Down Expand Up @@ -50,6 +50,55 @@ def test_pnm(tmp_path):
assert_image_equal(im, reloaded)


def test_not_ppm(tmp_path):
path = str(tmp_path / "temp.ppm")
with open(path, "wb") as f:
f.write(b"PyInvalid")

with pytest.raises(UnidentifiedImageError):
with Image.open(path):
pass
radarhere marked this conversation as resolved.
Show resolved Hide resolved


radarhere marked this conversation as resolved.
Show resolved Hide resolved
def test_header_with_comments(tmp_path):
path = str(tmp_path / "temp.ppm")
with open(path, "wb") as f:
f.write(b"P6 #comment\n#comment\r12#comment\r8\n128 #comment\n255\n")

with Image.open(path) as im:
assert im.size == (128, 128)


def test_nondecimal_header(tmp_path):
path = str(tmp_path / "temp.ppm")
with open(path, "wb") as f:
f.write(b"P6\n128\x00")

with pytest.raises(ValueError):
with Image.open(path):
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite on board with this one. The idea here is that b"\x00" is not caught by

if c > b"\x79":
raise ValueError("Expected ASCII value, found binary")

even though \x00 is not greater than \x79, and only for it to be raised as a different ValueError later when it turns out that b"128\x00" is not an integer.

There isn't a valid scenario where the token/magic needs to be something other than a number or a letter. I've pushed a commit to change this test into a check that a ValueError is raised when a token is not an integer.



def test_token_too_long(tmp_path):
path = str(tmp_path / "temp.ppm")
with open(path, "wb") as f:
f.write(b"P6\n 0123456789")

with pytest.raises(ValueError):
with Image.open(path):
pass


def test_too_many_colors(tmp_path):
path = str(tmp_path / "temp.ppm")
with open(path, "wb") as f:
f.write(b"P6\n1 1\n1000\n")

with pytest.raises(ValueError):
with Image.open(path):
pass


def test_truncated_file(tmp_path):
path = str(tmp_path / "temp.pgm")
with open(path, "w") as f:
Expand Down
94 changes: 58 additions & 36 deletions src/PIL/PpmImagePlugin.py
Expand Up @@ -20,7 +20,7 @@
#
# --------------------------------------------------------------------

b_whitespace = b"\x20\x09\x0a\x0b\x0c\x0d"
B_WHITESPACE = b"\x20\x09\x0a\x0b\x0c\x0d"

MODES = {
# standard
Expand Down Expand Up @@ -49,26 +49,52 @@ class PpmImageFile(ImageFile.ImageFile):
format = "PPM"
format_description = "Pbmplus image"

def _token(self, s=b""):
def _read_magic(self, magic=b""):
while True: # read until next whitespace
c = self.fp.read(1)
if not c or c in b_whitespace:
if c in B_WHITESPACE:
radarhere marked this conversation as resolved.
Show resolved Hide resolved
break
if c > b"\x79":
raise ValueError("Expected ASCII value, found binary")
radarhere marked this conversation as resolved.
Show resolved Hide resolved
s = s + c
if len(s) > 9:
raise ValueError("Expected int, got > 9 digits")
return s
magic += c
if len(magic) > 6: # exceeded max magic number length
break
return magic

def _open(self):
def _read_token(self, token=b""):
def _ignore_comment(): # ignores rest of the line; stops at CR, LF or EOF
while self.fp.read(1) not in b"\r\n":
pass

while True: # read until non-whitespace is found
c = self.fp.read(1)
if c == b"#": # found comment, ignore it
_ignore_comment()
radarhere marked this conversation as resolved.
Show resolved Hide resolved
continue
if c in B_WHITESPACE: # found whitespace, ignore it
if c == b"": # reached EOF
raise ValueError("Reached EOF while reading header")
continue
break

# check magic
s = self.fp.read(1)
if s != b"P":
raise SyntaxError("not a PPM file")
radarhere marked this conversation as resolved.
Show resolved Hide resolved
magic_number = self._token(s)
mode = MODES[magic_number]
token += c

while True: # read until next whitespace
c = self.fp.read(1)
if c == b"#":
_ignore_comment()
continue
if c in B_WHITESPACE: # token ended
break
token += c
if len(token) > 10:
raise ValueError(f"Token too long in file header: {token}")
return token

def _open(self):
magic_number = self._read_magic()
try:
mode = MODES[magic_number]
except KeyError:
raise SyntaxError("Not a PPM image file") from None

self.custom_mimetype = {
b"P4": "image/x-portable-bitmap",
Expand All @@ -83,29 +109,25 @@ def _open(self):
self.mode = rawmode = mode

for ix in range(3):
while True:
while True:
s = self.fp.read(1)
if s not in b_whitespace:
break
if s == b"":
raise ValueError("File does not extend beyond magic number")
if s != b"#":
break
s = self.fp.readline()
s = int(self._token(s))
if ix == 0:
xsize = s
elif ix == 1:
ysize = s
token = self._read_token()
try: # check token sanity
token = int(token)
except ValueError:
raise ValueError(
f"Non-decimal-ASCII found in header: {token}"
) from None
if ix == 0: # token is the x size
xsize = token
elif ix == 1: # token is the y size
ysize = token
if mode == "1":
break
elif ix == 2:
# maxgrey
if s > 255:
elif ix == 2: # token is maxval
maxval = token
if maxval > 255:
if not mode == "L":
raise ValueError(f"Too many colors for band: {s}")
if s < 2 ** 16:
raise ValueError(f"Too many colors for band: {token}")
radarhere marked this conversation as resolved.
Show resolved Hide resolved
if token < 2 ** 16:
self.mode = "I"
rawmode = "I;16B"
else:
Expand Down