Repository navigation
fs.lchown should be undeprecated #19868
Description
- Version: v8.9.3
- Platform: Linux xxx 4.15.0-2-amd64 deps: update openssl to 1.0.1j #1 SMP Debian 4.15.11-1 (2018-03-20) x86_64 GNU/Linux
- Subsystem: fs
fs.lchown is currently deprecated
But fs.lchown can be helpful to address Time of check to time of use (TOCTOU) bugs, like this one,
as it does not dereference symbolic links and merely changes the owner of the link.
Please undeprecate fs.lchown and implement it on the linux platform.
Activity
ryzokuken commentedon Apr 7, 2018
@nodejs/fs thoughts on this?
@simevo you sure there's no alternative?
addaleax commentedon Apr 7, 2018
I’m surprised to hear it’s deprecated and only implemented on some platforms, tbh.
ryzokuken commentedon Apr 7, 2018
Strange. I'm equally surprised to hear that it's implemented on Windows and not on Linux. I mean, it's
chown. How do you even not implementchownon linux? 😛@addaleax that said, if we decide to go ahead and add this functionality, I'd love to take a shot at it.
anliting commentedon Apr 7, 2018
@ryzokuken
I don't think @simevo means it is implemented on Windows. As nodejs/node-v0.x-archive#7382, it is only implemented on macOS.
ryzokuken commentedon Apr 7, 2018
@anliting I just found that out myself, and needless to say, it makes a lot of sense.
Could we somehow implement this functionality on Windows too? I'd love if we could make this platform-independent somehow. Let me look into it.
bnoordhuis commentedon Apr 7, 2018
Related: #16695
There is no way to implement it with old Linux kernels:
fchmodat()was added in 2.6.16 and it was until recently that we still still supported 2.6.9.2.6.18 is the baseline now so that's no longer a blocker.
ryzokuken commentedon Apr 7, 2018
@bnoordhuis Sounds great!
Now that the baseline has moved, should someone start working on making it work on Linux? We'd probably have to un-
deprecatethe function before that though, wouldn't we?Also, does that mean it'd be
semver-major? Or would it besemver-minorbecause we're not really removing anything, just adding new functionality?bnoordhuis commentedon Apr 7, 2018
Yes. It should be added to libuv first.
No, the other way around. It was deprecated because it's platform-specific. It should be un-deprecated only when that's no longer true.
Yep, I'd say so.
ryzokuken commentedon Apr 7, 2018
Great! I think @chris--young had been working on adding it to libuv? If there's no open PR regarding this in there, I could try looking into it, or submit an issue. That way, we could go with whichever way helps us ship this quicker.
bnoordhuis commentedon Apr 7, 2018
I don't remember any libuv PRs. A quick search doesn't turn up anything either.
ryzokuken commentedon Apr 7, 2018
@bnoordhuis in that case, I'd make an issue on libuv and start looking into it myself as well.
simevo commentedon Jun 19, 2018
Hi please note that libuv now has lchown, see: libuv/libuv#1826 (comment)
deps: upgrade to libuv 1.21.0
fs: undeprecate lchown()
fs: undeprecate lchown()