I added some comments. I'd like to get a second opinion about the fix
implementation, but if it looks okay to someone else, I'll be happy to land
this.
https://bugs.python.org/review/23228/diff/13610/Lib/tarfile.py
File Lib/tarfile.py (right):
https://bugs.python.org/review/23228/diff/13610/Lib/tarfile.py#newcode2183
Lib/tarfile.py:2183: os.remove(targetpath)
I'm a little concerned about the security implications of this. It means a
carefully crafted tarball could retarget existing symlinks. But I think that's
the way tar(1) works if you don't give it -k (at least GNU tar). I'd like to
get a second opinion.
https://bugs.python.org/review/23228/diff/13610/Lib/test/test_tarfile.py
File Lib/test/test_tarfile.py (right):
https://bugs.python.org/review/23228/diff/13610/Lib/test/test_tarfile.py#newc...
Lib/test/test_tarfile.py:2177: self.cwd = os.getcwd()
I'd probably move the chdir into a try/finally inside the test itself, just so
any weird failure causing tearDown not to be called wouldn't corrupt cwd for
other tests. Also, putting the chdir closer to the os.mkdirs in the test makes
the latter a little more comprehensible at first glance.
https://bugs.python.org/review/23228/diff/17198/Lib/tarfile.py File Lib/tarfile.py (right): https://bugs.python.org/review/23228/diff/17198/Lib/tarfile.py#newcode2205 Lib/tarfile.py:2205: if getattr(e, "winerror", None) is None: What about AttributeError ...
Issue 23228: Crashes when tarfile contains a symlink and unpack directory contain it too
Created 4 years, 11 months ago by michael.vogt_gmail.com
Modified 11 months, 3 weeks ago
Reviewers: barry, Martin Panter
Base URL: None
Comments: 3