Regression in zipfile writing in 2.7.13

[peterebden]

BPO	29094
Nosy	@Yhg1s, @gpshead, @vstinner, @larryhastings, @benjaminp, @ned-deily, @serhiy-storchaka, @peterebden
PRs	Revert "Issue #29094: Offsets in a ZIP file created with extern file object and modes" #1467 Revert bpo-26293 for zipfile breakage. See also bpo-29094. #1484 [3.6] Revert bpo-26293 for zipfile breakage. See also bpo-29094. (GH-1484). #1485 [3.5] Revert bpo-26293 for zipfile breakage. See also bpo-29094. (GH-1484). #1486
Files	zipfile_write_absolute_offsets.patch

^{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-05-18.15:15:26.153>
created_at = <Date 2016-12-28.13:26:31.940>
labels = ['3.7', 'type-bug', 'library', 'release-blocker']
title = 'Regression in zipfile writing in 2.7.13'
updated_at = <Date 2017-05-18.15:15:26.152>
user = 'https://github.com/peterebden'

bugs.python.org fields:

activity = <Date 2017-05-18.15:15:26.152>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-05-18.15:15:26.153>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-12-28.13:26:31.940>
creator = 'Peter Ebden'
dependencies = []
files = ['46107']
hgrepos = []
issue_num = 29094
keywords = ['patch']
message_count = 21.0
messages = ['284172', '284421', '284423', '284424', '284425', '284432', '284436', '292803', '292826', '292827', '292828', '292838', '292842', '292896', '292897', '292914', '293025', '293047', '293154', '293156', '293158']
nosy_count = 9.0
nosy_names = ['twouters', 'gregory.p.smith', 'vstinner', 'larry', 'benjamin.peterson', 'ned.deily', 'python-dev', 'serhiy.storchaka', 'Peter Ebden']
pr_nums = ['1467', '1484', '1485', '1486']
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29094'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

[peterebden]

In Python 2.7.13, using zipfile.ZipFile to write into a file with some initial preamble produces a zip file that cannot be read again by some zip implementations.
Our use case is using pex (https://github.com/pantsbuild/pex) which writes a zip that begins with a shebang, and later attempting to manipulate that using Go's standard archive/zip package. In 2.7.12 that works OK, but in 2.7.13 the .pex file is rejected on reading. Linux's command-line unzip tool will read the archive, but issues a warning ("4 extra bytes at beginning or within zipfile") which wasn't present previously.
zipfile.ZipFile does read the files OK.

I assume this is related to https://bugs.python.org/issue26293 since that's the most obvious zipfile change in 2.7.13. It's pretty easy to reproduce using the example in that issue:

from zipfile import ZipFile
with open('a.zip', 'wb') as base:
    base.write(b'old\n')
    with ZipFile(base, 'a') as myzip:
        myzip.write('eggs.txt')

unzip -t a.zip
Archive: a.zip
warning [a.zip]: 4 extra bytes at beginning or within zipfile
(attempting to process anyway)
...

[serhiy-storchaka]

The specification of ZIP files is not pretty clear, but the unzip utility first try to interpret offsets relatively to the start of the archive that can be different from the start of the file. Go's standard archive/zip package interprets offsets as absolute file positions and doesn't support concatenated ZIP files.

In any case there is a regression. Proposed patch partially reverts bpo-26293. Offsets in created ZIP file are now absolute when create with modes 'w' and 'x'. But they are relative when create with mode 'a'. This should fix a regression in pex, but keep the case of bpo-26293.

I'm going to push the patch before tagging 3.5.3 rc1.

[benjaminp]

Is there a reason to ever not use relative offsets? It seems that's strictly more general the absolute.

[serhiy-storchaka]

Actually all offsets are relative to some point, just this point not always at the start of the file. If concatenate the ZIP file to other file, the unzip utility and the zipfile module are able to infer the starting point and correct offsets. Thus there is no matter what is the starting point of offsets. But Go's standard archive/zip package works only if offsets are relative to the start of the file. I don't know how common such interpretation however.

[python-dev]

New changeset f5aa1c9c2b7e by Serhiy Storchaka in branch '3.5':
Issue bpo-29094: Offsets in a ZIP file created with extern file object and modes
https://hg.python.org/cpython/rev/f5aa1c9c2b7e

New changeset 342bc734f523 by Serhiy Storchaka in branch '2.7':
Issue bpo-29094: Offsets in a ZIP file created with extern file object and modes
https://hg.python.org/cpython/rev/342bc734f523

New changeset f36f9bce997d by Serhiy Storchaka in branch '3.6':
Issue bpo-29094: Offsets in a ZIP file created with extern file object and modes
https://hg.python.org/cpython/rev/f36f9bce997d

[larryhastings]

If this is fixed, can we close this issue? This release blocker is one of two issues blocking 3.5.3 rc1.

[python-dev]

New changeset a80c14ace927 by Serhiy Storchaka in branch 'default':
Issue bpo-29094: Offsets in a ZIP file created with extern file object and modes
https://hg.python.org/cpython/rev/a80c14ace927

[Yhg1s]

For the record, this new behaviour is wrong. It's not immediately obvious from the ZIP spec, but offsets have to be from the start of the file, not the archive, or ZIP64 can't work. And yes, that means you can't blindly concatenate ZIP files to other files, you have to rewrite them.

The way to create an 'embedded' ZIP archive like requested in issue bpo-26293 is to write it to a separate file (or file-like object). Making "a" have this magical and wrong-for-almost-anyone meaning is... confusing, to say the least. The change certainly doesn't belong in a bugfix release, but I guess it's too late for that now.

It's also not documented; the docs mention this:

If mode is 'a' and file refers to an existing ZIP file, then
additional files are added to it. If file does not refer to a ZIP
file, then a new ZIP archive is appended to the file. This is
meant for adding a ZIP archive to another file (such as
python.exe).

which is ambiguous at best and certainly suggests the resulting file would be usable as-is, which is not the case if you use mode "a" (but *is* the case if you use mode "w", since 342bc734f523).