Skip to content

Commit

Permalink
Fix closing of destination stream in get() calls
Browse files Browse the repository at this point in the history
When passing in a destination stream for a get() call and the
source file did not exist, the stream was not closed, leading
to potential file descriptor leak.
  • Loading branch information
theophilusx committed Aug 4, 2022
1 parent 681d06b commit 1d4bd73
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 11 deletions.
3 changes: 3 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,9 @@ class SftpClient {
};
rdr = this.sftp.createReadStream(rPath, opts.readStreamOptions);
rdr.once('error', (err) => {
if (dst && typeof dst !== 'string' && !dst.destroyed) {
dst.destroy();
}
reject(this.fmtError(`${err.message} ${rPath}`, '_get', err.code));
});
if (dst === undefined) {
Expand Down
34 changes: 23 additions & 11 deletions test/12get.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ describe('get() method tests', function () {
});

it('get returns a promise', function () {
return expect(sftp.get(config.sftpUrl + '/get-promise.txt')).to.be.a(
'promise'
);
return expect(sftp.get(config.sftpUrl + '/get-promise.txt')).to.be.a('promise');
});

it('get the file content', async function () {
Expand Down Expand Up @@ -82,9 +80,26 @@ describe('get() method tests', function () {
});

it('get non-existent file is rejected', function () {
return expect(
sftp.get(config.sftpUrl + '/file-not-exist.md')
).to.be.rejectedWith('No such file');
return expect(sftp.get(config.sftpUrl + '/file-not-exist.md')).to.be.rejectedWith(
'No such file'
);
});

it('get non-existent file closes stream', async function () {
const localPath = makeLocalPath(config.localUrl, 'get-a-file.txt');
const out = fs.createWriteStream(localPath, {
flags: 'w',
encoding: null,
});
try {
await sftp.get(`${config.sftpUrl}/file-not-exist.md`, out);
} catch (err) {
return expect(out.destroyed).to.be.true;
}
expect(sftp.get(`${config.sftpUrl}/file-not-exist.md`, out)).to.be.rejectedWith(
'No such file'
);
return expect(out.destroyed).to.be.true;
});

it('get with relative remote path 1', async function () {
Expand Down Expand Up @@ -116,8 +131,7 @@ describe('get() method tests', function () {
});

it('get with relative local path 4', async function () {
let localPath =
'../ssh2-sftp-client/test/testData/get-relative4-gzip.txt.gz';
let localPath = '../ssh2-sftp-client/test/testData/get-relative4-gzip.txt.gz';
let remotePath = config.sftpUrl + '/get-gzip.txt.gz';
await sftp.get(remotePath, localPath);
let stats = await sftp.stat(remotePath);
Expand All @@ -128,8 +142,6 @@ describe('get() method tests', function () {
it('get with no permission on destination dir', function () {
let localPath = makeLocalPath(config.localUrl, 'no-perm-dir', 'foo.txt');
let remotePath = `${config.sftpUrl}/get-gzip.txt.gz`;
return expect(sftp.get(remotePath, localPath)).to.be.rejectedWith(
/Bad path/
);
return expect(sftp.get(remotePath, localPath)).to.be.rejectedWith(/Bad path/);
});
});

0 comments on commit 1d4bd73

Please # to comment.