[CopyFilesOverSSH] Addressed code review points

microsoft · Jul 31, 2020 · 05c87c6 · 05c87c6
1 parent 6387df2
commit 05c87c6
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 112 deletions.
diff --git a/Tasks/CopyFilesOverSSHV0/ThirdPartyNotice.txt b/Tasks/CopyFilesOverSSHV0/ThirdPartyNotice.txt
@@ -24,7 +24,7 @@ This Azure DevOps extension (CopyFilesOverSSHV0) is based on or incorporates mat
 18.	path-is-absolute (git+https://github.com/sindresorhus/path-is-absolute.git)
 19.	q (git://github.com/kriskowal/q.git)
 20.	readable-stream (git://github.com/isaacs/readable-stream.git)
-21.	ssh2-sftp-client (git+https://https://github.com/theophilusx/ssh2-sftp-client.git)
+21.	ssh2-sftp-client (git+https://github.com/theophilusx/ssh2-sftp-client.git)
 22.	semver (git+https://github.com/npm/node-semver.git)
 23.	shelljs (git://github.com/arturadib/shelljs.git)
 24.	ssh2 (git+ssh://[email protected]/mscdex/ssh2.git)

diff --git a/Tasks/CopyFilesOverSSHV0/copyfilesoverssh.ts b/Tasks/CopyFilesOverSSHV0/copyfilesoverssh.ts
@@ -222,7 +222,7 @@ async function run() {
                     let targetPath = path.posix.join(targetFolder, relativePath);
 
                     if (!path.isAbsolute(targetPath)) {
-                        targetPath = './' + targetPath
+                        targetPath = `./${targetPath}`;
                     }
 
                     console.log(tl.loc('StartedFileCopy', fileToCopy, targetPath));

diff --git a/Tasks/CopyFilesOverSSHV0/package-lock.json b/Tasks/CopyFilesOverSSHV0/package-lock.json
diff --git a/Tasks/CopyFilesOverSSHV0/sshhelper.ts b/Tasks/CopyFilesOverSSHV0/sshhelper.ts
@@ -53,6 +53,19 @@ export class SshHelper {
         await defer.promise;
     }
 
+    /**
+     * Change path separator for Windows-based platforms
+     * See https://github.com/spmjs/node-scp2/blob/master/lib/client.js#L319
+     * 
+     * @param filePath 
+     */
+    private unixyPath(filePath) {
+        if (process.platform === 'win32') {
+            return filePath.replace(/\\/g, '/');
+        }
+        return filePath;
+    }
+
     /**
      * Sets up the SSH connection
      */
@@ -72,6 +85,9 @@ export class SshHelper {
     async closeConnection() {
         try {
             if (this.sftpClient) {
+                this.sftpClient.on('error', (err) => {
+                    tl.debug('sftpClient: Ignoring error diconnecting: ' + err);
+                }); // ignore logout errors; see: https://github.com/mscdex/node-imap/issues/695
                 await this.sftpClient.end();
                 this.sftpClient = null;
             }
@@ -98,9 +114,7 @@ export class SshHelper {
      * @returns {Promise<string>}
      */
     async uploadFile(sourceFile: string, dest: string) : Promise<string> {
-        if (process.platform === 'win32') {
-            dest = dest.replace(/\\/g, '/');
-        }
+        dest = this.unixyPath(dest);
 
         tl.debug('Upload ' + sourceFile + ' to ' + dest + ' on remote machine.');
 
@@ -142,7 +156,7 @@ export class SshHelper {
         if(!this.sftpClient) {
             defer.reject(tl.loc('ConnectionNotSetup'));
         }
-        if (await this.sftpClient.stat(path)) {
+        if (await this.sftpClient.exists(path)) {
             //path exists
             defer.resolve(true);
         } else {

diff --git a/Tasks/CopyFilesOverSSHV0/task.json b/Tasks/CopyFilesOverSSHV0/task.json
@@ -17,7 +17,7 @@
     "author": "Microsoft Corporation",
     "version": {
         "Major": 0,
-        "Minor": 173,
+        "Minor": 174,
         "Patch": 0
     },
     "demands": [],

diff --git a/Tasks/CopyFilesOverSSHV0/task.loc.json b/Tasks/CopyFilesOverSSHV0/task.loc.json
@@ -17,7 +17,7 @@
   "author": "Microsoft Corporation",
   "version": {
     "Major": 0,
-    "Minor": 173,
+    "Minor": 174,
     "Patch": 0
   },
   "demands": [],