New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chained exception/incorrect exception from tarfile.open on a non-existent file #55722
Comments
Using Python tip from Sunday, I noticed that tarfile does not elegantly handle ENOENT by raising a single exception: >>> tarfile.TarFile.gzopen('fdsfd', 'r')
Traceback (most recent call last):
File "/home/evan/hg/cpython/Lib/tarfile.py", line 1808, in gzopen
fileobj = gzip.GzipFile(name, mode + "b", compresslevel, fileobj)
File "/home/evan/hg/cpython/Lib/gzip.py", line 157, in __init__
fileobj = self.myfileobj = builtins.open(filename, mode or 'rb')
IOError: [Errno 2] No such file or directory: 'fdsfd'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/evan/hg/cpython/Lib/tarfile.py", line 1812, in gzopen
fileobj.close()
AttributeError: 'NoneType' object has no attribute 'close' I tried to fix this in a cross-platform way, by attempting to open the file before processing it, and letting the with statement handle calling close: diff -r 955547e57cff Lib/tarfile.py
--- a/Lib/tarfile.py Sun Mar 13 19:32:21 2011 +0100
+++ b/Lib/tarfile.py Mon Mar 14 19:33:21 2011 -0400
@@ -1793,6 +1793,10 @@
if len(mode) > 1 or mode not in "rw":
raise ValueError("mode must be 'r' or 'w'")
+ if mode == "r":
+ # force an exception if the file does not exist.
+ with open(name):
+ pass
try:
import gzip
gzip.GzipFile However, this produces infinite recursion: Curiously, if I force the ValueError in that same function to be triggered (by passing a three character string for the mode), that exception is raised fine. I am therefore wondering if this is a bug in the exit processing. Unfortunately, my attempts at producing a general test case have been unsuccessful thusfar. |
The infinite recursion happens because I also think that explicitly checking for "fileobj" being None is a cleaner solution. With your current method (trying to open the file beforehand), you will introduce a race condition: If the file exists when you open it for the first time but has been deleted when you try to open it the second time, the exact same error will happen ("fileobj" being None, that is). |
*facepalm* Indeed, thanks for pointing that out and nice catch on the race condition. Attached is a patch to fix the issue as you've suggested, and adds a test case for it. |
This fix reveals a second bug. Without this fix, a non-existent file raises an IOError with an appropriate error message, but with the chained exception. After this fix, it raises an error that says 'not a gzip file', which while technically true is not very helpful :) The correct IOError message only happened by accident in the original code, but we need to fix this second bug in order to fix the first one correctly. I suggest that the test case should read: with self.assertRaisesRegex("xxx", IOError) as ex:
tarfile.open("xxx", self.mode)
self.assertEqual(ex.exception.errno, errno.ENOENT) |
Note that the code in question was changed to the form with the bugs in 3.2, so this fix does not need to be backported to 3.1. The test could be, though. |
David, Thanks for the pointers. I've updated the patch hopefully adequately addressing your concerns. |
Commented on the patch. I'll be happy to land this for Evan. |
New changeset 843cd43206b4 by Georg Brandl in branch '3.2': |
Fixed in 3.2/default. 2.7 has even more primitive error handling; should the gzopen() be adapted to the 3.x case? |
Should this issue still remain open? The original report described a chained exception, which obviously doesn't happen in 2.7 (nor with Georg's changeset, in 3.2, 3.3, or 3.4). RDM's message implies there still may still be bugs lurking here in 2.7, but OTOH, the original issue isn't a problem (i.e. no chained exceptions). I'd be tempted to say that if there are still problems here, it would be better to open a 2.7 specific bug. |
I suppose that 2.7 may leak GzipFile in case of some errors, but this is another issue. |
Alright, I'm going to close this issue. Please open a new bug for Python 2.7. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: