spmjs / node-scp2

[MAINTAINER WANTED] A pure javascript scp program based on ssh2.
385 stars 96 forks source link

Failed error check in download callback #18

Closed confuser closed 8 years ago

confuser commented 9 years ago

https://github.com/lepture/node-scp2/blob/master/lib/client.js#L281

Error is never checked, and instead assumes sftp will always be defined. In cases where an error has occurred, a TypeError: Cannot call method 'createReadStream' of undefined is thrown

KSoto commented 9 years ago

Yes! I'm having the same problem :/

I use "scp" and have a block that says something like "if scp returns an err, print it and callback", but the function keeps going instead of exiting and is causing node to crash.

If I give it a hostname that works, this code works great! But if the path doesn't exist, it fails with:

TypeError: Cannot read property 'createReadStream' of undefined
    at .../app/node_modules/scp2/lib/client.js:282:31

I'll insert more code:

        scp.scp({
            host: 'somehost',
            username: 'myusername',
            password: 'mypassword',
            path: 'somepath'
        }, './', function(err) {
            console.log(err);
            callback(err); //the code doesn't stop here, though!
        });

I was able to fix this by changing this code:

Client.prototype.download = function(src, dest, callback) {
  var self = this;

  self.sftp(function(err,sftp){
    var sftp_readStream = sftp.createReadStream(src);
    sftp_readStream.on('error', function(err){
      callback(err);
    });
    sftp_readStream.pipe(fs.createWriteStream(dest))
    .on('close',function(){
      self.emit('read', src);
      callback(null);
    })
    .on('error', function(err){
      callback(err);
    });
  });
};

to this:

Client.prototype.download = function(src, dest, callback) {
  var self = this;

  self.sftp(function(err,sftp){
  if(!err) //<---
  { //<---
    var sftp_readStream = sftp.createReadStream(src);
    sftp_readStream.on('error', function(err){
      callback(err);
    });
    sftp_readStream.pipe(fs.createWriteStream(dest))
    .on('close',function(){
      self.emit('read', src);
      callback(null);
    })
    .on('error', function(err){
      callback(err);
    });
  }else //<---
  { //<---
        callback(err); //<---
  } //<---
  });
};

I'll fork the repo and do a pull request