fs.copySync fails if user does not own file

[bdcribbs]

fs.copySync fails when the user running the script does not own the destination file, even when the user actually has sufficient permissions to perform the action.

The error occurs if the file exists, and clobber is true. In that case, copySync attempts to chmod the file prior to unlinking it (prior to performing the copy). Since the user doesn't own the file, the chmod fails with EPERM, even though the operation could have succeeded -- provided the user has sufficient permissions to unlink the file and recreate it (which depends on the permissions of the directory the file is in -- not the file itself).

The line in question is here:
https://github.com/jprichardson/node-fs-extra/blob/master/lib/copy-sync/copy-file-sync.js#L12

[jprichardson]

Thanks so much! Would you be willing to add a test?

[bdcribbs]

I'm not sure how we'd write such a test, since it requires creating a file as one user account and then attempting to manipulate it with another user account.

[jprichardson]

Good point. Maybe throw that line in a try/catch block?

[bdcribbs]

A try/catch would work. Another point of view is that attempting to copy over the contents of a read-only file really should fail, because that's what read-only means.

[jprichardson]

Another point of view is that attempting to copy over the contents of a read-only file really should fail, because that's what read-only means.

Hah. Excellent point as well.

So for a test, couldn't we just set a couple of files to be read only and try to copy over them and assert for an error? The error should have a descriptive message indicating that the destination is read-only.

[bdcribbs]

That works, and also matches how I would expect it to function (I was somewhat surprised when I found out copySync was calling chmod in the first place).

It's worth considering that this would revert the change that was originally made for #183, but in my opinion, it's more appropriate for the caller to chmod the file themselves if they need to.

If you'd like I could throw together a pull request, but I didn't want to revert #183 without your agreement.

[jprichardson]

@bdcribbs thanks for referencing the issue. In context it makes more sense. Don't you think that when clobber (aka overwrite) is set to true, it should overwrite regardless of whether the file is read only?

Pinging: @bartland for his feedback.

[bdcribbs]

I see. I thought clobber was for overwriting files that already exist (EEXIST), not EPERM. Now the behavior makes more sense.

Since there are actually two concerns here, how about two options instead one? "clobberExisting" and "clobberReadonly". That way, there is zero ambiguity about what each option does.

For backwards compatibility, "clobber" could be equivalent to setting both of the new options to true.

[bartland]

Seems a reasonable idea for more fine grained control given we would still have "clobber" for both.

In regard to #183, the reason for introducing the chmod call was to support node 0.10 (bug?). If support for 0.10 was dropped, the chmod call could be dropped. Which is what I'd prefer.

[jprichardson]

Since there are actually two concerns here, how about two options instead one? "clobberExisting" and "clobberReadonly". That way, there is zero ambiguity about what each option does.

Yes, I like this idea.

In regard to #183, the reason for introducing the chmod call was to support node 0.10 (bug?). If support for 0.10 was dropped, the chmod call could be dropped. Which is what I'd prefer.

I see. That makes sense. I'll be dropping support for Node v0.10 in April 2016. At that time, the next Ubuntu LTS will be released. Ubuntu LTS v0.14 defaults to Node v0.10.

[RyanZim]

@jprichardson Shall I try removing the chmod call?

[jprichardson]

@jprichardson Shall I try removing the chmod call?

Yep, sounds good.

[purepear]

I have the same issue with copy. Node user have permissions to copy the file to a different user's directory but fs.copy then fails to chmod the copied file and the whole thing fails Error: ENOENT: no such file or directory, chmod /some-direcotry/file.txt