tarfile.py: dict order dependency

[arigo]

BPO	1496501
Nosy	@arigo, @gustaebel, @jackdied

^{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/gustaebel'
closed_at = <Date 2006-12-27.10:39:49.000>
created_at = <Date 2006-05-28.18:42:52.000>
labels = ['library']
title = 'tarfile.py: dict order dependency'
updated_at = <Date 2006-12-27.10:39:49.000>
user = 'https://github.com/arigo'

bugs.python.org fields:

activity = <Date 2006-12-27.10:39:49.000>
actor = 'lars.gustaebel'
assignee = 'lars.gustaebel'
closed = True
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2006-05-28.18:42:52.000>
creator = 'arigo'
dependencies = []
files = []
hgrepos = []
issue_num = 1496501
keywords = []
message_count = 6.0
messages = ['28669', '28670', '28671', '28672', '28673', '28674']
nosy_count = 3.0
nosy_names = ['arigo', 'lars.gustaebel', 'jackdied']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1496501'
versions = ['Python 2.5']

[arigo]

When guessing the type of a file, tarfile.py tries to
open it in .tar, .tar.gz and .tar.bz2 modes
successfully. The problem is that in conjunction with a
fileobj argument, failed attempts can advance the
current position of the fileobj, and cause the following
attempts to fail as well.

This didn't show up so far because, by chance, the order
in which the OPEN_METH dictionary is enumerated is
somehow the "correct" one. The problem can be seen by
changing this order and re-running test_tarfile.py; for
example, reverse the order (tarfile.py line 1013):

    for comptype in reversed(cls.OPEN_METH):

[jackdied]

Logged In: YES
user_id=591932

I took a look at tarfile.py and though there is this comment
about OPEN_METH

# This concept allows one to subclass TarFile without losing
the comfort of
# the super-constructor. A sub-constructor is registered and
made available
# by adding it to the mapping in OPEN_METH.

because adding items to OPEN_METH could change the order and
break the Tarfile.open() classmethod I doubt adding to
OPEN_METH is done in practice.

+1 for renaming OPEN_METH to _OPEN_METH and making it a list
of two-tuples.

+0 on changing the two tups from ('typename', 'funcname') to
('typename', function_ref) and dropping the getattr
functionality. The three default openers are not listed in
the public interface so if anyone is subclassing Tarfile and
overriding them (doubtful) they are doing so at their own peril.

[arigo]

Logged In: YES
user_id=4771

If possible, I would prefer that the constructors were fixed
to become order-independent. Not sure how easy this is,
though.

[jackdied]

Logged In: YES
user_id=591932

The invidual openers could tell() the fileobj and revert
with a seek() on failure. The module requires those methods
be implemented already to work so that is safe. I'll cook
up a patch.

But basically open() isn't as extensible as it seems.
Stream reading and writing isn't supported for injected
compression types and appending only works on plain tarfiles.

I'm guessing that isn't a problem in practice or someone
would have mentioned it by now ;)

[jackdied]

Logged In: YES
user_id=591932

added patch 1504073 because SF won't let just anyone attach
files to bugs.

[gustaebel]

Fixed with said patch.