From 016ad4a9788fa3015849c8260600a832f8e60f17 Mon Sep 17 00:00:00 2001 From: Christian Kandeler Date: Mon, 30 Sep 2019 14:32:49 +0200 Subject: [PATCH] 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 --- src/libs/ssh/sftptransfer.cpp | 30 +++++++++------ src/plugins/remotelinux/linuxdevicetester.cpp | 38 +++++++++---------- 2 files changed, 36 insertions(+), 32 deletions(-) diff --git a/src/libs/ssh/sftptransfer.cpp b/src/libs/ssh/sftptransfer.cpp index 52b821bc392..853907eaa0d 100644 --- a/src/libs/ssh/sftptransfer.cpp +++ b/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() - + '\n'); + 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) + ' ' - + QtcProcess::quoteArgUnix(sourceFileOrLinkTarget).toLocal8Bit() + ' ' - + QtcProcess::quoteArgUnix(f.targetFile).toLocal8Bit() + '\n'); + 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(); } diff --git a/src/plugins/remotelinux/linuxdevicetester.cpp b/src/plugins/remotelinux/linuxdevicetester.cpp index bef26118c3b..a5327f8c5e4 100644 --- a/src/plugins/remotelinux/linuxdevicetester.cpp +++ b/src/plugins/remotelinux/linuxdevicetester.cpp @@ -31,7 +31,7 @@ #include #include #include -#include +#include #include #include #include @@ -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; - d->sftpWorks = false; - emit errorMessage(tr("Error setting up SFTP connection: %1\n").arg(theError)); + 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(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);