Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(139)

#9720: zipfile writes incorrect local file header for large files in zip64

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 2 months ago by craig.ds
Modified:
4 years, 10 months ago
Reviewers:
martin, storchaka
CC:
loewis, gregory.p.smith, Ronald Oussoren, amaury.forgeotdarc, alanmcintyre, christian.heimes, nadeem.vawda, eric.araujo, lambacck, site+python.org_davidandrzejewski.com, brice.nahum_free.fr, craig.ds_gmail.com, devnull_psf.upfronthosting.co.za, enlavin_gmail.com, paul.bauer.spearstone_gmail.com, storchaka, Kristof.Keppens, rubenrua_teltek.es, jhenry_ccs.neu.edu, nmoeller_uos.de
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Patch Set 3 #

Total comments: 5

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Patch Set 7 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Lib/zipfile.py View 1 2 3 4 5 6 5 chunks +32 lines, -15 lines 0 comments Download

Messages

Total messages: 3
loewis
http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py#newcode349 Lib/zipfile.py:349: def FileHeader(self, zip64=None): The default should be False, not ...
5 years, 2 months ago #1
loewis
http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py File Lib/zipfile.py (right): http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py#newcode349 Lib/zipfile.py:349: def FileHeader(self, zip64=None): > The default should be False, ...
5 years, 2 months ago #2
storchaka_gmail.com
5 years, 2 months ago #3
http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py
File Lib/zipfile.py (right):

http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py#newcode349
Lib/zipfile.py:349: def FileHeader(self, zip64=None):
On 2012/09/24 00:17:56, loewis wrote:
> > The default should be False, not None.
> 
> I now see why it's None; it means that the old computation is used.

Yes, FileHeader is not mentioned in the module documentation, but it's name is
not starts with underscore and it has a docstring. Someone can use FileHeader
and we not want to break this in a bugfix release.

It might be worth it to start the procedure for the deprecating of FileHeader?

http://bugs.python.org/review/9720/diff/6041/Lib/zipfile.py#newcode1419
Lib/zipfile.py:1419: self.fp.flush()
On 2012/09/24 00:15:00, loewis wrote:
> This change seems unrelated. Why is it even necessary?

I think flush() has been placed before writing of data descriptor because
writing can raise exception if sizes greater a 32-bit limit. Now this reason
eliminated.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7