diff --git a/autotests/plugins/cli7zplugin/cli7ztest.h b/autotests/plugins/cli7zplugin/cli7ztest.h --- a/autotests/plugins/cli7zplugin/cli7ztest.h +++ b/autotests/plugins/cli7zplugin/cli7ztest.h @@ -41,6 +41,8 @@ void testArchive(); void testList_data(); void testList(); + void testListArgs_data(); + void testListArgs(); void testExtractArgs_data(); void testExtractArgs(); diff --git a/autotests/plugins/cli7zplugin/cli7ztest.cpp b/autotests/plugins/cli7zplugin/cli7ztest.cpp --- a/autotests/plugins/cli7zplugin/cli7ztest.cpp +++ b/autotests/plugins/cli7zplugin/cli7ztest.cpp @@ -178,6 +178,52 @@ plugin->deleteLater(); } +void Cli7zTest::testListArgs_data() +{ + QTest::addColumn("archiveName"); + QTest::addColumn("password"); + QTest::addColumn("expectedArgs"); + + QTest::newRow("unencrypted") + << QStringLiteral("/tmp/foo.7z") + << QString() + << QStringList { + QStringLiteral("l"), + QStringLiteral("-slt"), + QStringLiteral("/tmp/foo.7z") + }; + + QTest::newRow("header-encrypted") + << QStringLiteral("/tmp/foo.7z") + << QStringLiteral("1234") + << QStringList { + QStringLiteral("l"), + QStringLiteral("-slt"), + QStringLiteral("-p1234"), + QStringLiteral("/tmp/foo.7z") + }; +} + +void Cli7zTest::testListArgs() +{ + QFETCH(QString, archiveName); + CliPlugin *plugin = new CliPlugin(this, {QVariant(archiveName)}); + QVERIFY(plugin); + + const QStringList listArgs = { QStringLiteral("l"), + QStringLiteral("-slt"), + QStringLiteral("$PasswordSwitch"), + QStringLiteral("$Archive") }; + + QFETCH(QString, password); + const auto replacedArgs = plugin->substituteListVariables(listArgs, password); + + QFETCH(QStringList, expectedArgs); + QCOMPARE(replacedArgs, expectedArgs); + + plugin->deleteLater(); +} + void Cli7zTest::testExtractArgs_data() { QTest::addColumn("archiveName"); diff --git a/autotests/plugins/clirarplugin/clirartest.h b/autotests/plugins/clirarplugin/clirartest.h --- a/autotests/plugins/clirarplugin/clirartest.h +++ b/autotests/plugins/clirarplugin/clirartest.h @@ -42,6 +42,8 @@ void testArchive(); void testList_data(); void testList(); + void testListArgs_data(); + void testListArgs(); void testExtractArgs_data(); void testExtractArgs(); diff --git a/autotests/plugins/clirarplugin/clirartest.cpp b/autotests/plugins/clirarplugin/clirartest.cpp --- a/autotests/plugins/clirarplugin/clirartest.cpp +++ b/autotests/plugins/clirarplugin/clirartest.cpp @@ -212,6 +212,52 @@ rarPlugin->deleteLater(); } +void CliRarTest::testListArgs_data() +{ + QTest::addColumn("archiveName"); + QTest::addColumn("password"); + QTest::addColumn("expectedArgs"); + + QTest::newRow("unencrypted") + << QStringLiteral("/tmp/foo.rar") + << QString() + << QStringList { + QStringLiteral("vt"), + QStringLiteral("-v"), + QStringLiteral("/tmp/foo.rar") + }; + + QTest::newRow("header-encrypted") + << QStringLiteral("/tmp/foo.rar") + << QStringLiteral("1234") + << QStringList { + QStringLiteral("vt"), + QStringLiteral("-v"), + QStringLiteral("-p1234"), + QStringLiteral("/tmp/foo.rar") + }; +} + +void CliRarTest::testListArgs() +{ + QFETCH(QString, archiveName); + CliPlugin *plugin = new CliPlugin(this, {QVariant(archiveName)}); + QVERIFY(plugin); + + const QStringList listArgs = { QStringLiteral("vt"), + QStringLiteral("-v"), + QStringLiteral("$PasswordSwitch"), + QStringLiteral("$Archive") }; + + QFETCH(QString, password); + const auto replacedArgs = plugin->substituteListVariables(listArgs, password); + + QFETCH(QStringList, expectedArgs); + QCOMPARE(replacedArgs, expectedArgs); + + plugin->deleteLater(); +} + void CliRarTest::testExtractArgs_data() { QTest::addColumn("archiveName"); diff --git a/autotests/plugins/cliunarchiverplugin/cliunarchivertest.h b/autotests/plugins/cliunarchiverplugin/cliunarchivertest.h --- a/autotests/plugins/cliunarchiverplugin/cliunarchivertest.h +++ b/autotests/plugins/cliunarchiverplugin/cliunarchivertest.h @@ -36,6 +36,8 @@ void testArchive(); void testList_data(); void testList(); + void testListArgs_data(); + void testListArgs(); void testExtraction_data(); void testExtraction(); void testExtractArgs_data(); diff --git a/autotests/plugins/cliunarchiverplugin/cliunarchivertest.cpp b/autotests/plugins/cliunarchiverplugin/cliunarchivertest.cpp --- a/autotests/plugins/cliunarchiverplugin/cliunarchivertest.cpp +++ b/autotests/plugins/cliunarchiverplugin/cliunarchivertest.cpp @@ -173,6 +173,50 @@ unarPlugin->deleteLater(); } +void CliUnarchiverTest::testListArgs_data() +{ + QTest::addColumn("archiveName"); + QTest::addColumn("password"); + QTest::addColumn("expectedArgs"); + + QTest::newRow("unencrypted") + << QStringLiteral("/tmp/foo.rar") + << QString() + << QStringList { + QStringLiteral("-json"), + QStringLiteral("/tmp/foo.rar") + }; + + QTest::newRow("header-encrypted") + << QStringLiteral("/tmp/foo.rar") + << QStringLiteral("1234") + << QStringList { + QStringLiteral("-json"), + QStringLiteral("/tmp/foo.rar"), + QStringLiteral("-password"), + QStringLiteral("1234") + }; +} + +void CliUnarchiverTest::testListArgs() +{ + QFETCH(QString, archiveName); + CliPlugin *plugin = new CliPlugin(this, {QVariant(archiveName)}); + QVERIFY(plugin); + + const QStringList listArgs = { QStringLiteral("-json"), + QStringLiteral("$Archive"), + QStringLiteral("$PasswordSwitch") }; + + QFETCH(QString, password); + const auto replacedArgs = plugin->substituteListVariables(listArgs, password); + + QFETCH(QStringList, expectedArgs); + QCOMPARE(replacedArgs, expectedArgs); + + plugin->deleteLater(); +} + void CliUnarchiverTest::testExtraction_data() { QTest::addColumn("archivePath"); diff --git a/kerfuffle/cliinterface.h b/kerfuffle/cliinterface.h --- a/kerfuffle/cliinterface.h +++ b/kerfuffle/cliinterface.h @@ -317,6 +317,7 @@ */ bool moveToDestination(const QDir &tempDir, const QDir &destDir, bool preservePaths); + QStringList substituteListVariables(const QStringList &listArgs, const QString &password); QStringList substituteCopyVariables(const QStringList &extractArgs, const QVariantList &files, bool preservePaths, const QString &password, const QString &rootNode); /** @@ -343,7 +344,6 @@ virtual void handleLine(const QString& line); virtual void cacheParameterList(); - void substituteListVariables(QStringList& params); /** * Run @p programName with the given @p arguments. diff --git a/kerfuffle/cliinterface.cpp b/kerfuffle/cliinterface.cpp --- a/kerfuffle/cliinterface.cpp +++ b/kerfuffle/cliinterface.cpp @@ -127,8 +127,7 @@ cacheParameterList(); m_operationMode = List; - QStringList args = m_param.value(ListArgs).toStringList(); - substituteListVariables(args); + const auto args = substituteListVariables(m_param.value(ListArgs).toStringList(), password()); if (!runProcess(m_param.value(ListProgram).toStringList(), args)) { failOperation(); @@ -639,6 +638,33 @@ return true; } +QStringList CliInterface::substituteListVariables(const QStringList &listArgs, const QString &password) +{ + // Required if we call this function from unit tests. + cacheParameterList(); + + QStringList args; + foreach (const QString& arg, listArgs) { + if (arg == QLatin1String("$Archive")) { + args << filename(); + continue; + } + + if (arg == QLatin1String("$PasswordSwitch")) { + args << passwordSwitch(password); + continue; + } + + // Simple argument (e.g. -slt in 7z), nothing to substitute, just add it to the list. + args << arg; + } + + // Remove empty strings, if any. + args.removeAll(QString()); + + return args; +} + QStringList CliInterface::substituteCopyVariables(const QStringList &extractArgs, const QVariantList &files, bool preservePaths, const QString &password, const QString &rootNode) { // Required if we call this function from unit tests. @@ -659,7 +685,6 @@ } if (arg == QLatin1String("$PasswordSwitch")) { - args << passwordSwitch(password); continue; } @@ -1093,45 +1118,6 @@ return false; } -void CliInterface::substituteListVariables(QStringList& params) -{ - for (int i = 0; i < params.size(); ++i) { - const QString parameter = params.at(i); - - if (parameter == QLatin1String( "$Archive" )) { - params[i] = filename(); - } - - if (parameter == QLatin1String("$PasswordSwitch")) { - //if the PasswordSwitch argument has been added, we at least - //assume that the format of the switch has been added as well - Q_ASSERT(m_param.contains(PasswordSwitch)); - - //we will set it afterwards, if there is a password - params.removeAt(i); - - QString pass = password(); - - if (!pass.isEmpty()) { - QStringList theSwitch = m_param.value(PasswordSwitch).toStringList(); - - for (int j = 0; j < theSwitch.size(); ++j) { - //get the argument part - QString newArg = theSwitch.at(j); - - //substitute the $Password - newArg.replace(QLatin1String("$Password"), pass); - - //put it in the arg list - params.insert(i + j, newArg); - ++i; - } - } - --i; - } - } -} - QString CliInterface::escapeFileName(const QString& fileName) const { return fileName; diff --git a/plugins/cliunarchiverplugin/cliplugin.cpp b/plugins/cliunarchiverplugin/cliplugin.cpp --- a/plugins/cliunarchiverplugin/cliplugin.cpp +++ b/plugins/cliunarchiverplugin/cliplugin.cpp @@ -50,8 +50,7 @@ cacheParameterList(); m_operationMode = List; - QStringList args = m_param.value(ListArgs).toStringList(); - substituteListVariables(args); + const auto args = substituteListVariables(m_param.value(ListArgs).toStringList(), password()); if (!runProcess(m_param.value(ListProgram).toStringList(), args)) { failOperation();