| msg124523 - (view) |
Author: Sridhar Ratnakumar (srid) |
Date: 2010-12-23 00:17 |
| tarfile.extractall overwrites normal files and directories, yet it fails to overwrite symlinks: [..] tf.extractall() File "/opt/ActivePython-2.7/lib/python2.7/tarfile.py", line 2046, in extractall self.extract(tarinfo, path) File "/opt/ActivePython-2.7/lib/python2.7/tarfile.py", line 2083, in extract self._extract_member(tarinfo, os.path.join(path, tarinfo.name)) File "/opt/ActivePython-2.7/lib/python2.7/tarfile.py", line 2167, in _extract_member self.makelink(tarinfo, targetpath) File "/opt/ActivePython-2.7/lib/python2.7/tarfile.py", line 2243, in makelink os.symlink(tarinfo.linkname, targetpath) OSError: [Errno 17] File exists To reproduce, use a .tar.gz file containing relative (i.e., in the same directory) symlinks. Perhaps it should delete `targetpath` before attempting to create a symlink. |
|
|
| msg134502 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-04-26 21:20 |
| I just hit the same issue. This seems to work: Modified:Lib/tarfile.py =================================================================== ---Lib/tarfile.py 2011-04-26 20:36:33 UTC (rev 49502) +++Lib/tarfile.py 2011-04-26 21:01:24 UTC (rev 49503) @@ -2239,6 +2239,8 @@ if hasattr(os, "symlink") and hasattr(os, "link"): # For systems that support symbolic and hard links. if tarinfo.issym(): + if os.path.exists(targetpath): + os.unlink(targetpath) os.symlink(tarinfo.linkname, targetpath) else: # See extract(). |
|
|
| msg134519 - (view) |
Author: Senthil Kumaran (orsenthil) *  |
Date: 2011-04-26 23:23 |
| Scott- which platform did you observe this? I can't reproduce this on the 2.7 code on Linux. |
|
|
| msg134555 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-04-27 12:24 |
| It happens on RedHat and CentOS 5, but I suspect it would happen on any Unix variant. Here's a test that exacerbates the issue: # # test_tarfile.py # # Description: # tests for python tarfile module # # IdIdId # import os import shutil import tarfile import tempfile def test_tar_overwrites_symlink(): d = tempfile.mkdtemp() try: new_dir_name = os.path.join(d, 'newdir') archive_name = os.path.join(d, 'newdir.tar') os.mkdir(new_dir_name) source_file_name = os.path.join(new_dir_name, 'source') target_link_name = os.path.join(new_dir_name, 'symlink') with open(source_file_name, 'w') as f: f.write('something\n') os.symlink(source_file_name, target_link_name) with tarfile.open(archive_name, 'w') as tar: for f in [source_file_name, target_link_name]: tar.add(f, arcname=os.path.basename(f)) # this should not raise OSError: [Errno 17] File exists with tarfile.open(archive_name, 'r') as tar: tar.extractall(path=new_dir_name) finally: shutil.rmtree(d) |
|
|
| msg134657 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2011-04-28 07:32 |
| New changeset 0c8bc3a0130a by Senthil Kumaran in branch '2.7': Fix closes : tarfile.extractall failure when symlinked files are present. http://hg.python.org/cpython/rev/0c8bc3a0130a |
|
|
| msg134658 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2011-04-28 07:35 |
| I had tried/tested against 3.x branch and did not find the problem. Later realized that it was only again 2.7. Pushed in the changes and the tests. I shall the tests only in 3.x codeline. |
|
|
| msg134782 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2011-04-29 16:21 |
| Senthil, Windows buildbots on 3.1, 3.2 and 3.x show test failures. See e.g. http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.1/builds/1780/steps/test/logs/stdio |
|
|
| msg134816 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2011-04-29 22:12 |
| New changeset 2665a28643b8 by Senthil Kumaran in branch 'default': Wrap the correct test with the skip decorator for the . http://hg.python.org/cpython/rev/2665a28643b8 |
|
|
| msg134817 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2011-04-29 22:14 |
| I had wrapped skipUnless decorator for the wrong test (test_extractall instead of test_extractall_symlinks) in the 3.x code. Corrected it and waiting for next bb reports. Thank you. |
|
|
| msg134828 - (view) |
Author: Senthil Kumaran (orsenthil) * |
Date: 2011-04-30 01:17 |
| buildbots are green again. |
|
|
| msg135924 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-05-13 16:34 |
| It turns out that my fix was at least one byte short of complete. If the target pathname is a broken symlink, os.path.exists() returns False, and the OSError is raised. I should have used os.path.lexists(). Also, I believe the same problem exists for the hardlink case a few lines below. |
|
|
| msg135925 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-05-13 16:57 |
| here is a diff of a better fix based on the previous patch: Index: tarfile.py =================================================================== --- tarfile.py (revision 49758) +++ tarfile.py (working copy) @@ -2239,12 +2239,14 @@ if hasattr(os, "symlink") and hasattr(os, "link"): # For systems that support symbolic and hard links. if tarinfo.issym(): - if os.path.exists(targetpath): + if os.path.lexists(targetpath): os.unlink(targetpath) os.symlink(tarinfo.linkname, targetpath) else: # See extract(). if os.path.exists(tarinfo._link_target): + if os.path.lexists(targetpath): + os.unlink(targetpath) os.link(tarinfo._link_target, targetpath) else: self._extract_member(self._find_link_target(tarinfo), targetpath) |
|
|
| msg135926 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-05-13 16:58 |
| tests that verify the bug/fix: def test_extractall_broken_symlinks(self): # Test if extractall works properly when tarfile contains symlinks tempdir = os.path.join(TEMPDIR, "testsymlinks") temparchive = os.path.join(TEMPDIR, "testsymlinks.tar") os.mkdir(tempdir) try: source_file = os.path.join(tempdir,'source') target_file = os.path.join(tempdir,'symlink') with open(source_file,'w') as f: f.write('something\n') os.symlink(source_file, target_file) tar = tarfile.open(temparchive,'w') tar.add(target_file, arcname=os.path.basename(target_file)) tar.close() # remove the real file os.unlink(source_file) # Let's extract it to the location which contains the symlink tar = tarfile.open(temparchive,'r') # this should not raise OSError: [Errno 17] File exists try: tar.extractall(path=tempdir) except OSError: self.fail("extractall failed with broken symlinked files") finally: tar.close() finally: os.unlink(temparchive) shutil.rmtree(TEMPDIR) def test_extractall_hardlinks(self): # Test if extractall works properly when tarfile contains symlinks TEMPDIR = tempfile.mkdtemp() tempdir = os.path.join(TEMPDIR, "testsymlinks") temparchive = os.path.join(TEMPDIR, "testsymlinks.tar") os.mkdir(tempdir) try: source_file = os.path.join(tempdir,'source') target_file = os.path.join(tempdir,'symlink') with open(source_file,'w') as f: f.write('something\n') os.link(source_file, target_file) tar = tarfile.open(temparchive,'w') tar.add(source_file, arcname=os.path.basename(source_file)) tar.add(target_file, arcname=os.path.basename(target_file)) tar.close() # Let's extract it to the location which contains the symlink tar = tarfile.open(temparchive,'r') # this should not raise OSError: [Errno 17] File exists try: tar.extractall(path=tempdir) except OSError: self.fail("extractall failed with linked files") finally: tar.close() finally: os.unlink(temparchive) shutil.rmtree(TEMPDIR) |
|
|
| msg135936 - (view) |
Author: Scott Leerssen (Scott.Leerssen) |
Date: 2011-05-13 18:08 |
| oops... I left some of my local edits in those tests. be sure to fix the TEMPDIR use if you add these into the tarfile tests. |
|
|