After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 634735 - Permissions for .tar archives (and any other applicable)
Permissions for .tar archives (and any other applicable)
Status: RESOLVED FIXED
Product: gvfs
Classification: Core
Component: archive backend
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gvfs-maint
gvfs-maint
Depends on:
Blocks:
 
 
Reported: 2010-11-13 08:29 UTC by Erika Ahlswede
Modified: 2013-10-25 08:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
archive: Set file permissions from archive contents (2.28 KB, patch)
2013-10-06 09:50 UTC, Ross Lagerwall
reviewed Details | Review
archive: Set file executable bit from archive contents (2.20 KB, patch)
2013-10-07 20:11 UTC, Ross Lagerwall
accepted-commit_now Details | Review

Description Erika Ahlswede 2010-11-13 08:29:41 UTC
Currently, the libarchive backend for gvfs doesn't appear to handle permissions at all--it would be helpful for tasks such as extracting a source tarball, to expose the permissions contained in tar archives--otherwise, files such as the configure script have to be manually marked executable before being used.
Comment 1 Ross Lagerwall 2013-10-06 09:50:34 UTC
Created attachment 256558 [details] [review]
archive: Set file permissions from archive contents
Comment 2 Tomas Bzatek 2013-10-07 09:49:16 UTC
Review of attachment 256558 [details] [review]:

Generally the idea is not bad. However, libarchive handles many formats, not just POSIX-like TARs. Also there are different versions of ZIP archives out there, better to grab some old DOS era ZIP and test it against it. I'm concerned what does archive_entry_mode() return in that case.

::: daemon/gvfsbackendarchive.c
@@ +416,3 @@
 
+  mode = archive_entry_perm (entry);
+  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_READ, mode & S_IRUSR);

What is the point of exposing file/directory that can't be read by the owner itself? How would you extract a directory flagged like this?

@@ +417,3 @@
+  mode = archive_entry_perm (entry);
+  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_READ, mode & S_IRUSR);
+  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE, mode & S_IWUSR);

Please check whether applications do honour the G_FILE_ATTRIBUTE_FILESYSTEM_READONLY flag, otherwise this will lead to false information given for the case when user tries to edit a file directly from the archive. For extracting this is fine (this is somewhat similar to CD-ROM read-only flag).

@@ +420,2 @@
   g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE, FALSE);
+  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, mode & S_IXUSR);

Agreed with the execute flag.
Comment 3 Ross Lagerwall 2013-10-07 20:11:21 UTC
Created attachment 256670 [details] [review]
archive: Set file executable bit from archive contents
Comment 4 Ross Lagerwall 2013-10-07 20:15:35 UTC
(In reply to comment #2)
> Review of attachment 256558 [details] [review]:
> 
> Generally the idea is not bad. However, libarchive handles many formats, not
> just POSIX-like TARs. Also there are different versions of ZIP archives out
> there, better to grab some old DOS era ZIP and test it against it. I'm
> concerned what does archive_entry_mode() return in that case.

I have tested it against several archive types and formats and libarchive seems to give a sensible default of rw- for files and rwx for directories in the absence of any other information.

> 
> ::: daemon/gvfsbackendarchive.c
> @@ +416,3 @@
> 
> +  mode = archive_entry_perm (entry);
> +  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_READ,
> mode & S_IRUSR);
> 
> What is the point of exposing file/directory that can't be read by the owner
> itself? How would you extract a directory flagged like this?
> 
> @@ +417,3 @@
> +  mode = archive_entry_perm (entry);
> +  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_READ,
> mode & S_IRUSR);
> +  g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_WRITE,
> mode & S_IWUSR);
> 
> Please check whether applications do honour the
> G_FILE_ATTRIBUTE_FILESYSTEM_READONLY flag, otherwise this will lead to false
> information given for the case when user tries to edit a file directly from the
> archive. For extracting this is fine (this is somewhat similar to CD-ROM
> read-only flag).
> 
> @@ +420,2 @@
>    g_file_info_set_attribute_boolean (info, G_FILE_ATTRIBUTE_ACCESS_CAN_DELETE,
> FALSE);
> +  g_file_info_set_attribute_boolean (info,
> G_FILE_ATTRIBUTE_ACCESS_CAN_EXECUTE, mode & S_IXUSR);
> 
> Agreed with the execute flag.

Given these comments, I updated the patch to leave the read and write flags alone and only set the executable flag if it is a directory or it is explicitly marked executable.  This should at least fix the use case of running a configure script from a mounted archive.
Comment 5 Ondrej Holy 2013-10-17 14:04:54 UTC
Review of attachment 256558 [details] [review]:

I agree with tbaztek review. 

In writable subfolders e.g. Nautilus doesn't have grayed out option for creating new folder, however it fails with error:
Error while copying to "foobar".
The destination is not writable.

What is probably wrong.
Comment 6 Ondrej Holy 2013-10-17 14:05:04 UTC
Review of attachment 256670 [details] [review]:

It looks good.
Comment 7 Ross Lagerwall 2013-10-25 08:32:53 UTC
Pushed to master as 9299dac.