-
Notifications
You must be signed in to change notification settings - Fork 723
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
Copying fifos hangs. #727
base: master
Are you sure you want to change the base?
Copying fifos hangs. #727
Changes from 18 commits
e17d624
52c412f
e4a911d
eaaa40a
476d4b4
c71edc2
d469212
5689dc0
777fe04
8fa217b
18c513e
be93375
599a2c5
2d3af53
d942f3f
fad43aa
e6760bb
045c290
40518f7
c69ddc3
b2932cf
6a4a6a0
f7957bf
a4e5411
117894c
48a0198
0d70322
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
var fs = require('fs'); | ||
var path = require('path'); | ||
var common = require('./common'); | ||
var exec = require('./exec'); | ||
|
||
common.register('cp', _cp, { | ||
cmdOptions: { | ||
|
@@ -34,16 +35,33 @@ function copyFileSync(srcFile, destFile, options) { | |
// If we're here, destFile probably doesn't exist, so just do a normal copy | ||
} | ||
|
||
if (fs.lstatSync(srcFile).isSymbolicLink() && !options.followsymlink) { | ||
try { | ||
fs.lstatSync(destFile); | ||
common.unlinkSync(destFile); // re-link it | ||
} catch (e) { | ||
// it doesn't exist, so no work needs to be done | ||
} | ||
var srcFileStat = fs.lstatSync(srcFile); | ||
|
||
if (srcFileStat.isSymbolicLink()) { | ||
if (options.followsymlink) { | ||
// recurse to copy the target of the symlink to the dest to ensure we handle complex scenarios | ||
return copyFileSync(fs.realpathSync(srcFile), destFile, options); | ||
} else { | ||
try { | ||
fs.lstatSync(destFile); | ||
common.unlinkSync(destFile); // re-link it | ||
} catch (e) { | ||
// it doesn't exist, so no work needs to be done | ||
} | ||
|
||
var symlinkFull = fs.readlinkSync(srcFile); | ||
fs.symlinkSync(symlinkFull, destFile, isWindows ? 'junction' : null); | ||
var symlinkFull = fs.readlinkSync(srcFile); | ||
fs.symlinkSync(symlinkFull, destFile, isWindows ? 'junction' : null); | ||
} | ||
// recursive is a special case for non-files that means "create an equivalent at the dest" rather than reading and writing | ||
} else if (options.recursive && (srcFileStat.isFIFO() || srcFileStat.isCharacterDevice() || srcFileStat.isBlockDevice())) { | ||
if (mknod(destFile, srcFileStat)) { | ||
fs.chmodSync(destFile, srcFileStat.mode); | ||
try { | ||
fs.chownSync(destFile, srcFileStat.uid, srcFileStat.gid); | ||
} catch (e) { | ||
common.error('copyFileSync: could not set ownership for dest file (' + srcFile + ')'); | ||
} | ||
} | ||
} else { | ||
var buf = common.buffer(); | ||
var bufLength = buf.length; | ||
|
@@ -67,7 +85,7 @@ function copyFileSync(srcFile, destFile, options) { | |
} | ||
|
||
while (bytesRead === bufLength) { | ||
bytesRead = fs.readSync(fdr, buf, 0, bufLength, pos); | ||
bytesRead = fs.readSync(fdr, buf, 0, bufLength, pos || -1); | ||
fs.writeSync(fdw, buf, 0, bytesRead); | ||
pos += bytesRead; | ||
} | ||
|
@@ -153,6 +171,42 @@ function cpdirSyncRecursive(sourceDir, destDir, currentDepth, opts) { | |
} // for files | ||
} // cpdirSyncRecursive | ||
|
||
function major(rdev) { | ||
return ((rdev >> 31 >> 1) & 0xfffff000) | ((rdev >> 8) & 0x00000fff); | ||
} | ||
function minor(rdev) { | ||
return ((rdev >> 12) & 0xffffff00) | (rdev & 0x000000ff); | ||
} | ||
|
||
// attempts to create a dev (character or block) by shelling out to mknod. | ||
function mknod(destFile, stat) { | ||
var created; | ||
var failureMessage; | ||
var args = ['mknod']; | ||
args.push('"' + destFile + '"'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At first glance this looks wrong. What if $ mknod "hello world"; rm -rf *; echo "" p
|
||
if (stat.isFIFO()) { | ||
args.push('p'); | ||
} else { | ||
if (stat.isCharacterDevice()) args.push('c'); | ||
if (stat.isBlockDevice()) args.push('b'); | ||
args.push(major(stat.rdev), minor(stat.rdev)); | ||
} | ||
try { | ||
var cmd = exec(args.join(' '), { silent: true }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For security, I recommend try {
child_process.execFileSync('mknod', argsArray);
} catch (e) {
failureMessage = e.stderr;
} |
||
if (cmd.code === 0) { | ||
created = true; | ||
} else { | ||
failureMessage = cmd.stderr; | ||
} | ||
} catch (e) { | ||
failureMessage = 'cannot create special file (' + destFile + '): ' + e.message; | ||
} | ||
if (!created) { | ||
common.error('copyFileSync: ' + failureMessage); | ||
} | ||
return created; | ||
} | ||
|
||
// Checks if cureent file was created recently | ||
function checkRecentCreated(sources, index) { | ||
var lookedSource = sources[index]; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ import utils from './utils/utils'; | |
|
||
const oldMaxDepth = shell.config.maxdepth; | ||
const CWD = process.cwd(); | ||
const isRoot = process.getuid && process.getuid() === 0; | ||
|
||
test.beforeEach(t => { | ||
t.context.tmp = utils.getTempDir(); | ||
|
@@ -755,3 +756,111 @@ test('should not overwrite recently created files (not give error no-force mode) | |
// Ensure First file is copied | ||
t.is(shell.cat(`${t.context.tmp}/file1`).toString(), 'test1'); | ||
}); | ||
|
||
test('should copy fifos contents', t => { | ||
if (process.platform !== 'win32') { // fs.exists doesn't support fifos on windows | ||
shell.exec(`mkfifo ${t.context.tmp}/fifo`); | ||
shell.exec(`printf test1 > ${t.context.tmp}/fifo`, { async: true }); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/fifo`)); | ||
const result = shell.cp(`${t.context.tmp}/fifo`, `${t.context.tmp}/newFifo`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.is(shell.cat(`${t.context.tmp}/newFifo`).toString(), 'test1'); | ||
t.falsy(fs.statSync(`${t.context.tmp}/newFifo`).isFIFO()); | ||
} | ||
}); | ||
|
||
test('should create an equivalent fifo when copying fifos when set to recursive mode', t => { | ||
if (process.platform !== 'win32') { // fs.exists doesn't support fifos on windows | ||
shell.exec(`mkfifo ${t.context.tmp}/fifo`); | ||
shell.exec(`printf test1 > ${t.context.tmp}/fifo`, { async: true }); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/fifo`)); | ||
const result = shell.cp('-r', `${t.context.tmp}/fifo`, `${t.context.tmp}/newFifo`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newFifo`).isFIFO()); | ||
} | ||
}); | ||
|
||
test('should copy fifos contents via symlinks', t => { | ||
if (process.platform !== 'win32') { // fs.exists doesn't support fifos on windows | ||
shell.exec(`mkfifo ${t.context.tmp}/fifo`); | ||
shell.exec(`printf test1 > ${t.context.tmp}/fifo`, { async: true }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Iirc, osx's echo doesn't support the Also worth noting named pipes aren't supported directly by Windows (only via cygwin etc, which didn't appear reliable) (Could match the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah :-) good point :-) Is it possible to easily shell out to shelljs's echo implementation? (I've shelled out here because the event loop is blocked by the cp, so need a way to get data into the fifo in the background; and atm this is shelling out to the /bin/sh impl (iirc?) and using it's echo/printf (although maybe there's a better way?)). |
||
shell.pushd(t.context.tmp); | ||
fs.symlinkSync('fifo', 'symFifo'); | ||
shell.popd(); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/symFifo`)); | ||
const result = shell.cp(`${t.context.tmp}/symFifo`, `${t.context.tmp}/newFifo`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.is(shell.cat(`${t.context.tmp}/newFifo`).toString(), 'test1'); | ||
t.falsy(fs.statSync(`${t.context.tmp}/newFifo`).isFIFO()); | ||
} | ||
}); | ||
|
||
test('should create an equivalent fifo when copying fifos when set to recursive mode via symlinks', t => { | ||
if (process.platform !== 'win32') { // fs.exists doesn't support fifos on windows | ||
shell.exec(`mkfifo ${t.context.tmp}/fifo`); | ||
shell.exec(`printf test1 > ${t.context.tmp}/fifo`, { async: true }); | ||
shell.pushd(t.context.tmp); | ||
fs.symlinkSync('fifo', 'symFifo'); | ||
shell.popd(); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/symFifo`)); | ||
const result = shell.cp('-rL', `${t.context.tmp}/symFifo`, `${t.context.tmp}/newFifo`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newFifo`).isFIFO()); | ||
} | ||
}); | ||
|
||
test('should copy character devices when set to recursive mode', t => { | ||
if (process.platform !== 'win32' && isRoot) { | ||
shell.exec(`mknod ${t.context.tmp}/zero c 1 5`); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/zero`)); | ||
const result = shell.cp('-r', `${t.context.tmp}/zero`, `${t.context.tmp}/newZero`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newZero`).isCharacterDevice()); | ||
} | ||
}); | ||
|
||
test('should set the mode when copying character devices', t => { | ||
if (process.platform !== 'win32' && isRoot) { | ||
shell.exec(`mknod ${t.context.tmp}/zero c 1 5`); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/zero`)); | ||
const originalMode = fs.statSync(`${t.context.tmp}/zero`).mode; | ||
fs.chmodSync(`${t.context.tmp}/zero`, parseInt('777', 8)); | ||
t.not(fs.statSync(`${t.context.tmp}/zero`).mode, originalMode); | ||
const result = shell.cp('-r', `${t.context.tmp}/zero`, `${t.context.tmp}/newZero`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newZero`).isCharacterDevice()); | ||
t.is(fs.statSync(`${t.context.tmp}/zero`).mode, fs.statSync(`${t.context.tmp}/newZero`).mode); | ||
} | ||
}); | ||
|
||
test('should copy block devices when set to recursive mode', t => { | ||
if (process.platform !== 'win32' && isRoot) { | ||
shell.exec(`mknod ${t.context.tmp}/sda1 b 8 1`); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/sda1`)); | ||
const result = shell.cp('-r', `${t.context.tmp}/sda1`, `${t.context.tmp}/newSda1`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newSda1`).isBlockDevice()); | ||
} | ||
}); | ||
|
||
test('should set the mode when copying block devices', t => { | ||
if (process.platform !== 'win32' && isRoot) { | ||
shell.exec(`mknod ${t.context.tmp}/sda1 b 8 1`); | ||
t.truthy(fs.existsSync(`${t.context.tmp}/sda1`)); | ||
const originalMode = fs.statSync(`${t.context.tmp}/sda1`).mode; | ||
fs.chmodSync(`${t.context.tmp}/sda1`, parseInt('777', 8)); | ||
t.not(fs.statSync(`${t.context.tmp}/sda1`).mode, originalMode); | ||
const result = shell.cp('-r', `${t.context.tmp}/sda1`, `${t.context.tmp}/newSda1`); | ||
t.falsy(shell.error()); | ||
t.is(result.code, 0); | ||
t.truthy(fs.statSync(`${t.context.tmp}/newSda1`).isBlockDevice()); | ||
t.is(fs.statSync(`${t.context.tmp}/sda1`).mode, fs.statSync(`${t.context.tmp}/newSda1`).mode); | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An explanatory comment might be good, since this is pretty niche and not very obvious