Index: Misc/NEWS =================================================================== --- Misc/NEWS (revision 54219) +++ Misc/NEWS (working copy) @@ -152,6 +152,10 @@ Library ------- +- 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', ''). - Patch #787789: allow to pass custom TestRunner instances to unittest's Index: Doc/lib/libzipfile.tex =================================================================== --- Doc/lib/libzipfile.tex (revision 54219) +++ Doc/lib/libzipfile.tex (working copy) @@ -24,7 +24,7 @@ The available attributes of this module are: -\begin{excdesc}{error} +\begin{excdesc}{BadZipfile} The error raised for bad ZIP files. \end{excdesc} @@ -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[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 internal use only. The \method{writepy()} method makes archives with file names like this: Index: Lib/test/test_zipfile.py =================================================================== --- Lib/test/test_zipfile.py (revision 54219) +++ Lib/test/test_zipfile.py (working copy) @@ -13,12 +13,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 @@ -238,6 +238,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) @@ -360,7 +417,6 @@ self.assertEqual(zipfp.namelist(), ["absolute"]) zipfp.close() - def tearDown(self): zipfile.ZIP64_LIMIT = self._limit os.remove(TESTFN) @@ -431,6 +487,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): @@ -516,8 +577,62 @@ # 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) + os.remove(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() + os.unlink(TESTFN) + + 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() + os.unlink(TESTFN) + + 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") + os.unlink(TESTFN) + + 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']) + os.unlink(TESTFN) + class DecryptionTests(unittest.TestCase): # This test checks that ZIP decryption works. Since the library does not # support encryption at the moment, we use a pre-generated encrypted Index: Lib/zipfile.py =================================================================== --- Lib/zipfile.py (revision 54219) +++ Lib/zipfile.py (working copy) @@ -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,8 +724,13 @@ 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.""" self.pwd = pwd @@ -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)