classification
Title: Zipfile.extractall does not preserve file permissions
Type: enhancement Stage: needs patch
Components: Library (Lib) Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Aaron.Train, Claudiu.Popa, Glenn.Jones, Orborde, arobert, belopolsky, bwanamarko, cbrannon, desaintmartin, kmtracey, lars.gustaebel, pitrou, r.david.murray, ronaldoussoren, serhiy.storchaka, swarren, techtonik, uruz
Priority: normal Keywords: patch

Created on 2012-08-27 22:36 by uruz, last changed 2016-12-24 04:27 by kmtracey.

Files
File name Uploaded Description Edit
issue15795.patch uruz, 2012-09-01 13:10 review
issue15795_updated.patch Glenn.Jones, 2014-04-14 21:55 review
issue15795_cleaned.patch Glenn.Jones, 2014-04-15 14:46 review
issue15795_test_and_doc_fixes.patch Glenn.Jones, 2014-04-15 20:12 review
Messages (17)
msg169233 - (view) Author: Alexey Boriskin (uruz) * Date: 2012-08-27 22:36
Zipfile._extract_member does not preserve file permissions while extracting it. As may be seen at link[1], raw open() is used and no os.chmod() applied after that, therefore any permission data stored in zipfile is dropped and file is created with default permission depending on current user's umask.

[1] http://hg.python.org/cpython/file/52159aa5d401/Lib/zipfile.py#l1251
msg169242 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-08-28 00:33
Does this have any relationship to issue 3394?  From the discussion on that issue it sounds like zipfile is doing things with external_attributes if it is set.  But I don't know much about zipfile internals.
msg169629 - (view) Author: Alexey Boriskin (uruz) * Date: 2012-09-01 13:10
I'm attaching a patch, which solves the issue. Patch intoduces new argument "preserve_permissions" for extract and extractall methods. That argument may accept one of the three values: do not preserve permissions, preserve a safe subset of them or preserve all permissions. Three constants introduced for these options. Patch also contains test and docs. Tests pass for me on OS X 10.5, but I'm not sure if they'll pass on other operating systems.
msg174356 - (view) Author: Aaron Train (Aaron.Train) Date: 2012-10-31 19:33
Thanks for the patch. Is this going to be resolved soon?
msg174359 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-10-31 19:48
OK, so this is an enhancement to specifically allow preservation of "unsafe" permissions?

Adding the nosy list from issue 3394.
msg191425 - (view) Author: anatoly techtonik (techtonik) Date: 2013-06-18 18:54
There should be an easy way to restore file attributes.
msg191459 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2013-06-19 08:31
On first glance the patch looks good. I haven't tested it with the current trunk though.
msg216228 - (view) Author: Glenn Jones (Glenn.Jones) * Date: 2014-04-14 21:55
Here is an updated patch that applies cleanly to head. Tests pass against head of repo.
msg216267 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-04-15 01:47
Thanks.

The patch contains a number of lines that are not wrapped to <80, which is one of our requirements.  It would be great to get that fixed.  (In the documentation, you can use \ to wrap the prototype line.)

There is non-ascii in one place in the documentation...probably an em-dash, which is written in sphinx as '---'.

