Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cp: fail when trying to copy to read only file on mac #5261

Merged
merged 5 commits into from
Oct 2, 2023

Conversation

PGIII
Copy link
Contributor

@PGIII PGIII commented Sep 7, 2023

Fixes issue #5257 by first checking if file is read only or not before removing file and trying clonefile again

@PGIII PGIII changed the title cp: fail when trying to copy to read only file cp: fail when trying to copy to read only file on mac Sep 7, 2023
@cakebaker cakebaker linked an issue Sep 8, 2023 that may be closed by this pull request
@tertsdiepraam
Copy link
Member

Cool! Thanks! Since you're on mac, would you be able to figure out if this is at all related to the --no-clobber flag? I presume that is handled elsewhere, but it would still be nice to check

@PGIII
Copy link
Contributor Author

PGIII commented Sep 8, 2023

Looking at the man page for cp on mac it looks like there is a flag to use clonefile for the copy -c. Which does let you overwrite a read only file. without that flag however it will fail with Permission Denied as expected. Might be better to have clonefile behind a flag like that if you want to exactly match the MacOs behavior. Not sure why clonefile can overwrite read only files though.

@tertsdiepraam
Copy link
Member

tertsdiepraam commented Sep 8, 2023

Interesting, I can't find -c online. Do you have a link to an online manpage/documentation that explains it? In general though, we don't try to be conpatible with the default cp on mac but the GNU cp on mac. So I think for us it's the reflink flag that controls this.

@PGIII
Copy link
Contributor Author

PGIII commented Sep 8, 2023

Can't find it online but here what man cp on my mac shows

CP(1)                                                                                                                                                                                                     General Commands Manual                                                                                                                                                                                                    CP(1)

NAME
     cp – copy files

SYNOPSIS
     cp [-R [-H | -L | -P]] [-fi | -n] [-alpsvXx] source_file target_file
     cp [-R [-H | -L | -P]] [-fi | -n] [-alpsvXx] source_file ... target_directory
     cp [-f | -i | -n] [-alPpsvx] source_file target_file
     cp [-f | -i | -n] [-alPpsvx] source_file ... target_directory

DESCRIPTION
     In the first synopsis form, the cp utility copies the contents of the source_file to the target_file.  In the second synopsis form, the contents of each named source_file is copied to the destination target_directory.  The names of the files themselves are not changed.  If cp detects an attempt to copy a file to itself, the copy will fail.

     The following options are available:

     -H    If the -R option is specified, symbolic links on the command line are followed.  (Symbolic links encountered in the tree traversal are not followed.)

     -L    If the -R option is specified, all symbolic links are followed.

     -P    No symbolic links are followed.  This is the default if the -R option is specified.

     -R    If source_file designates a directory, cp copies the directory and the entire subtree connected at that point.  If the source_file ends in a /, the contents of the directory are copied rather than the directory itself.  This option also causes symbolic links to be copied, rather than indirected through, and for cp to create special files rather than copying them as normal files.  Created directories have the
           same mode as the corresponding source directory, unmodified by the process' umask.

           In -R mode, cp will continue copying even if errors are detected.

           Note that cp copies hard linked files as separate files.  If you need to preserve hard links, consider using tar(1), cpio(1), or pax(1) instead.

     -a    Archive mode.  Same as -RpP options. Preserves structure and attributes of files but not directory structure.

     -f    If the destination file cannot be opened, remove it and create a new file, without prompting for confirmation regardless of its permissions.  (The -f option overrides any previous -n option.)

           The target file is not unlinked before the copy.  Thus, any existing access rights will be retained.

     -i    Cause cp to write a prompt to the standard error output before copying a file that would overwrite an existing file.  If the response from the standard input begins with the character ‘y’ or ‘Y’, the file copy is attempted.  (The -i option overrides any previous -n option.)

     -l    Create hard links to regular files in a hierarchy instead of copying.

     -n    Do not overwrite an existing file.  (The -n option overrides any previous -f or -i options.)

     -p    Cause cp to preserve the following attributes of each source file in the copy: modification time, access time, file flags, file mode, user ID, and group ID, as allowed by permissions.  Access Control Lists (ACLs) and Extended Attributes (EAs), including resource forks, will also be preserved.

           If the user ID and group ID cannot be preserved, no error message is displayed and the exit value is not altered.

           If the source file has its set-user-ID bit on and the user ID cannot be preserved, the set-user-ID bit is not preserved in the copy's permissions.  If the source file has its set-group-ID bit on and the group ID cannot be preserved, the set-group-ID bit is not preserved in the copy's permissions.  If the source file has both its set-user-ID and set-group-ID bits on, and either the user ID or group ID cannot be
           preserved, neither the set-user-ID nor set-group-ID bits are preserved in the copy's permissions.

     -s    Create symbolic links to regular files in a hierarchy instead of copying.

     -v    Cause cp to be verbose, showing files as they are copied.

     -X    Do not copy Extended Attributes (EAs) or resource forks.

     -x    File system mount points are not traversed.

     -c    copy files using clonefile(2)

     For each destination file that already exists, its contents are overwritten if permissions allow.  Its mode, user ID, and group ID are unchanged unless the -p option was specified.

     In the second synopsis form, target_directory must exist unless there is only one named source_file which is a directory and the -R flag is specified.

     If the destination file does not exist, the mode of the source file is used as modified by the file mode creation mask (umask, see csh(1)).  If the source file has its set-user-ID bit on, that bit is removed unless both the source file and the destination file are owned by the same user.  If the source file has its set-group-ID bit on, that bit is removed unless both the source file and the destination file are in the
     same group and the user is a member of that group.  If both the set-user-ID and set-group-ID bits are set, all of the above conditions must be fulfilled or both bits are removed.

     Appropriate permissions are required for file creation or overwriting.

     Symbolic links are always followed unless the -R flag is set, in which case symbolic links are not followed, by default.  The -H or -L flags (in conjunction with the -R flag) cause symbolic links to be followed as described above.  The -H, -L and -P options are ignored unless the -R option is specified.  In addition, these options override each other and the command's actions are determined by the last one specified.

     If cp receives a SIGINFO (see the status argument for stty(1)) signal, the current input and output file and the percentage complete will be written to the standard output.

     If cp encounters an I/O error during the copy, then cp may leave a partially copied target_file in place.  cp specifically avoids cleaning up the output file in error cases to avoid further data loss in cases where the source may not be recoverable.  Alternatives, like install(1), may be preferred if stronger guarantees about the target_file are required.

