tarfile does not handle file .name being an int

[seirl]

BPO	21044
Nosy	@warsaw, @gustaebel, @merwok, @bitdancer, @vadmium, @zware, @serhiy-storchaka, @eryksun, @seirl
Files	tarfile-fileobjname.diff test_tarfile_fobj_int.diff: Test case for Lib/test/tarfile.py test_tarfile_all_modes.diff test_tarfile_fdopen.diff

^{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:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-06-01.07:22:24.510>
created_at = <Date 2014-03-24.01:32:57.313>
labels = ['type-bug', 'library']
title = 'tarfile does not handle file .name being an int'
updated_at = <Date 2017-06-01.07:22:24.509>
user = 'https://github.com/seirl'

bugs.python.org fields:

activity = <Date 2017-06-01.07:22:24.509>
actor = 'antoine.pietri'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-06-01.07:22:24.510>
closer = 'antoine.pietri'
components = ['Library (Lib)']
creation = <Date 2014-03-24.01:32:57.313>
creator = 'antoine.pietri'
dependencies = []
files = ['34603', '34605', '34739', '34740']
hgrepos = []
issue_num = 21044
keywords = ['patch']
message_count = 24.0
messages = ['214662', '214663', '214670', '214685', '214708', '214754', '214756', '214757', '214760', '215216', '215230', '215626', '215627', '215632', '216984', '223275', '223280', '223319', '223335', '223338', '223345', '223643', '223645', '223742']
nosy_count = 10.0
nosy_names = ['barry', 'lars.gustaebel', 'eric.araujo', 'r.david.murray', 'python-dev', 'martin.panter', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'antoine.pietri']
pr_nums = []
priority = 'normal'
resolution = 'remind'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21044'
versions = ['Python 2.7']

[seirl]

The fact that tempfile.TemporaryFile() has a "name" integer attribute causes weird behavior when interacting with libraries that rely on this attribute being a valid string for file objects.
For instance, it led to this exception with the "tarfile" module, which I resolved by using a NamedTemporaryFile():

>>> tarfile.open(fileobj=tempfile.TemporaryFile(), mode='w')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.4/tarfile.py", line 1585, in open
    return cls.taropen(name, mode, fileobj, **kwargs)
  File "/usr/lib/python3.4/tarfile.py", line 1595, in taropen
    return cls(name, mode, fileobj, **kwargs)
  File "/usr/lib/python3.4/tarfile.py", line 1431, in __init__
    self.name = os.path.abspath(name) if name else None
  File "/usr/lib/python3.4/posixpath.py", line 360, in abspath
    if not isabs(path):
  File "/usr/lib/python3.4/posixpath.py", line 64, in isabs
    return s.startswith(sep)
AttributeError: 'int' object has no attribute 'startswith'

Which is caused by these lines in the "tarfile" module:

if name is None and hasattr(fileobj, "name"):
    name = fileobj.name

If TemporaryFile() didn't have a name attribute, tarfile, which doesn't really need the file name, would simply have continued without errors.

I am not aware of any place where this "name" integer attribute is actually useful, and, as a matter of fact, it is not even documented: http://docs.python.org/3.4/library/tempfile.html#tempfile.TemporaryFile

[seirl]

Alternatively, if the "name" attribute can't be removed, I propose the following diff for the tarfile module:

-if name is None and hasattr(fileobj, "name"):
+if name is None and hasattr(fileobj, "name") and isinstance(fileobj.name, str):

:-)

[eryksun]

This name attribute is documented here:

http://docs.python.org/3/library/io#io.FileIO.name

3.4 Source:

http://hg.python.org/cpython/file/04f714765c13/Modules/_io/fileio.c#l432

In PY2, os.fdopen sets the name to '<fdopen>'. See the related bpo-13781. Here's the workaround in gzip.py:

    filename = getattr(fileobj, 'name', '')
    if not isinstance(filename, (str, bytes)):
        filename = ''

[seirl]

I attached a patch for tarfile with eryksun's suggestion.

[seirl]

Here's the test case as requested by berkerpeksag in the patch review.

[merwok]

Isn’t the bug here really that TemporaryFile has a name attribute that’s not a string? I don’t see how an integer name makes sense within the IO system.

[seirl]

Yes, the bug report was originally titled like this ("TemporaryFile should'nt have a name attribute") but apparently this is a common and expected behavior for FileIO subclasses.

[bitdancer]

See the documentation link in msg214670. This isn't a characteristic of TemporaryFile, it's a characteristic of the Python IO system. So you'd have to argue that the documented behavior of the io system is a bug.

[seirl]

Well, this behavior seems pretty inconsistent to me, it means that every library has to check if the name attribute is actually a string (which would correspond to a path) or an int (that would mean a fd), and I think a "fd" attribute would seem more consistent.
But as I don't think we could change this behavior that easily (it means breaking retrocompatibility after all...), we must at least make tarlib deal with that.

[seirl]

Ping. The patch is just one line and there's a test case, if someone could review that, it would be great!

(For the record I signed the contributor agreement a week ago and my profile still hasn't been updated).

[serhiy-storchaka]

Does tarfile work with bytes file names?

I think a test should explicitly test file objects with str, bytes and int name and file objects without name in all write modes ("w", "w:gz", "w|gz", etc).