From: Georg Brandl Date: Thu, 12 Jul 2007 09:59:22 +0000 (+0000) Subject: Patch #1675424: Added tests for uncovered code in the zipfile module. X-Git-Tag: v2.6a1~1577 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4b3ab6fcc09d93cfe47fd0087fa694f1166a7b0b;p=python 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 6cdd1d35e6..366cee9a4f 100644 --- a/Doc/lib/libzipfile.tex +++ b/Doc/lib/libzipfile.tex @@ -24,8 +24,8 @@ it cannot currently create an encrypted file. 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 @@ The available attributes of this module are: (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 @@ cat myzip.zip >> python.exe \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 @@ cat myzip.zip >> python.exe 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 @@ cat myzip.zip >> python.exe 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 @@ cat myzip.zip >> python.exe \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 @@ cat myzip.zip >> python.exe 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 @@ cat myzip.zip >> python.exe \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 @@ cat myzip.zip >> python.exe \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 @@ those of \class{ZipFile} objects. 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 49602ffdd4..3d2f9bd43f 100644 --- a/Lib/test/test_zipfile.py +++ b/Lib/test/test_zipfile.py @@ -14,12 +14,12 @@ import test.test_support as support 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 @@ class TestsWithSourceFile(unittest.TestCase): 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 @@ class TestZip64InSmallFiles(unittest.TestCase): self.assertEqual(zipfp.namelist(), ["absolute"]) zipfp.close() - def tearDown(self): zipfile.ZIP64_LIMIT = self._limit os.remove(TESTFN) @@ -432,6 +488,11 @@ class PyZipFileTests(unittest.TestCase): 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 @@ class OtherTests(unittest.TestCase): # 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 67d2c5d8d2..a53f8ed229 100644 --- a/Lib/zipfile.py +++ b/Lib/zipfile.py @@ -568,8 +568,9 @@ class ZipFile: 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 @@ class ZipFile: "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 @@ class ZipFile: 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 @@ class ZipFile: 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 @@ class ZipFile: 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 c40cf78cea..b5cf34963e 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -420,6 +420,10 @@ Library - 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