@PGIII
Copy link
Contributor Author

PGIII commented Sep 8, 2023

cp -c does fail if the directory the destination file is in doesnt have write permission, interestly it fails with cp: invalid/invalid.txt: clonefile failed: File exists

@PGIII
Copy link
Contributor Author

PGIII commented Sep 8, 2023

reading through reflink behavior it seems like these changes should match whats expected in gnu cp. Seems like -c is equivalent to --reflink=always link

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for picking this up again! This is a trickier issue than it might appear, because the whole file removal thing might be wrong...

For example, the linux equivalent first opens the files for reading, before using a syscall. Maybe that's the better way to approach this. For example using fclonefileat instead of clonefile. It's a bit unfortunate I don't have a mac, so I can't check this for myself. Could you maybe check whether that would work?

If not then I think we should merge (a simplified version) of this patch, because it still fixes an important issue, maybe with a comment to state that it is a workaround and that this code should be expanded and rewritten.

src/uu/cp/src/platform/macos.rs Outdated Show resolved Hide resolved
src/uu/cp/src/platform/macos.rs Outdated Show resolved Hide resolved
@PGIII
Copy link
Contributor Author

PGIII commented Sep 10, 2023

No problem happy to help out! I can take a look at reimplementing this to match the Linux version better this week.

@PGIII
Copy link
Contributor Author

PGIII commented Sep 11, 2023

looking at fclonefileat(int srcfd, int dst_dirfd, const char * dst, int flags) it only takes a fd for the parent dir, not the destination file. Seems like the reason the linux version returns permission denied properly is because it first attempts to open the file with File::create here

@tertsdiepraam
Copy link
Member

Hi! Sorry for not responding for a while, I was very busy... Anyway, let's pick this back up! I think the File::create would be a good idea, but we can merge this first too. What do you think?

@PGIII
Copy link
Contributor Author

PGIII commented Oct 2, 2023

Yeah, I think merging this first is good. Should be ready to go

Copy link
Member

@tertsdiepraam tertsdiepraam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Do you want to pick up that followup PR as well? I made a new issue for this, if you're interested in picking this up just respond there.

@tertsdiepraam tertsdiepraam merged commit 252d01a into uutils:main Oct 2, 2023
44 of 45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

cp: copy to readonly file should fail on mac
2 participants