homepage

Run the following Python script, on Unix/Linux:
==========
import zipfile
z = zipfile.ZipFile('zipbad.zip', 'w')
z.writestr('filebad.txt', 'Some content')
z.close()
z = zipfile.ZipFile('zipgood.zip', 'w')
zi = zipfile.ZipInfo('filegood.txt')
zi.external_attr = 0660 << 16L
z.writestr(zi, 'Some content')
z.close()
==========
Like this:
python testzip.py && unzip zipbad.zip && unzip zipgood.zip && ls -l
file*.txt
You'll see:
---------- 1 swarren swarren 12 2008年07月17日 12:54 filebad.txt
-rw-rw---- 1 swarren swarren 12 1980年01月01日 00:00 filegood.txt
Note that filebad.txt is extracted with mode 000.
The WAR (used for filegood.txt) is to pass writestr a ZipInfo class with
external_attr pre-initialized. However, writestr should perform this
assignment itself, to be consistent with write. I haven't checked, but
there's probably a bunch of other stuff in write that writestr should do
too.

Oops. Forgot to set "type" field.

What value should the new archive entry's external_attr attribute have?
ZipFile.write sets it to the permissions of the file being archived, but
writestr is archiving a string, not a file. Setting it to 0600 << 16
seems reasonable.
Stephen's script exposed a second bug in writestr. When passed a name
rather than a ZipInfo instance, the new archive member receives a timestamp
of 01/01/1980. However, the docs say that the timestamp should correspond to
the current date and time.
ZipFile.writestr begins with the following code:
 def writestr(self, zinfo_or_arcname, bytes):
 """Write a file into the archive. The contents is the string
 'bytes'. 'zinfo_or_arcname' is either a ZipInfo instance or
 the name of the file in the archive."""
 if not isinstance(zinfo_or_arcname, ZipInfo):
 zinfo = ZipInfo(filename=zinfo_or_arcname,
 date_time=time.localtime(time.time())[:6])
 zinfo.compress_type = self.compression
The "date_time=" line should read:
 zinfo.date_time=time.localtime(time.time())[:6])

I'd probably argue for at least 0660<<16, if not 0666<<16, since group
permissions are pretty typically set, but even 0666<<16 would be OK,
since the umask on extraction would take away any permissions the
extracting user didn't want.
But, as long as the chosen mask includes at least 0600, I'd consider the
issue fixed.

Here is a patch containing code and a unit test. I set external_attr
to 0600, for the following reason.
When I extract with Infozip, my umask is ignored when setting permissions of
extracted entries. They have the permissions assigned to them when archived.
tar does respect umask, but it's not pertinent.
The following shell script demonstrates Infozip's behavior:
=====begin=====
#!/bin/sh
mkdir ziptest_dir
echo hello > ziptest_dir/foo.txt
chmod 666 ziptest_dir/foo.txt
zip -r ziptest_file.zip ziptest_dir/
rm -rf ziptest_dir
umask 077
unzip ziptest_file.zip
=====end=====
Setting permissions to 0600 seems like the safest course.
I'm not sure if this patch should be accompanied by some documentation,
since the zipfile docs don't say much about external_attr or permissions.
PS. My earlier comments about timestamps were incorrect and spurious!

Agree with using 0600 as default permissions.

Committed in r65235. Thanks!