Make zipimport work with zipfile containing comments

@Yhg1s

BPO	5950
Nosy	@Yhg1s, @warsaw, @brettcannon, @theller, @birkenfeld, @terryjreedy, @gpshead, @ncoghlan, @ericsnowcurrently, @dimaqq, @serhiy-storchaka, @ZackerySpytz
PRs	bpo-5950: Support reading zips with comments in zipimport #9548
Files	testcase.zip: Testcase zipfile with comment inside zipimport_with_comments.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 = None
closed_at = <Date 2018-09-25.19:16:49.640>
created_at = <Date 2009-05-06.16:49:46.012>
labels = ['3.8', 'type-feature', 'library']
title = 'Make zipimport work with zipfile containing comments'
updated_at = <Date 2018-09-25.19:16:49.638>
user = 'https://bugs.python.org/dsamersoff'

bugs.python.org fields:

activity = <Date 2018-09-25.19:16:49.638>
actor = 'barry'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-25.19:16:49.640>
closer = 'barry'
components = ['Library (Lib)']
creation = <Date 2009-05-06.16:49:46.012>
creator = 'dsamersoff'
dependencies = []
files = ['13905', '17975']
hgrepos = []
issue_num = 5950
keywords = ['patch']
message_count = 25.0
messages = ['87340', '97316', '109678', '109686', '110014', '110016', '110059', '110151', '110157', '113452', '113467', '113484', '113486', '139187', '139192', '175127', '175169', '219263', '219266', '226234', '325717', '325721', '325733', '326284', '326398']
nosy_count = 17.0
nosy_names = ['twouters', 'barry', 'brett.cannon', 'theller', 'georg.brandl', 'terry.reedy', 'gregory.p.smith', 'ncoghlan', 'peter.otten', 'grubert', 'rfk', 'dsamersoff', 'eric.snow', 'Dima.Tisnek', 'serhiy.storchaka', 'superluser', 'ZackerySpytz']
pr_nums = ['9548']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue5950'
versions = ['Python 3.8']

Synopsys:
zimport not able to import a module from zipfile if zipfile contains
comment.
Versions:
This is Zip 2.32 (June 19th 2006), by Info-ZIP
Python 2.5.2 or 2.6.2

Steps to reproduce:
create a module, create an app that imports this module.
zip the module, make sure it works.
Run: echo "Some comments" | zip -z module.zip
the app stop working.

Yes, comments are not supported right now. Documented this in r77333, and turning this issue into a feature request for adding that support.

Does 'zimport' refer to the 3.1 zipfile or zipimport modules or an internal function called zimport?

I'm talking about internal zimport function (see attached testcase):

i.e.

import sys;
sys.path.insert(0,'test.zip');

import test
test.testme()

doesn't work if test.zip contains comment.

Attached is my attempt at a patch for this functionality, along with some simple tests. This basically mirrors what's done in zipfile.py, searching backwards through the file until it finds the end-of-central-directory marker. It tries to be memory conscious by reading in small chunks.

Patch is against trunk; I've also tested it against 2.7 and it seems to work. Any chance of it being backported into 2.7?

Also wanted to mention a real-world usecase for this functionality. I want to digitally sign a frozen python program with appended zipfile, which involves appending the signature to the EXE. Simple to do if only zipimport would support appended comments.

Whoops, forgot to remove the line from the docs about comments not being supported. Updated the patch accordingly.

"the line from the docs about comments not being supported." answers your question. Current behavior is documented behavior and therefor changing it is not a bugfix.

I can't imagine anyone depending on this lack-of-feature, but there's no arguing with the technicality of it. One more small incentive to make the jump to Python 3 then.

Anyway, I've revisited the patch to clean up the logic and control flow, and added another test to ensure that it works even when the EOCD record crosses a chunk boundary. Also checked that it works on win32, which fortunately it did without modification.

I can't imagine anyone depending on this lack-of-feature,

Neither can I, but I can imaging someone writing code that depended on the feature. If added to 3.1.3, then code that depended on it would not run on 3.1, 3.1.1, and 3.1.2. The same is true, of course, for bug fixes, but the doc does not need changing for bug fixes, and the impact is otherwise less.

One more small incentive to make the jump to Python 3 then.

Or to 3.2 from any 3.1 version, which is the normal progession absent the dual track anomaly.

I believe this is covered by the PEP-3003 3.2 change moratorium.

Why?

also applies to 2.7 series

over a year passed since last comment, any progress on this?

I just ran into this issue myself.
zipfile module handles commented zip's fine, but zipimport doesn't.
I didn't expect a gotcha like this from Python!

This is a feature request so it won't change in Python 2.7.

As for progress, the answer is no as the hope is to eventually replace zipimport with something in pure Python thanks to importlib.

As for progress, the answer is no as the hope is to eventually replace
zipimport with something in pure Python thanks to importlib.

Does this mean there is no chance to put this into 3.4 and 3.3, do we really have to wait until 3.5 with it's pure-python zipimport?

I (together with the py2exe-users) have the same usecase as Ryan Kelly in msg110014:

I want to digitally sign a frozen python program with appended zipfile,
which involves appending the signature to the EXE. Simple to do if
only zipimport would support appended comments.

I actually stopped development of a pure Python zip importer so it won't be in Python 3.5 either (http://bugs.python.org/issue17630). The motivation simply wasn't there if zipimport is going to be sticking around for bootstrapping reasons. This can still get fixed in 3.5 (3.4 is Larry's call).

I have added Thomas Wouters and Greg Smith who have done the most work with zipimport to the nosy list to see if they have interest in taking a look at the patch.

See bpo-22322 for yet one use case ("git archive" creates ZIP file with archive comment).

zipimport have been rewritten in pure Python (bpo-25711).

Very glad to hear!
Let's document what Python version(s) are "fixed".
Perhaps this issue deserves a test case in bpo-25711.

It is not fixed yet. But it is now easier to fix.

I've created a PR.

New changeset 5a5ce06 by Barry Warsaw (Zackery Spytz) in branch 'master':
bpo-5950: Support reading zips with comments in zipimport (bpo-9548)
5a5ce06

Navigation Menu

Repository navigation

Make zipimport work with zipfile containing comments #50200

4 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Make zipimport work with zipfile containing comments #50200

Description

Activity

dsamersoff commented on May 6, 2009

birkenfeld commented on Jan 6, 2010

terryjreedy commented on Jul 9, 2010

dsamersoff commented on Jul 9, 2010

rfk commented on Jul 11, 2010

rfk commented on Jul 11, 2010

terryjreedy commented on Jul 12, 2010

rfk commented on Jul 13, 2010

terryjreedy commented on Jul 13, 2010

terryjreedy commented on Aug 9, 2010

birkenfeld commented on Aug 9, 2010

4 remaining items

dimaqq commented on Nov 7, 2012

brettcannon commented on Nov 8, 2012

theller commented on May 28, 2014

brettcannon commented on May 28, 2014

serhiy-storchaka commented on Sep 1, 2014

serhiy-storchaka commented on Sep 19, 2018

dimaqq commented on Sep 19, 2018

serhiy-storchaka commented on Sep 19, 2018

ZackerySpytz commented on Sep 24, 2018

warsaw commented on Sep 25, 2018

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions