Crystal: [RFC] Add File instance chmod/chown

[didactic-drunk]

Proposal:

File.open(...) do |file|
  file.chown(uid, gid) # Matches existing `File.chown` arguments.
  file.chmod(perm) # Matches existing `File.chmod` arguments.
end

[straight-shoota]

File instances are only meant to be used as an IO. I'd very much prefer to keep all other operations as class methods (or ideally even move them to a separate namespace combined with the sister methods from Dir). They would just be aliases, and I don't see a particularly relevant use case for instance methods. Since the OS APIs are based on paths there's not even any benefit for reusing a file handle.

File#delete is the only exception to this and it was probably a mistake to add this in the process of removing Tempfile (#6485). I'd rather remove that than add others.

[didactic-drunk]

Since the OS APIs are based on paths.

Incorrect. I was suggeting use of fchmod and fchown.

there's not even any benefit.

Incorrect. Attempting to do file operations by path as root in non-root owned directories creates race conditions which this issue closes.

It's also slightly faster with some network file systems.

[RX14]

Race-free file system handling is a tricky subject, we should try and support the OS-provided functions for doing this.

[RX14]

On windows there is SetNamedSecurityInfo (chown) and SetSecurityInfo (fchown, takes a handle)

[didactic-drunk]

One 👍 up and one 👎 . Where does that leave this?

[oprypin]

One 👍 up and one 👎 . Where does that leave this?

If only everything was that simple :(
https://mobile.twitter.com/waj/status/1250486551898918914

[straight-shoota]

Sure, go ahead if you wan to implement this 👍

[j8r]

It is a good idea, but not on the existing File (which is an IO).

I would like to have OOP for file system interactions, like in Java or C#.
The only place I see is introducing System::File:

file = System::File:new "my_file"
file.chmod 750
file.chown 0, 0

And even having an overload System::File#chown(user : System::User, group : System::Group)

[waj]

I think the whole idea was to use the fd, not the path. @didactic-drunk can you describe the race condition? I'm not aware of that issue.

[j8r]

Right @waj .
What is the main advantage @didactic-drunk , performance?

[didactic-drunk]

Example 1: Privileged (re)placement of user files with races.

File.open "path/to/file", Create|Exclusive|SymNoFollow do |file|
  ...
end
File.chown "path/to/file", ...
File.chmod "path/to/file", ...

# In another process
mv path path.renamed
mkdir -p path/to
ln -s /etc/sudoers path/to/file
ln /home/not/my/file path/to/file # Or

Example 2: Privileged (re)placement of user files without races.

File.open "path/to/file", Create|Exclusive|SymNoFollow do |file|
  file.chown ...
  file.chmod ...
end

# No race conditions available except for the initial opening of "path/to/file"

It's also a little faster.

[dannote]

@didactic-drunk shouldn't you use File#flock_exclusive in this case? (Why isn't there a single method accepting a bit mask as in Ruby by the way?)

[didactic-drunk]

Flock only does advisory locking, doesn't stop bad actors, any program (mv) not using flock or directory renames.