SSH: Fix for Microsoft-provided sftp.exe

The sftp.exe shipped with Windows 10 does not emit a prompt to stdout
when invoked via QProcess and thus cannot be used to establish an SFTP
session. However, batch mode can be made to work, so we now use that in
our device tester.

Fixes: QTCREATORBUG-22471
Change-Id: Ia55e66c6f342720c9a0f0c1943f6d9d969842bad
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>

src/libs/ssh/sftptransfer.cpp

@@ -25,6 +25,7 @@
 
 #include "sftptransfer.h"
 
+#include "sshlogging_p.h"
 #include "sshprocess.h"
 #include "sshsettings.h"
 
@@ -48,7 +49,7 @@ struct SftpTransfer::SftpTransferPrivate
     Internal::FileTransferType transferType;
     FileTransferErrorHandling errorHandlingMode;
     QStringList connectionArgs;
-    QTemporaryFile batchFile;
+    QString batchFilePath;
 
     QStringList dirsToCreate() const
     {
@@ -88,6 +89,8 @@ struct SftpTransfer::SftpTransferPrivate
 
 SftpTransfer::~SftpTransfer()
 {
+    if (!d->batchFilePath.isEmpty() && !QFile::remove(d->batchFilePath))
+        qCWarning(Internal::sshLog) << "failed to remove batch file" << d->batchFilePath;
     delete d;
 }
 
@@ -137,15 +140,18 @@ void SftpTransfer::doStart()
         emitError(tr("sftp binary \"%1\" does not exist.").arg(sftpBinary.toUserOutput()));
         return;
     }
