[ python-Bugs-1050828 ] reindent.py strips execute permission on Unix (original) (raw)

SourceForge.net noreply at sourceforge.net
Fri Nov 5 02:36:32 CET 2004

Bugs item #1050828, was opened at 2004-10-20 12:48 Message generated for change (Comment added) made by facundobatista You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1050828&group_id=5470

Category: Demos and Tools Group: Python 2.4 Status: Open Resolution: None Priority: 5 Submitted By: David Ripton (dripton) Assigned to: Facundo Batista (facundobatista) Summary: reindent.py strips execute permission on Unix

Initial Comment: Tools/scripts/reindent.py strips execute permission from its target file.

If the tool detects that changes need to be made, then the target file is renamed to its original name + '.bak', then a new file is created with the original file's name and its whitespace-modified contents. No manipulation of the new file's permissions is done, so it ends up with the default for the user's umask.

This makes reindent.py annoying to run in an automated fashion, if you rely on execute permission.

The fix is trivial, except for portability. Here's the Posix-only version:

Patch available upon request.

Comment By: Facundo Batista (facundobatista) Date: 2004-11-04 22:36

Message: Logged In: YES user_id=752496

I'll just let this bug live, don't want to commit it for 2.4 without bigger consensus.

Comment By: Facundo Batista (facundobatista) Date: 2004-10-28 21:49

Message: Logged In: YES user_id=752496

I could touch the permissions of the copy to be only readable, but with os.chmod() it'll be portable only to Windows and Unix (according to the docs). And in Mac?

Take note that in this very moment, the copy is created via the open(..., "w") which leaves the copy with the same permissions that with the patch. We're not creating a problem... maybe just don't solving it!

Comment By: Sjoerd Mullender (sjoerd) Date: 2004-10-27 14:34

Message: Logged In: YES user_id=43607

Now that I looked at the patch, I have one concern.

I don't mind if the backup copy doesn't have the same permissions as the original, especially not if you can't guarantee that the owner/group doesn't change. However, I do mind if the backup copy has more permissions than the original. Consider a file that was only readable by the owner (probably for some good reason), and because of a permissive umask, the backup copy all of a sudden is world-readable.

Comment By: Facundo Batista (facundobatista) Date: 2004-10-27 13:07

Message: Logged In: YES user_id=752496

That is exactly what the patch does, but David complained about the metadata of the copy.

Comment By: Sjoerd Mullender (sjoerd) Date: 2004-10-27 12:29

Message: Logged In: YES user_id=43607

Then perhaps it should do in-place replacement, so first copy the original to the backup, and then overwrite the original.

Comment By: Facundo Batista (facundobatista) Date: 2004-10-27 11:56

Message: Logged In: YES user_id=752496

I thought about it. The problem with this approach is that it also copies the permissions, but not the owner/group ids (you get the defaults there).

So, I think is better to leave the user defaults than mix the old permissions with new owner/group (which may lead to a security risk in some combinations).

There's a better approach: to copy all the file metadata. But don't know how to do it in a portable way...

Comment By: David Ripton (dripton) Date: 2004-10-27 11:47

Message: Logged In: YES user_id=9425

s/wrong/right/ in my last comment. The target is more important than the backup, but we should go for the win/win and give both of them the correct permissions, since it's easy.

Comment By: David Ripton (dripton) Date: 2004-10-27 11:45

Message: Logged In: YES user_id=9425

The patch is an improvement (better to have the wrong permissions on the target file than on the backup file), but I think it would be even better if the .bak file had the same permissions as the target file.

How about using shutil.copy2 (which preserves permissions, among other things) instead of shutil.copy?

Thanks.

Comment By: Johannes Gijsbers (jlgijsbers) Date: 2004-10-27 09:38

Message: Logged In: YES user_id=469548

The patch works for me (Linux 2.6.8-1-686 i686 GNU/Linux) and a coworker (Mac OS X 10.3).

Comment By: Facundo Batista (facundobatista) Date: 2004-10-26 21:44

Message: Logged In: YES user_id=752496

This fixes the problem: the original file metadata (permisions, etc) stays unchanged.

The issue with the metadata of the bak file is none: the new file has the user defaults permissions, group and owner. In the previous version, the original file left with those defaults permissions, so security is not getting worse.

The only left issue is, if the user wants to recover the data from the bak file, he would need to change its metadata (this is minor, if something goes wrong and the bak is needed, it's likely that the user will need a manual approach).

My primary concern is that I don't want to commit the changes now that we're beta without testing the changes in other platforms (however, I'll ask this on python-dev).

Comment By: Tim Peters (tim_one) Date: 2004-10-26 00:19

Message: Logged In: YES user_id=31435

Sorry, I don't normally run on Unix, so I have no way to tell whether this fixes whatever the complaint is. Offhand it doesn't seem right to create a .bak file with default permissions either. For example, if only the user had permission to get at the original, letting the .bak file have broader permissions seems wrong too.

Comment By: Facundo Batista (facundobatista) Date: 2004-10-25 23:57

Message: Logged In: YES user_id=752496

Attaching the file.

Comment By: Facundo Batista (facundobatista) Date: 2004-10-24 18:20

Message: Logged In: YES user_id=752496

Instead of renaming the file and creating a new one, just copied the original and that's all (the target file is open with write mode, the content replaced, but the permissions stay).

A patch is attached.

Tim, I'm assigning this to you to doublecheck (not found unit tests for this script).

. Facundo

You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=105470&aid=1050828&group_id=5470

More information about the Python-bugs-list mailing list