Also, please remove the news entry from the patch, it will just make it harder to apply (and is in the wrong place anyway...we add entries at the top of the appropriate section, not the bottom).
msg216298 - (view) Author: Glenn Jones (Glenn.Jones) * Date: 2014-04-15 14:46
Patch cleaned up based on previous comments.
msg216304 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-15 15:04
Hello. I added a couple of comments to your latest patch.
msg216376 - (view) Author: Glenn Jones (Glenn.Jones) * Date: 2014-04-15 20:12
Patch with docs and tests fixed
msg225526 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-19 10:16
The TarFile.extract() method has the set_attrs parameter which controls similar behavior but is less flexible. It would be good to unify zipfile and tarfile abilities. set_attrs also controls setting file owner and time. When we restore unsafe uid/gid/sticky bits, it would be worth to restore also file owner. And it would be not bad to restore file time.
msg233445 - (view) Author: ABR (arobert) Date: 2015-01-05 08:07
I hope this can be finally gotten in for 3.5, even if it's not the perfect solution.  I hit this issue and needed to call out to a subprocess as a work-around, but that's far less reliable.
msg237139 - (view) Author: Alexey Boriskin (uruz) * Date: 2015-03-03 14:21
I'm working on updating the patch to unify tarfile and zipfile interfaces and to restore owner/timestamp for zipfile
msg261145 - (view) Author: Mark Mikofski (bwanamarko) Date: 2016-03-03 01:26
same problem in 2.7.5 on Oracle Linux 7.2
msg283918 - (view) Author: Karen Tracey (kmtracey) Date: 2016-12-24 04:27
Note the zipfile being processed may have been created on a non-Unix system, and the external_attr value can't be usefully interpreted as permission bits when the value at _CD_CREATE_SYSTEM (https://hg.python.org/cpython/file/default/Lib/zipfile.py#l107) in the central directory entry is not Unix (3). Patches so far don't seem to guard against mistakenly assuming a foreign-system external_attr value can be interpreted as Unix permission bits.

(At present github is delivering zipfiles of repos with a create system value of 0 and external_attr values of 0.)
History
Date User Action Args
2016-12-24 04:27:07kmtraceysetnosy: + kmtracey
messages: + msg283918
2016-03-03 01:26:04bwanamarkosetnosy: + bwanamarko
messages: + msg261145
2015-03-04 02:04:41r.david.murraysetstage: commit review -> needs patch
2015-03-03 14:21:51uruzsetmessages: + msg237139
2015-02-08 04:11:30belopolskysetnosy: + belopolsky
2015-01-05 16:31:41r.david.murraysetstage: patch review -> commit review
2015-01-05 08:07:11arobertsetnosy: + arobert
messages: + msg233445
2014-08-19 10:16:11serhiy.storchakasetversions: + Python 3.5, - Python 3.4
nosy: + serhiy.storchaka, lars.gustaebel

messages: + msg225526

assignee: serhiy.storchaka
2014-06-16 00:14:26Orbordesetnosy: + Orborde
2014-06-13 07:05:57Claudiu.Popasetstage: patch review
2014-04-15 20:12:07Glenn.Jonessetfiles: + issue15795_test_and_doc_fixes.patch

messages: + msg216376
2014-04-15 15:04:56Claudiu.Popasetnosy: + Claudiu.Popa
messages: + msg216304
2014-04-15 14:46:30Glenn.Jonessetfiles: + issue15795_cleaned.patch

messages: + msg216298
2014-04-15 01:47:14r.david.murraysetmessages: + msg216267
2014-04-14 21:55:13Glenn.Jonessetfiles: + issue15795_updated.patch
nosy: + Glenn.Jones
messages: + msg216228

2013-06-19 08:31:53ronaldoussorensetnosy: + ronaldoussoren
messages: + msg191459
2013-06-19 08:26:50ronaldoussorenlinkissue18262 superseder
2013-06-18 18:54:31techtoniksetmessages: + msg191425
2013-06-18 18:47:19techtoniksetnosy: + techtonik
2013-03-01 16:28:02desaintmartinsetnosy: + desaintmartin
2012-10-31 19:48:02r.david.murraysetversions: - Python 2.7
nosy: + pitrou, swarren, cbrannon

messages: + msg174359

type: behavior -> enhancement
2012-10-31 19:33:18Aaron.Trainsetnosy: + Aaron.Train
messages: + msg174356
2012-09-01 13:10:52uruzsetfiles: + issue15795.patch
keywords: + patch
messages: + msg169629
2012-09-01 13:09:10r.david.murraysethgrepos: - hgrepo147
2012-09-01 12:39:58uruzsethgrepos: + hgrepo147
2012-08-28 00:33:46r.david.murraysetnosy: + r.david.murray
messages: + msg169242
2012-08-27 22:36:03uruzcreate