From e817ed0d3eb838e63836dd5f976a007a9775ce89 Mon Sep 17 00:00:00 2001 From: Jon Dufresne Date: Sun, 3 Nov 2019 12:51:40 -0800 Subject: [PATCH 1/3] Correct str/bytes mixup in ContainerIO Image data is expected to be read in bytes mode, not text mode so ContainerIO should return bytes in all methods. The passed in file handler is expected to be opened in bytes mode (as TarIO already does). --- Tests/test_file_container.py | 40 ++++++++++++++++++------------------ src/PIL/ContainerIO.py | 6 +++--- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/Tests/test_file_container.py b/Tests/test_file_container.py index 91166b39e72..d494e10880a 100644 --- a/Tests/test_file_container.py +++ b/Tests/test_file_container.py @@ -20,7 +20,7 @@ def test_isatty(): def test_seek_mode_0(): # Arrange mode = 0 - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -34,7 +34,7 @@ def test_seek_mode_0(): def test_seek_mode_1(): # Arrange mode = 1 - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -48,7 +48,7 @@ def test_seek_mode_1(): def test_seek_mode_2(): # Arrange mode = 2 - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -61,7 +61,7 @@ def test_seek_mode_2(): def test_read_n0(): # Arrange - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -69,12 +69,12 @@ def test_read_n0(): data = container.read() # Assert - assert data == "7\nThis is line 8\n" + assert data == b"7\nThis is line 8\n" def test_read_n(): # Arrange - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -82,12 +82,12 @@ def test_read_n(): data = container.read(3) # Assert - assert data == "7\nT" + assert data == b"7\nT" def test_read_eof(): # Arrange - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 22, 100) # Act @@ -95,34 +95,34 @@ def test_read_eof(): data = container.read() # Assert - assert data == "" + assert data == b"" def test_readline(): # Arrange - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 0, 120) # Act data = container.readline() # Assert - assert data == "This is line 1\n" + assert data == b"This is line 1\n" def test_readlines(): # Arrange expected = [ - "This is line 1\n", - "This is line 2\n", - "This is line 3\n", - "This is line 4\n", - "This is line 5\n", - "This is line 6\n", - "This is line 7\n", - "This is line 8\n", + b"This is line 1\n", + b"This is line 2\n", + b"This is line 3\n", + b"This is line 4\n", + b"This is line 5\n", + b"This is line 6\n", + b"This is line 7\n", + b"This is line 8\n", ] - with open(TEST_FILE) as fh: + with open(TEST_FILE, "rb") as fh: container = ContainerIO.ContainerIO(fh, 0, 120) # Act diff --git a/src/PIL/ContainerIO.py b/src/PIL/ContainerIO.py index 9727601abc3..8e904121000 100644 --- a/src/PIL/ContainerIO.py +++ b/src/PIL/ContainerIO.py @@ -82,7 +82,7 @@ def read(self, n=0): else: n = self.length - self.pos if not n: # EOF - return "" + return b"" self.pos = self.pos + n return self.fh.read(n) @@ -92,13 +92,13 @@ def readline(self): :returns: An 8-bit string. """ - s = "" + s = b"" while True: c = self.read(1) if not c: break s = s + c - if c == "\n": + if c == b"\n": break return s From f958e2f8ed7b12582836a6b6c83468c51230182a Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 7 Jan 2020 21:06:02 +1100 Subject: [PATCH 2/3] Return strings or bytes from ContainerIO according to the file object mode --- Tests/test_file_container.py | 106 ++++++++++++++++++++--------------- src/PIL/ContainerIO.py | 6 +- 2 files changed, 63 insertions(+), 49 deletions(-) diff --git a/Tests/test_file_container.py b/Tests/test_file_container.py index d494e10880a..b752e217faa 100644 --- a/Tests/test_file_container.py +++ b/Tests/test_file_container.py @@ -61,73 +61,87 @@ def test_seek_mode_2(): def test_read_n0(): # Arrange - with open(TEST_FILE, "rb") as fh: - container = ContainerIO.ContainerIO(fh, 22, 100) + for bytesmode in (True, False): + with open(TEST_FILE, "rb" if bytesmode else "r") as fh: + container = ContainerIO.ContainerIO(fh, 22, 100) - # Act - container.seek(81) - data = container.read() + # Act + container.seek(81) + data = container.read() - # Assert - assert data == b"7\nThis is line 8\n" + # Assert + if bytesmode: + data = data.decode() + assert data == "7\nThis is line 8\n" def test_read_n(): # Arrange - with open(TEST_FILE, "rb") as fh: - container = ContainerIO.ContainerIO(fh, 22, 100) + for bytesmode in (True, False): + with open(TEST_FILE, "rb" if bytesmode else "r") as fh: + container = ContainerIO.ContainerIO(fh, 22, 100) - # Act - container.seek(81) - data = container.read(3) + # Act + container.seek(81) + data = container.read(3) - # Assert - assert data == b"7\nT" + # Assert + if bytesmode: + data = data.decode() + assert data == "7\nT" def test_read_eof(): # Arrange - with open(TEST_FILE, "rb") as fh: - container = ContainerIO.ContainerIO(fh, 22, 100) + for bytesmode in (True, False): + with open(TEST_FILE, "rb" if bytesmode else "r") as fh: + container = ContainerIO.ContainerIO(fh, 22, 100) - # Act - container.seek(100) - data = container.read() + # Act + container.seek(100) + data = container.read() - # Assert - assert data == b"" + # Assert + if bytesmode: + data = data.decode() + assert data == "" def test_readline(): # Arrange - with open(TEST_FILE, "rb") as fh: - container = ContainerIO.ContainerIO(fh, 0, 120) + for bytesmode in (True, False): + with open(TEST_FILE, "rb" if bytesmode else "r") as fh: + container = ContainerIO.ContainerIO(fh, 0, 120) - # Act - data = container.readline() + # Act + data = container.readline() - # Assert - assert data == b"This is line 1\n" + # Assert + if bytesmode: + data = data.decode() + assert data == "This is line 1\n" def test_readlines(): # Arrange - expected = [ - b"This is line 1\n", - b"This is line 2\n", - b"This is line 3\n", - b"This is line 4\n", - b"This is line 5\n", - b"This is line 6\n", - b"This is line 7\n", - b"This is line 8\n", - ] - with open(TEST_FILE, "rb") as fh: - container = ContainerIO.ContainerIO(fh, 0, 120) - - # Act - data = container.readlines() - - # Assert - - assert data == expected + for bytesmode in (True, False): + expected = [ + "This is line 1\n", + "This is line 2\n", + "This is line 3\n", + "This is line 4\n", + "This is line 5\n", + "This is line 6\n", + "This is line 7\n", + "This is line 8\n", + ] + with open(TEST_FILE, "rb" if bytesmode else "r") as fh: + container = ContainerIO.ContainerIO(fh, 0, 120) + + # Act + data = container.readlines() + + # Assert + if bytesmode: + data = [line.decode() for line in data] + assert data == expected diff --git a/src/PIL/ContainerIO.py b/src/PIL/ContainerIO.py index 8e904121000..48c0081fc9e 100644 --- a/src/PIL/ContainerIO.py +++ b/src/PIL/ContainerIO.py @@ -82,7 +82,7 @@ def read(self, n=0): else: n = self.length - self.pos if not n: # EOF - return b"" + return b"" if "b" in self.fh.mode else "" self.pos = self.pos + n return self.fh.read(n) @@ -92,13 +92,13 @@ def readline(self): :returns: An 8-bit string. """ - s = b"" + s = b"" if "b" in self.fh.mode else "" while True: c = self.read(1) if not c: break s = s + c - if c == b"\n": + if c == (b"\n" if "b" in self.fh.mode else "\n"): break return s From 8acf77a0420fbf244684d5735a30747123a3b543 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sat, 15 Feb 2020 20:53:02 +1100 Subject: [PATCH 3/3] For effiency, set newline character outside of loop --- src/PIL/ContainerIO.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PIL/ContainerIO.py b/src/PIL/ContainerIO.py index 48c0081fc9e..5bb0086f6e7 100644 --- a/src/PIL/ContainerIO.py +++ b/src/PIL/ContainerIO.py @@ -93,12 +93,13 @@ def readline(self): :returns: An 8-bit string. """ s = b"" if "b" in self.fh.mode else "" + newline_character = b"\n" if "b" in self.fh.mode else "\n" while True: c = self.read(1) if not c: break s = s + c - if c == (b"\n" if "b" in self.fh.mode else "\n"): + if c == newline_character: break return s