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

fs.writeFile() removes execute permissions from file #9380

Closed
bpasero opened this issue Oct 31, 2016 · 6 comments
Closed

fs.writeFile() removes execute permissions from file #9380

bpasero opened this issue Oct 31, 2016 · 6 comments
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.

Comments

@bpasero
Copy link
Contributor

bpasero commented Oct 31, 2016

  • Version: 6.9.0
  • Platform: Windows 10, Mac OS 10.11.6
  • Subsystem: Parallels Desktop 11

Setup:

  • MacBook Pro running Parallels Desktop 11
  • Windows 10 as Parallels VM
  • Windows 10 is seeing the host file system as mounted network drive
  • Create a file executable.sh on Mac with +x bits
  • Make sure Windows 10 can access this file via the network share

Script:

var fs = require("fs");

fs.writeFileSync("Y:\\Desktop\\execute.sh", "loosing executable bits now!");

console.log("Done");

=> the file has lost its executable bit

If you change to pass in a r+ flag like so:

fs.writeFileSync("Y:\\Desktop\\execute.sh", "loosing executable bits now!", { flag: "r+"});

the executable bits are kept.

Now maybe someone can enlighten me here why fs.writeFile by default does not use r+ if it is better at keeping executable bits.

@thefourtheye thefourtheye added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. labels Oct 31, 2016
@sam-github
Copy link
Contributor

You've a lot of moving parts here:

  1. Windows, which doesn't support an executable bit
  2. Some kind of windows to OS X translation layer
  3. OS X

Can you repro it in a simpler setup?

Default for writeFileSync is to truncate target file with 'w', and when you specify 'r+', you are specifying read/write, so while 'r+' may keep the bits in your situation, it results in this, by design, which is even less what you want!

% node                                           
> fs.writeFileSync("f.txt", 'xxxxx')
undefined
> fs.readFileSync("f.txt", 'utf-8')
'xxxxx'
> fs.writeFileSync("f.txt", 'yyy', {flag: 'r+'})
undefined
> fs.readFileSync("f.txt", 'utf-8')
'yyyxx'

@chrmarti
Copy link

Interestingly the following has the desired behavior and preserves the executable flag:

const fs = require('fs');
const fd = fs.openSync('script.sh', 'r+');
fs.ftruncateSync(fd, 0);
fs.writeSync(fd, '#!/bin/bash\n\necho yay!\n');
fs.closeSync(fd);

You'd think the default 'w' would do just that, but it doesn't. Can we find out if that's due to the Windows to OS X translation layer or node?

@sam-github
Copy link
Contributor

From

node/lib/internal/fs.js

Lines 30 to 52 in 40366df

switch (flag) {
case 'r' : return O_RDONLY;
case 'rs' : // Fall through.
case 'sr' : return O_RDONLY | O_SYNC;
case 'r+' : return O_RDWR;
case 'rs+' : // Fall through.
case 'sr+' : return O_RDWR | O_SYNC;
case 'w' : return O_TRUNC | O_CREAT | O_WRONLY;
case 'wx' : // Fall through.
case 'xw' : return O_TRUNC | O_CREAT | O_WRONLY | O_EXCL;
case 'w+' : return O_TRUNC | O_CREAT | O_RDWR;
case 'wx+': // Fall through.
case 'xw+': return O_TRUNC | O_CREAT | O_RDWR | O_EXCL;
case 'a' : return O_APPEND | O_CREAT | O_WRONLY;
case 'ax' : // Fall through.
case 'xa' : return O_APPEND | O_CREAT | O_WRONLY | O_EXCL;
case 'a+' : return O_APPEND | O_CREAT | O_RDWR;
case 'ax+': // Fall through.
case 'xa+': return O_APPEND | O_CREAT | O_RDWR | O_EXCL;
, 'w' maps to O_TRUNC | O_CREAT | O_WRONLY, 'r+' to O_RDWR, what that maps to on Windows can be seen at

node/deps/uv/src/win/fs.c

Lines 403 to 453 in 40366df

switch (flags & (_O_RDONLY | _O_WRONLY | _O_RDWR)) {
case _O_RDONLY:
access = FILE_GENERIC_READ;
attributes |= FILE_FLAG_BACKUP_SEMANTICS;
break;
case _O_WRONLY:
access = FILE_GENERIC_WRITE;
break;
case _O_RDWR:
access = FILE_GENERIC_READ | FILE_GENERIC_WRITE;
break;
default:
goto einval;
}
if (flags & _O_APPEND) {
access &= ~FILE_WRITE_DATA;
access |= FILE_APPEND_DATA;
attributes &= ~FILE_FLAG_BACKUP_SEMANTICS;
}
/*
* Here is where we deviate significantly from what CRT's _open()
* does. We indiscriminately use all the sharing modes, to match
* UNIX semantics. In particular, this ensures that the file can
* be deleted even whilst it's open, fixing issue #1449.
*/
share = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
switch (flags & (_O_CREAT | _O_EXCL | _O_TRUNC)) {
case 0:
case _O_EXCL:
disposition = OPEN_EXISTING;
break;
case _O_CREAT:
disposition = OPEN_ALWAYS;
break;
case _O_CREAT | _O_EXCL:
case _O_CREAT | _O_TRUNC | _O_EXCL:
disposition = CREATE_NEW;
break;
case _O_TRUNC:
case _O_TRUNC | _O_EXCL:
disposition = TRUNCATE_EXISTING;
break;
case _O_CREAT | _O_TRUNC:
disposition = CREATE_ALWAYS;
break;
default:
goto einval;
}
, but what it means on windows I'm not familiar enough with CreateFile() to know.

That CREATE_ALWAYS though looks like something worth looking into.

@chrmarti
Copy link

'w' preserves at least some metadata (archive/compress/index flags, permissions) when running on Windows against a local NTFS filesystem. That suggests CREATE_ALWAYS might be the right choice and the translation to OS X is at fault, but I'm not an expert in this area.

@thefourtheye
Copy link
Contributor

cc @nodejs/fs

@bzoz
Copy link
Contributor

bzoz commented Jun 20, 2017

IMHO mapping w to CREATE_ALWAYS is perfectly correct. I don't see anything we can do in Node.js here.

@bzoz bzoz closed this as completed Jun 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

No branches or pull requests

5 participants