Patch #1675424: Added tests for uncovered code in the zipfile module.
The KeyError raised by Zipfile.getinfo for nonexistent names now has
a descriptive message.
diff --git a/Doc/lib/libzipfile.tex b/Doc/lib/libzipfile.tex
index 6cdd1d3..366cee9 100644
--- a/Doc/lib/libzipfile.tex
+++ b/Doc/lib/libzipfile.tex
@@ -24,8 +24,8 @@
The available attributes of this module are:
-\begin{excdesc}{error}
- The error raised for bad ZIP files.
+\begin{excdesc}{BadZipfile}
+ The error raised for bad ZIP files (old name: \code{zipfile.error}).
\end{excdesc}
\begin{excdesc}{LargeZipFile}
@@ -90,7 +90,7 @@
(a string) or a file-like object. The \var{mode} parameter
should be \code{'r'} to read an existing file, \code{'w'} to
truncate and write a new file, or \code{'a'} to append to an
- existing file. For \var{mode} is \code{'a'} and \var{file}
+ existing file. If \var{mode} is \code{'a'} and \var{file}
refers to an existing ZIP file, then additional files are added to
it. If \var{file} does not refer to a ZIP file, then a new ZIP
archive is appended to the file. This is meant for adding a ZIP
@@ -128,7 +128,8 @@
\begin{methoddesc}{getinfo}{name}
Return a \class{ZipInfo} object with information about the archive
- member \var{name}.
+ member \var{name}. Calling \method{getinfo()} for a name not currently
+ contained in the archive will raise a \exception{KeyError}.
\end{methoddesc}
\begin{methoddesc}{infolist}{}
@@ -147,7 +148,9 @@
parameter, if included, must be one of the following: \code{'r'} (the
default), \code{'U'}, or \code{'rU'}. Choosing \code{'U'} or
\code{'rU'} will enable universal newline support in the read-only
- object. \var{pwd} is the password used for encrypted files.
+ object. \var{pwd} is the password used for encrypted files. Calling
+ \method{open()} on a closed ZipFile will raise a
+ \exception{RuntimeError}.
\begin{notice}
The file-like object is read-only and provides the following methods:
\method{read()}, \method{readline()}, \method{readlines()},
@@ -182,7 +185,8 @@
Return the bytes of the file in the archive. The archive must be
open for read or append. \var{pwd} is the password used for encrypted
files and, if specified, it will override the default password set with
- \method{setpassword()}.
+ \method{setpassword()}. Calling \method{read()} on a closed ZipFile
+ will raise a \exception{RuntimeError}.
\versionchanged[\var{pwd} was added]{2.6}
\end{methoddesc}
@@ -190,6 +194,8 @@
\begin{methoddesc}{testzip}{}
Read all the files in the archive and check their CRC's and file
headers. Return the name of the first bad file, or else return \code{None}.
+ Calling \method{testzip()} on a closed ZipFile will raise a
+ \exception{RuntimeError}.
\end{methoddesc}
\begin{methoddesc}{write}{filename\optional{, arcname\optional{,
@@ -200,7 +206,10 @@
separators removed). If given, \var{compress_type} overrides the
value given for the \var{compression} parameter to the constructor
for the new entry. The archive must be open with mode \code{'w'}
- or \code{'a'}.
+ or \code{'a'} -- calling \method{write()} on a ZipFile created with
+ mode \code{'r'} will raise a \exception{RuntimeError}. Calling
+ \method{write()} on a closed ZipFile will raise a
+ \exception{RuntimeError}.
\note{There is no official file name encoding for ZIP files.
If you have unicode file names, please convert them to byte strings
@@ -210,6 +219,11 @@
\note{Archive names should be relative to the archive root, that is,
they should not start with a path separator.}
+
+ \note{If \code{arcname} (or \code{filename}, if \code{arcname} is
+ not given) contains a null byte, the name of the file in the archive will
+ be truncated at the null byte.}
+
\end{methoddesc}
\begin{methoddesc}{writestr}{zinfo_or_arcname, bytes}
@@ -218,7 +232,10 @@
\class{ZipInfo} instance. If it's an instance, at least the
filename, date, and time must be given. If it's a name, the date
and time is set to the current date and time. The archive must be
- opened with mode \code{'w'} or \code{'a'}.
+ opened with mode \code{'w'} or \code{'a'} -- calling
+ \method{writestr()} on a ZipFile created with mode \code{'r'}
+ will raise a \exception{RuntimeError}. Calling \method{writestr()}
+ on a closed ZipFile will raise a \exception{RuntimeError}.
\end{methoddesc}
@@ -243,12 +260,13 @@
available, else a \file{*.pyc} file, compiling if necessary. If the
pathname is a file, the filename must end with \file{.py}, and just
the (corresponding \file{*.py[co]}) file is added at the top level
- (no path information). If it is a directory, and the directory is
- not a package directory, then all the files \file{*.py[co]} are
- added at the top level. If the directory is a package directory,
- then all \file{*.py[oc]} are added under the package name as a file
- path, and if any subdirectories are package directories, all of
- these are added recursively. \var{basename} is intended for
+ (no path information). If the pathname is a file that does not end with
+ \file{.py}, a \exception{RuntimeError} will be raised. If it is a
+ directory, and the directory is not a package directory, then all the
+ files \file{*.py[co]} are added at the top level. If the directory is
+ a package directory, then all \file{*.py[co]} are added under the package
+ name as a file path, and if any subdirectories are package directories, all
+ of these are added recursively. \var{basename} is intended for
internal use only. The \method{writepy()} method makes archives
with file names like this:
diff --git a/Lib/test/test_zipfile.py b/Lib/test/test_zipfile.py
index 49602ff..3d2f9bd 100644
--- a/Lib/test/test_zipfile.py
+++ b/Lib/test/test_zipfile.py
@@ -14,12 +14,12 @@
from test.test_support import TESTFN, run_unittest
TESTFN2 = TESTFN + "2"
-FIXEDTEST_SIZE = 10
+FIXEDTEST_SIZE = 1000
class TestsWithSourceFile(unittest.TestCase):
def setUp(self):
- self.line_gen = ("Zipfile test line %d. random float: %f" % (i, random())
- for i in xrange(FIXEDTEST_SIZE))
+ self.line_gen = ["Zipfile test line %d. random float: %f" % (i, random())
+ for i in xrange(FIXEDTEST_SIZE)]
self.data = '\n'.join(self.line_gen) + '\n'
# Make a source file with some lines
@@ -239,6 +239,63 @@
self.assertEqual(zipfp.namelist(), ["absolute"])
zipfp.close()
+ def testAppendToZipFile(self):
+ # Test appending to an existing zipfile
+ zipfp = zipfile.ZipFile(TESTFN2, "w", zipfile.ZIP_STORED)
+ zipfp.write(TESTFN, TESTFN)
+ zipfp.close()
+ zipfp = zipfile.ZipFile(TESTFN2, "a", zipfile.ZIP_STORED)
+ zipfp.writestr("strfile", self.data)
+ self.assertEqual(zipfp.namelist(), [TESTFN, "strfile"])
+ zipfp.close()
+
+ def testAppendToNonZipFile(self):
+ # Test appending to an existing file that is not a zipfile
+ # NOTE: this test fails if len(d) < 22 because of the first
+ # line "fpin.seek(-22, 2)" in _EndRecData
+ d = 'I am not a ZipFile!'*10
+ f = file(TESTFN2, 'wb')
+ f.write(d)
+ f.close()
+ zipfp = zipfile.ZipFile(TESTFN2, "a", zipfile.ZIP_STORED)
+ zipfp.write(TESTFN, TESTFN)
+ zipfp.close()
+
+ f = file(TESTFN2, 'rb')
+ f.seek(len(d))
+ zipfp = zipfile.ZipFile(f, "r")
+ self.assertEqual(zipfp.namelist(), [TESTFN])
+ zipfp.close()
+ f.close()
+
+ def test_WriteDefaultName(self):
+ # Check that calling ZipFile.write without arcname specified produces the expected result
+ zipfp = zipfile.ZipFile(TESTFN2, "w")
+ zipfp.write(TESTFN)
+ self.assertEqual(zipfp.read(TESTFN), file(TESTFN).read())
+ zipfp.close()
+
+ def test_PerFileCompression(self):
+ # Check that files within a Zip archive can have different compression options
+ zipfp = zipfile.ZipFile(TESTFN2, "w")
+ zipfp.write(TESTFN, 'storeme', zipfile.ZIP_STORED)
+ zipfp.write(TESTFN, 'deflateme', zipfile.ZIP_DEFLATED)
+ sinfo = zipfp.getinfo('storeme')
+ dinfo = zipfp.getinfo('deflateme')
+ self.assertEqual(sinfo.compress_type, zipfile.ZIP_STORED)
+ self.assertEqual(dinfo.compress_type, zipfile.ZIP_DEFLATED)
+ zipfp.close()
+
+ def test_WriteToReadonly(self):
+ # Check that trying to call write() on a readonly ZipFile object
+ # raises a RuntimeError
+ zipf = zipfile.ZipFile(TESTFN2, mode="w")
+ zipf.writestr("somefile.txt", "bogus")
+ zipf.close()
+ zipf = zipfile.ZipFile(TESTFN2, mode="r")
+ self.assertRaises(RuntimeError, zipf.write, TESTFN)
+ zipf.close()
+
def tearDown(self):
os.remove(TESTFN)
os.remove(TESTFN2)
@@ -361,7 +418,6 @@
self.assertEqual(zipfp.namelist(), ["absolute"])
zipfp.close()
-
def tearDown(self):
zipfile.ZIP64_LIMIT = self._limit
os.remove(TESTFN)
@@ -432,6 +488,11 @@
finally:
shutil.rmtree(TESTFN2)
+ def testWriteNonPyfile(self):
+ zipfp = zipfile.PyZipFile(TemporaryFile(), "w")
+ file(TESTFN, 'w').write('most definitely not a python file')
+ self.assertRaises(RuntimeError, zipfp.writepy, TESTFN)
+ os.remove(TESTFN)
class OtherTests(unittest.TestCase):
@@ -513,7 +574,56 @@
# a RuntimeError, and so should calling .testzip. An earlier
# version of .testzip would swallow this exception (and any other)
# and report that the first file in the archive was corrupt.
+ self.assertRaises(RuntimeError, zipf.read, "foo.txt")
+ self.assertRaises(RuntimeError, zipf.open, "foo.txt")
self.assertRaises(RuntimeError, zipf.testzip)
+ self.assertRaises(RuntimeError, zipf.writestr, "bogus.txt", "bogus")
+ file(TESTFN, 'w').write('zipfile test data')
+ self.assertRaises(RuntimeError, zipf.write, TESTFN)
+
+ def test_BadConstructorMode(self):
+ # Check that bad modes passed to ZipFile constructor are caught
+ self.assertRaises(RuntimeError, zipfile.ZipFile, TESTFN, "q")
+
+ def test_BadOpenMode(self):
+ # Check that bad modes passed to ZipFile.open are caught
+ zipf = zipfile.ZipFile(TESTFN, mode="w")
+ zipf.writestr("foo.txt", "O, for a Muse of Fire!")
+ zipf.close()
+ zipf = zipfile.ZipFile(TESTFN, mode="r")
+ # read the data to make sure the file is there
+ zipf.read("foo.txt")
+ self.assertRaises(RuntimeError, zipf.open, "foo.txt", "q")
+ zipf.close()
+
+ def test_Read0(self):
+ # Check that calling read(0) on a ZipExtFile object returns an empty
+ # string and doesn't advance file pointer
+ zipf = zipfile.ZipFile(TESTFN, mode="w")
+ zipf.writestr("foo.txt", "O, for a Muse of Fire!")
+ # read the data to make sure the file is there
+ f = zipf.open("foo.txt")
+ for i in xrange(FIXEDTEST_SIZE):
+ self.assertEqual(f.read(0), '')
+
+ self.assertEqual(f.read(), "O, for a Muse of Fire!")
+ zipf.close()
+
+ def test_OpenNonexistentItem(self):
+ # Check that attempting to call open() for an item that doesn't
+ # exist in the archive raises a RuntimeError
+ zipf = zipfile.ZipFile(TESTFN, mode="w")
+ self.assertRaises(KeyError, zipf.open, "foo.txt", "r")
+
+ def test_BadCompressionMode(self):
+ # Check that bad compression methods passed to ZipFile.open are caught
+ self.assertRaises(RuntimeError, zipfile.ZipFile, TESTFN, "w", -1)
+
+ def test_NullByteInFilename(self):
+ # Check that a filename containing a null byte is properly terminated
+ zipf = zipfile.ZipFile(TESTFN, mode="w")
+ zipf.writestr("foo.txt\x00qqq", "O, for a Muse of Fire!")
+ self.assertEqual(zipf.namelist(), ['foo.txt'])
def tearDown(self):
support.unlink(TESTFN)
diff --git a/Lib/zipfile.py b/Lib/zipfile.py
index 67d2c5d..a53f8ed 100644
--- a/Lib/zipfile.py
+++ b/Lib/zipfile.py
@@ -568,8 +568,9 @@
def __init__(self, file, mode="r", compression=ZIP_STORED, allowZip64=False):
"""Open the ZIP file with mode read "r", write "w" or append "a"."""
- self._allowZip64 = allowZip64
- self._didModify = False
+ if mode not in ("r", "w", "a"):
+ raise RuntimeError('ZipFile() requires mode "r", "w", or "a"')
+
if compression == ZIP_STORED:
pass
elif compression == ZIP_DEFLATED:
@@ -578,6 +579,9 @@
"Compression requires the (missing) zlib module"
else:
raise RuntimeError, "That compression method is not supported"
+
+ self._allowZip64 = allowZip64
+ self._didModify = False
self.debug = 0 # Level of printing: 0 through 3
self.NameToInfo = {} # Find file info given name
self.filelist = [] # List of ZipInfo instances for archive
@@ -720,7 +724,12 @@
def getinfo(self, name):
"""Return the instance of ZipInfo given 'name'."""
- return self.NameToInfo[name]
+ info = self.NameToInfo.get(name)
+ if info is None:
+ raise KeyError(
+ 'There is no item named %r in the archive' % name)
+
+ return info
def setpassword(self, pwd):
"""Set default password for encrypted files."""
@@ -824,6 +833,10 @@
def write(self, filename, arcname=None, compress_type=None):
"""Put the bytes from filename into the archive under the name
arcname."""
+ if not self.fp:
+ raise RuntimeError(
+ "Attempt to write to ZIP archive that was already closed")
+
st = os.stat(filename)
mtime = time.localtime(st.st_mtime)
date_time = mtime[0:6]
@@ -896,6 +909,11 @@
zinfo.compress_type = self.compression
else:
zinfo = zinfo_or_arcname
+
+ if not self.fp:
+ raise RuntimeError(
+ "Attempt to write to ZIP archive that was already closed")
+
zinfo.file_size = len(bytes) # Uncompressed size
zinfo.header_offset = self.fp.tell() # Start of header bytes
self._writecheck(zinfo)
diff --git a/Misc/NEWS b/Misc/NEWS
index c40cf78..b5cf349 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -420,6 +420,10 @@
- Patch #1481079: add support for HTTP_REFERER to CGIHTTPServer.
+- Patch #1675424: Added tests for uncovered code in the zipfile module.
+ The KeyError raised by Zipfile.getinfo for nonexistent names now has
+ a descriptive message.
+
- Bug #1115886: os.path.splitext('.cshrc') gives now ('.cshrc', '').
- unittest now verifies more of its assumptions. In particular, TestCase