-    if (!d->batchFile.isOpen() && !d->batchFile.open()) {
-        emitError(tr("Could not create temporary file: %1").arg(d->batchFile.errorString()));
+    QTemporaryFile batchFile;
+    batchFile.setAutoRemove(false);
+    if (!batchFile.isOpen() && !batchFile.open()) {
+        emitError(tr("Could not create temporary file: %1").arg(batchFile.errorString()));
         return;
     }
-    d->batchFile.resize(0);
+    d->batchFilePath = batchFile.fileName();
+    batchFile.resize(0);
     for (const QString &dir : d->dirsToCreate()) {
         switch (d->transferType) {
         case Internal::FileTransferType::Upload:
-            d->batchFile.write("-mkdir " + QtcProcess::quoteArgUnix(dir).toLocal8Bit() + '\n');
+            batchFile.write("-mkdir " + QtcProcess::quoteArgUnix(dir).toLocal8Bit() + '\n');
             break;
         case Internal::FileTransferType::Download:
             if (!QDir::root().mkdir(dir)) {
@@ -163,20 +169,20 @@ void SftpTransfer::doStart()
             QFileInfo fi(f.sourceFile);
             if (fi.isSymLink()) {
                 link = true;
-                d->batchFile.write("-rm " + QtcProcess::quoteArgUnix(f.targetFile).toLocal8Bit()
+                batchFile.write("-rm " + QtcProcess::quoteArgUnix(f.targetFile).toLocal8Bit()
                                 + '\n');
                 sourceFileOrLinkTarget = fi.dir().relativeFilePath(fi.symLinkTarget()); // see QTBUG-5817.
             } else {
                 sourceFileOrLinkTarget = f.sourceFile;
             }
          }
-         d->batchFile.write(d->transferCommand(link) + ' '
+         batchFile.write(d->transferCommand(link) + ' '
                          + QtcProcess::quoteArgUnix(sourceFileOrLinkTarget).toLocal8Bit() + ' '
                          + QtcProcess::quoteArgUnix(f.targetFile).toLocal8Bit() + '\n');
     }
-    d->batchFile.flush();
     d->sftpProc.start(sftpBinary.toString(),
-                      QStringList{"-b", d->batchFile.fileName()} << d->connectionArgs);
+                      QStringList{"-b", QDir::toNativeSeparators(batchFile.fileName())}
+                          << d->connectionArgs);
     emit started();
 }
 

src/plugins/remotelinux/linuxdevicetester.cpp

@@ -31,7 +31,7 @@
 #include <projectexplorer/devicesupport/deviceusedportsgatherer.h>
 #include <utils/port.h>
 #include <utils/qtcassert.h>
-#include <ssh/sftpsession.h>
+#include <ssh/sftptransfer.h>
 #include <ssh/sshremoteprocess.h>
 #include <ssh/sshconnection.h>
 #include <ssh/sshconnectionmanager.h>
@@ -54,7 +54,7 @@ public:
     SshConnection *connection = nullptr;
     SshRemoteProcessPtr process;
     DeviceUsedPortsGatherer portsGatherer;
-    SftpSessionPtr sftpSession;
+    SftpTransferPtr sftpTransfer;
     SshProcess rsyncProcess;
     State state = Inactive;
     bool sftpWorks = false;
@@ -108,7 +108,7 @@ void GenericLinuxDeviceTester::stopTest()
         d->process->close();
         break;
     case TestingSftp:
-        d->sftpSession->quit();
+        d->sftpTransfer->stop();
         break;
     case TestingRsync:
         d->rsyncProcess.disconnect();
@@ -190,32 +190,30 @@ void GenericLinuxDeviceTester::handlePortListReady()
     }
 
     emit progressMessage(tr("Checking whether an SFTP connection can be set up..."));
-    d->sftpSession = d->connection->createSftpSession();
-    connect(d->sftpSession.get(), &SftpSession::started,
-            this, &GenericLinuxDeviceTester::handleSftpStarted);
-    connect(d->sftpSession.get(), &SftpSession::done,
+    d->sftpTransfer = d->connection->createDownload(FilesToTransfer(),
+                                                    FileTransferErrorHandling::Abort);
+    connect(d->sftpTransfer.get(), &SftpTransfer::done,
             this, &GenericLinuxDeviceTester::handleSftpFinished);
     d->state = TestingSftp;
-    d->sftpSession->start();
+    d->sftpTransfer->start();
 }
 
 void GenericLinuxDeviceTester::handleSftpStarted()
 {
     QTC_ASSERT(d->state == TestingSftp, return);
-    emit progressMessage(tr("SFTP service available.\n"));
-    d->sftpWorks = true;
-    disconnect(d->sftpSession.get(), nullptr, this, nullptr);
-    d->sftpSession->quit();
-    testRsync();
 }
 
 void GenericLinuxDeviceTester::handleSftpFinished(const QString &error)
 {
     QTC_ASSERT(d->state == TestingSftp, return);
-    QString theError;
-    theError = error.isEmpty() ? tr("sftp finished unexpectedly.") : error;
+    if (error.isEmpty()) {
+        d->sftpWorks = true;
+        emit progressMessage(tr("SFTP service available.\n"));
+    } else {
         d->sftpWorks = false;
-    emit errorMessage(tr("Error setting up SFTP connection: %1\n").arg(theError));
+        emit errorMessage(tr("Error setting up SFTP connection: %1\n").arg(error));
+    }
+    disconnect(d->sftpTransfer.get(), nullptr, this, nullptr);
     testRsync();
 }
 
@@ -271,9 +269,9 @@ void GenericLinuxDeviceTester::setFinished(TestResult result)
 {
     d->state = Inactive;
     disconnect(&d->portsGatherer, nullptr, this, nullptr);
-    if (d->sftpSession) {
-        disconnect(d->sftpSession.get(), nullptr, this, nullptr);
-        d->sftpSession.release()->deleteLater();
+    if (d->sftpTransfer) {
+        disconnect(d->sftpTransfer.get(), nullptr, this, nullptr);
+        d->sftpTransfer.release()->deleteLater();
     }
     if (d->connection) {
         disconnect(d->connection, nullptr, this, nullptr);