This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Zipfile.extractall does not preserve file permissions
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.11
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Aaron.Train, Claudiu.Popa, Glenn.Jones, Orborde, belopolsky, bwanamarko, cbrannon, desaintmartin, eric.araujo, kmtracey, kulakov-n, lars.gustaebel, nanjekyejoannah, pitrou, r.david.murray, ronaldoussoren, sam_ezeh, selimb, serhiy.storchaka, swarren, techtonik, uruz, yossarian, Étienne Dupuis
Priority: normal Keywords: patch

Created on 2012-08-27 22:36 by uruz, last changed 2022-04-11 14:57 by admin.

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
Pull Requests
URL Status Linked Edit
PR 17790 open python-dev, 2020-01-01 23:41
PR 32289 open sam_ezeh, 2022-04-03 15:48
Messages (24)
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: PCManticore (Claudiu.Popa) * (Python triager) 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.)
msg311328 - (view) Author: Étienne Dupuis (Étienne Dupuis) Date: 2018-01-31 13:31
A workaround is provided here: https://stackoverflow.com/questions/39296101/python-zipfile-removes-execute-permissions-from-binaries
msg378712 - (view) Author: Nikolay (kulakov-n) Date: 2020-10-16 12:24
Is there any chance that the pull request will be accepted? I'm a bit tired of using workaround every time I need unzip something on linux.
msg394819 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2021-05-31 17:04
The pull request needs unit tests added to validate the changes.

Note that the patch attached here was a new feature, adding constants and parameters to control the behaviour, but the PR simply checks and applies permissions bits stored in the entry.  That seems correct and nice to me, and arguably a bugfix that should be backported, but more active core devs and/or zipfile experts should weigh in.
msg404877 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python committer) Date: 2021-10-23 14:33
I left a review on the PR requesting for some tests, if it makes sense.
msg416661 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-04-04 10:54
The new PR uses new constants*, so could not be backported as is (see my previous message).


(*side question: should the constants use an enum?)
msg416672 - (view) Author: Sam Ezeh (sam_ezeh) * Date: 2022-04-04 14:40
I don't know what the best course of action would be but if preserving permissions needs to be back-ported, could the default permission preservation flag in 3.11+ be the one to preserve safe permissions and then make it so that the previous versions (<3.11, without the constants) always take this course of action? Maintaining the different options for preserving permissions while still allowing for this functionality to be back-ported.

I don't have a strong opinion on backporting permission preservation but to me, it seems like it would be a change in behaviour instead of a bug fix. The current default in the PR is to not preserve any permissions but if necessary, I'll change it to whatever is agreed upon.

I'll move the constants into an enum, but right now I'm not sure how I'd name the class.

As an aside, while writing this comment I realised that the reason tests aren't passing on my end might very well be due to the fact I do CPython work on an NTFS partition instead of on my main EXT4 partition.
msg416674 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2022-04-04 14:55
On second thought, maybe no fix should be backported.
Changing behaviour silently might break things, changing with a new option puts the change in a grey fix/enhancement area, so maybe better to let current versions as they are, with the bug/limitation noted in the documentation, and give control in 3.11 only.
History
Date User Action Args
2022-04-11 14:57:35adminsetgithub: 59999
2022-04-04 14:55:21eric.araujosetmessages: + msg416674
2022-04-04 14:40:22sam_ezehsetmessages: + msg416672
2022-04-04 10:54:30eric.araujosetversions: + Python 3.11, - Python 3.10
2022-04-04 10:54:23eric.araujosetmessages: + msg416661
2022-04-03 15:48:53sam_ezehsetpull_requests: + pull_request30350
2022-04-03 15:46:25sam_ezehsetnosy: + sam_ezeh
2021-10-25 20:24:21selimbsetnosy: + selimb
2021-10-23 14:33:42nanjekyejoannahsetnosy: + nanjekyejoannah
messages: + msg404877
2021-05-31 17:04:03eric.araujosetnosy: + eric.araujo

messages: + msg394819
versions: + Python 3.10, - Python 3.5
2021-04-14 17:41:40yossariansetnosy: + yossarian
2020-10-16 12:24:14kulakov-nsetnosy: + kulakov-n
messages: + msg378712
2020-01-01 23:41:34python-devsetstage: needs patch -> patch review
pull_requests: + pull_request17223
2018-01-31 13:38:48arobertsetnosy: - arobert
2018-01-31 13:31:05Étienne Dupuissetnosy: + Étienne Dupuis
messages: + msg311328
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