summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRagnar Thomsen <rthomsen6@gmail.com>2016-10-09 16:33:39 (GMT)
committerRagnar Thomsen <rthomsen6@gmail.com>2016-10-09 16:33:39 (GMT)
commita3921c6ceced49499ac024d0ae67563275eaadb6 (patch)
treeaac8ee71c6057e3970ea2b41a48e2d5b14bb1163
parente399dc70f017345a5d4dba3b1b57a379507b3246 (diff)
clirar: Detect error when unrar version is too old
Unrar 3 and 4 cannot open RAR5 archives. Currently, when opening a RAR5 archive with these old versions there is an uncaught error message which ends up in the comment widget. This commit adds a check for these error messages in clirar and emits an error signal with a proper message. CliInterface was changed so that handleLine() passes on the boolean return value of readListLine() to readStdOut(). This way, we can stop readStdOut() from handling further lines when readListLine() returns false. This was needed to avoid multiple error signals being emitted. Previously, the return value of readListLine() was unused. Unittests were added for both unrar 3 and 4. Differential Revision: D2993
-rw-r--r--autotests/plugins/clirarplugin/clirartest.cpp59
-rw-r--r--autotests/plugins/clirarplugin/data/archive-RARv5-unrar3.txt4
-rw-r--r--autotests/plugins/clirarplugin/data/archive-RARv5-unrar4.txt5
-rw-r--r--kerfuffle/cliinterface.cpp3
-rw-r--r--plugins/clirarplugin/cliplugin.cpp67
-rw-r--r--plugins/clirarplugin/cliplugin.h5
6 files changed, 96 insertions, 47 deletions
diff --git a/autotests/plugins/clirarplugin/clirartest.cpp b/autotests/plugins/clirarplugin/clirartest.cpp
index 945cd7e..df358b7 100644
--- a/autotests/plugins/clirarplugin/clirartest.cpp
+++ b/autotests/plugins/clirarplugin/clirartest.cpp
@@ -102,6 +102,7 @@ void CliRarTest::testArchive()
void CliRarTest::testList_data()
{
QTest::addColumn<QString>("outputTextFile");
+ QTest::addColumn<QString>("errorMessage");
QTest::addColumn<int>("expectedEntriesCount");
QTest::addColumn<bool>("isMultiVolume");
// Is zero for non-multi-volume archives:
@@ -120,57 +121,69 @@ void CliRarTest::testList_data()
// Unrar 5 tests
QTest::newRow("normal-file-unrar5")
- << QFINDTESTDATA("data/archive-with-symlink-unrar5.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-with-symlink-unrar5.txt") << QString() << 8 << false << 0
<< 2 << QStringLiteral("rartest/file2.txt") << false << false << QString() << (qulonglong) 14 << (qulonglong) 23 << QStringLiteral("2016-03-21T08:57:36");
QTest::newRow("symlink-unrar5")
- << QFINDTESTDATA("data/archive-with-symlink-unrar5.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-with-symlink-unrar5.txt") << QString() << 8 << false << 0
<< 3 << QStringLiteral("rartest/linktofile1.txt") << false << false << QStringLiteral("file1.txt") << (qulonglong) 9 << (qulonglong) 9 << QStringLiteral("2016-03-21T08:58:16");
QTest::newRow("encrypted-unrar5")
- << QFINDTESTDATA("data/archive-encrypted-unrar5.txt") << 7 << false << 0
+ << QFINDTESTDATA("data/archive-encrypted-unrar5.txt") << QString() << 7 << false << 0
<< 2 << QStringLiteral("rartest/file2.txt") << false << true << QString() << (qulonglong) 14 << (qulonglong) 32 << QStringLiteral("2016-03-21T17:03:36");
QTest::newRow("recovery-record-unrar5")
- << QFINDTESTDATA("data/archive-recovery-record-unrar5.txt") << 3 << false << 0
+ << QFINDTESTDATA("data/archive-recovery-record-unrar5.txt") << QString() << 3 << false << 0
<< 0 << QStringLiteral("file1.txt") << false << false << QString() << (qulonglong) 32 << (qulonglong) 33 << QStringLiteral("2015-07-26T19:04:38");
QTest::newRow("corrupt-archive-unrar5")
- << QFINDTESTDATA("data/archive-corrupt-file-header-unrar5.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-corrupt-file-header-unrar5.txt") << QString() << 8 << false << 0
<< 6 << QStringLiteral("dir1/") << true << false << QString() << (qulonglong) 0 << (qulonglong) 0 << QStringLiteral("2015-05-14T01:45:24");
//Note: The number of entries will be the total number of all entries in all volumes, i.e. if a file spans 3 volumes it will count as 3 entries.
QTest::newRow("multivolume-archive-unrar5")
- << QFINDTESTDATA("data/archive-multivol-unrar5.txt") << 6 << true << 5
+ << QFINDTESTDATA("data/archive-multivol-unrar5.txt") << QString() << 6 << true << 5
<< 5 << QStringLiteral("largefile2") << false << false << QString() << (qulonglong) 2097152 << (qulonglong) 11231 << QStringLiteral("2016-07-17T11:26:19");
// Unrar 4 tests
QTest::newRow("normal-file-unrar4")
- << QFINDTESTDATA("data/archive-with-symlink-unrar4.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-with-symlink-unrar4.txt") << QString() << 8 << false << 0
<< 2 << QStringLiteral("rartest/file2.txt") << false << false << QString() << (qulonglong) 14 << (qulonglong) 23 << QStringLiteral("2016-03-21T08:57:00");
QTest::newRow("symlink-unrar4")
- << QFINDTESTDATA("data/archive-with-symlink-unrar4.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-with-symlink-unrar4.txt") << QString() << 8 << false << 0
<< 3 << QStringLiteral("rartest/linktofile1.txt") << false << false << QStringLiteral("file1.txt") << (qulonglong) 9 << (qulonglong) 9 << QStringLiteral("2016-03-21T08:58:00");
QTest::newRow("encrypted-unrar4")
- << QFINDTESTDATA("data/archive-encrypted-unrar4.txt") << 7 << false << 0
+ << QFINDTESTDATA("data/archive-encrypted-unrar4.txt") << QString() << 7 << false << 0
<< 2 << QStringLiteral("rartest/file2.txt") << false << true << QString() << (qulonglong) 14 << (qulonglong) 32 << QStringLiteral("2016-03-21T17:03:00");
QTest::newRow("recovery-record-unrar4")
- << QFINDTESTDATA("data/archive-recovery-record-unrar4.txt") << 3 << false << 0
+ << QFINDTESTDATA("data/archive-recovery-record-unrar4.txt") << QString() << 3 << false << 0
<< 0 << QStringLiteral("file1.txt") << false << false << QString() << (qulonglong) 32 << (qulonglong) 33 << QStringLiteral("2015-07-26T19:04:00");
QTest::newRow("corrupt-archive-unrar4")
- << QFINDTESTDATA("data/archive-corrupt-file-header-unrar4.txt") << 8 << false << 0
+ << QFINDTESTDATA("data/archive-corrupt-file-header-unrar4.txt") << QString() << 8 << false << 0
<< 6 << QStringLiteral("dir1/") << true << false << QString() << (qulonglong) 0 << (qulonglong) 0 << QStringLiteral("2015-05-14T01:45:00");
+ QTest::newRow("RAR5-open-with-unrar4")
+ << QFINDTESTDATA("data/archive-RARv5-unrar4.txt")
+ << QStringLiteral("Your unrar executable is version 4.20, which is too old to handle this archive. Please update to a more recent version.")
+ << 0 << false << 0 << 0 << QString() << true << false << QString() << (qulonglong) 0 << (qulonglong) 0 << QString();
+
//Note: The number of entries will be the total number of all entries in all volumes, i.e. if a file spans 3 volumes it will count as 3 entries.
QTest::newRow("multivolume-archive-unrar4")
- << QFINDTESTDATA("data/archive-multivol-unrar4.txt") << 6 << true << 5
+ << QFINDTESTDATA("data/archive-multivol-unrar4.txt") << QString() << 6 << true << 5
<< 5 << QStringLiteral("largefile2") << false << false << QString() << (qulonglong) 2097152 << (qulonglong) 11231 << QStringLiteral("2016-07-17T11:26:00");
+ // Unrar 3 tests
+
+ QTest::newRow("RAR5-open-with-unrar3")
+ << QFINDTESTDATA("data/archive-RARv5-unrar3.txt")
+ << QStringLiteral("Unrar reported a non-RAR archive. The installed unrar version (3.71) is old. Try updating your unrar.")
+ << 0 << false << 0 << 0 << QString() << true << false << QString() << (qulonglong) 0 << (qulonglong) 0 << QString();
+
/*
* Check that the plugin will not crash when reading corrupted archives, which
* have lines such as "Unexpected end of archive" or "??? - the file header is
@@ -179,7 +192,7 @@ void CliRarTest::testList_data()
* See bug 262857 and commit 2042997013432cdc6974f5b26d39893a21e21011.
*/
QTest::newRow("corrupt-archive-unrar3")
- << QFINDTESTDATA("data/archive-corrupt-file-header-unrar3.txt") << 1 << true << 1
+ << QFINDTESTDATA("data/archive-corrupt-file-header-unrar3.txt") << QString() << 1 << true << 1
<< 0 << QStringLiteral("some-file.ext") << false << false << QString() << (qulonglong) 732522496 << (qulonglong) 14851208 << QStringLiteral("2010-10-29T20:47:00");
}
@@ -187,7 +200,8 @@ void CliRarTest::testList()
{
qRegisterMetaType<Archive::Entry*>("Archive::Entry*");
CliPlugin *rarPlugin = new CliPlugin(this, {QStringLiteral("dummy.rar")});
- QSignalSpy signalSpy(rarPlugin, &CliPlugin::entry);
+ QSignalSpy signalSpyEntry(rarPlugin, &CliPlugin::entry);
+ QSignalSpy signalSpyError(rarPlugin, &CliPlugin::error);
QFETCH(QString, outputTextFile);
QFETCH(int, expectedEntriesCount);
@@ -198,10 +212,19 @@ void CliRarTest::testList()
QTextStream outputStream(&outputText);
while (!outputStream.atEnd()) {
const QString line(outputStream.readLine());
- QVERIFY(rarPlugin->readListLine(line));
+ if (!rarPlugin->readListLine(line)) {
+ break;
+ }
+ }
+
+ QFETCH(QString, errorMessage);
+ if (!errorMessage.isEmpty()) {
+ QCOMPARE(signalSpyError.count(), 1);
+ QCOMPARE(signalSpyError.at(0).at(0).toString(), errorMessage);
+ return;
}
- QCOMPARE(signalSpy.count(), expectedEntriesCount);
+ QCOMPARE(signalSpyEntry.count(), expectedEntriesCount);
QFETCH(bool, isMultiVolume);
QCOMPARE(rarPlugin->isMultiVolume(), isMultiVolume);
@@ -210,8 +233,8 @@ void CliRarTest::testList()
QCOMPARE(rarPlugin->numberOfVolumes(), numberOfVolumes);
QFETCH(int, someEntryIndex);
- QVERIFY(someEntryIndex < signalSpy.count());
- Archive::Entry *entry = signalSpy.at(someEntryIndex).at(0).value<Archive::Entry*>();
+ QVERIFY(someEntryIndex < signalSpyEntry.count());
+ Archive::Entry *entry = signalSpyEntry.at(someEntryIndex).at(0).value<Archive::Entry*>();
QFETCH(QString, expectedName);
QCOMPARE(entry->fullPath(), expectedName);
diff --git a/autotests/plugins/clirarplugin/data/archive-RARv5-unrar3.txt b/autotests/plugins/clirarplugin/data/archive-RARv5-unrar3.txt
new file mode 100644
index 0000000..331be31
--- /dev/null
+++ b/autotests/plugins/clirarplugin/data/archive-RARv5-unrar3.txt
@@ -0,0 +1,4 @@
+
+UNRAR 3.71 beta 1 freeware Copyright (c) 1993-2007 Alexander Roshal
+
+testv5.rar is not RAR archive
diff --git a/autotests/plugins/clirarplugin/data/archive-RARv5-unrar4.txt b/autotests/plugins/clirarplugin/data/archive-RARv5-unrar4.txt
new file mode 100644
index 0000000..76d7f40
--- /dev/null
+++ b/autotests/plugins/clirarplugin/data/archive-RARv5-unrar4.txt
@@ -0,0 +1,5 @@
+
+UNRAR 4.20 freeware Copyright (c) 1993-2012 Alexander Roshal
+
+Unsupported archive format. Please update RAR to a newer version.
+testv5.rar is not RAR archive
diff --git a/kerfuffle/cliinterface.cpp b/kerfuffle/cliinterface.cpp
index 4793a54..59d6068 100644
--- a/kerfuffle/cliinterface.cpp
+++ b/kerfuffle/cliinterface.cpp
@@ -1313,8 +1313,7 @@ bool CliInterface::handleLine(const QString& line)
return true;
}
- readListLine(line);
- return true;
+ return readListLine(line);
}
if (m_operationMode == Test) {
diff --git a/plugins/clirarplugin/cliplugin.cpp b/plugins/clirarplugin/cliplugin.cpp
index a0beaf9..a4cad4c 100644
--- a/plugins/clirarplugin/cliplugin.cpp
+++ b/plugins/clirarplugin/cliplugin.cpp
@@ -27,6 +27,7 @@
#include <QDateTime>
+#include <KLocalizedString>
#include <KPluginFactory>
using namespace Kerfuffle;
@@ -56,6 +57,7 @@ void CliPlugin::resetParsing()
{
m_parseState = ParseStateTitle;
m_remainingIgnoreLines = 1;
+ m_unrarVersion.clear();
m_comment.clear();
m_numberOfVolumes = 0;
}
@@ -148,9 +150,9 @@ bool CliPlugin::readListLine(const QString &line)
if (matchVersion.hasMatch()) {
m_parseState = ParseStateComment;
- QString unrarVersion = matchVersion.captured(1);
- qCDebug(ARK) << "UNRAR version" << unrarVersion << "detected";
- if (unrarVersion.toFloat() >= 5) {
+ m_unrarVersion = matchVersion.captured(1);
+ qCDebug(ARK) << "UNRAR version" << m_unrarVersion << "detected";
+ if (m_unrarVersion.toFloat() >= 5) {
m_isUnrar5 = true;
qCDebug(ARK) << "Using UNRAR 5 parser";
} else {
@@ -166,15 +168,14 @@ bool CliPlugin::readListLine(const QString &line)
// Or see what version of unrar we are dealing with and call specific
// handler functions.
} else if (m_isUnrar5) {
- handleUnrar5Line(line);
+ return handleUnrar5Line(line);
} else {
- handleUnrar4Line(line);
+ return handleUnrar4Line(line);
}
-
return true;
}
-void CliPlugin::handleUnrar5Line(const QString &line)
+bool CliPlugin::handleUnrar5Line(const QString &line)
{
// Parses the comment field.
if (m_parseState == ParseStateComment) {
@@ -195,7 +196,7 @@ void CliPlugin::handleUnrar5Line(const QString &line)
m_comment.append(line + QLatin1Char('\n'));
}
- return;
+ return true;
}
// Parses the header, which is whatever is between the comment field
@@ -217,7 +218,7 @@ void CliPlugin::handleUnrar5Line(const QString &line)
qCDebug(ARK) << "Solid archive detected";
}
}
- return;
+ return true;
}
// Parses the entry details for each entry.
@@ -227,7 +228,7 @@ void CliPlugin::handleUnrar5Line(const QString &line)
// each volume.
if (line.startsWith(QLatin1String("Archive: "))) {
m_parseState = ParseStateHeader;
- return;
+ return true;
// Empty line indicates end of entry.
} else if (line.trimmed().isEmpty() && !m_unrar5Details.isEmpty()) {
@@ -238,7 +239,7 @@ void CliPlugin::handleUnrar5Line(const QString &line)
// All detail lines should contain a colon.
if (!line.contains(QLatin1Char(':'))) {
qCWarning(ARK) << "Unrecognized line:" << line;
- return;
+ return true;
}
// The details are on separate lines, so we store them in the QHash
@@ -247,8 +248,9 @@ void CliPlugin::handleUnrar5Line(const QString &line)
line.section(QLatin1Char(':'), 1).trimmed());
}
- return;
+ return true;
}
+ return true;
}
void CliPlugin::handleUnrar5Entry()
@@ -298,7 +300,7 @@ void CliPlugin::handleUnrar5Entry()
emit entry(e);
}
-void CliPlugin::handleUnrar4Line(const QString &line)
+bool CliPlugin::handleUnrar4Line(const QString &line)
{
// Parses the comment field.
if (m_parseState == ParseStateComment) {
@@ -307,6 +309,20 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// FIXME: Comment itself could also contain the Archive path string here.
QRegularExpression rxCommentEnd(QStringLiteral("^(Solid archive|Archive|Volume) .+$"));
+ // unrar 4 outputs the following string when opening v5 RAR archives.
+ if (line == QLatin1String("Unsupported archive format. Please update RAR to a newer version.")) {
+ emit error(i18n("Your unrar executable is version %1, which is too old to handle this archive. Please update to a more recent version.",
+ m_unrarVersion));
+ return false;
+ }
+
+ // unrar 3 reports a non-RAR archive when opening v5 RAR archives.
+ if (line.endsWith(QLatin1String(" is not RAR archive"))) {
+ emit error(i18n("Unrar reported a non-RAR archive. The installed unrar version (%1) is old. Try updating your unrar.",
+ m_unrarVersion));
+ return false;
+ }
+
if (rxCommentEnd.match(line).hasMatch()) {
if (line.startsWith(QLatin1String("Volume "))) {
@@ -332,7 +348,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
m_comment.append(line + QLatin1Char('\n'));
}
- return;
+ return true;
}
// Parses the header, which is whatever is between the comment field
@@ -345,7 +361,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
} else if (line.startsWith(QLatin1String("Volume "))) {
m_numberOfVolumes++;
}
- return;
+ return true;
}
// Parses the entry name, which is on the first line of each entry.
@@ -353,7 +369,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// Ignore empty lines.
if (line.trimmed().isEmpty()) {
- return;
+ return true;
}
// Three types of subHeaders can be displayed for unrar 3 and 4.
@@ -370,14 +386,14 @@ void CliPlugin::handleUnrar4Line(const QString &line)
} else if (matchSubHeader.captured(1) == QLatin1String("RR")) {
ignoreLines(3, ParseStateEntryFileName);
}
- return;
+ return true;
}
// The entries list ends with a horizontal line, followed by a
// single summary line or, for multi-volume archives, another header.
if (line.startsWith(QStringLiteral("-----------------"))) {
m_parseState = ParseStateHeader;
- return;
+ return true;
// Encrypted files are marked with an asterisk.
} else if (line.startsWith(QLatin1Char('*'))) {
@@ -388,7 +404,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// starting with a space is not an entry name.
} else if (!line.startsWith(QLatin1Char(' '))) {
qCWarning(ARK) << "Unrecognized line:" << line;
- return;
+ return true;
// If we reach this, then we can assume the line is an entry name, so
// save it, and move on to the rest of the entry details.
@@ -398,7 +414,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
m_parseState = ParseStateEntryDetails;
- return;
+ return true;
}
// Parses the remainder of the entry details for each entry.
@@ -412,7 +428,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// entry name, so go back to header.
if (line.startsWith(QStringLiteral("-----------------"))) {
m_parseState = ParseStateHeader;
- return;
+ return true;
}
// In unrar 3 and 4 the details are on a single line, so we
@@ -426,7 +442,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// not an archive entry.
if (m_unrar4Details.size() != 10) {
m_parseState = ParseStateHeader;
- return;
+ return true;
}
// When unrar 3 and 4 list a symlink, they output an extra line
@@ -434,7 +450,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// the line we ignore, so we first need to ignore one line.
if (m_unrar4Details.at(6).startsWith(QLatin1Char('l'))) {
ignoreLines(1, ParseStateLinkTarget);
- return;
+ return true;
} else {
handleUnrar4Entry();
}
@@ -444,7 +460,7 @@ void CliPlugin::handleUnrar4Line(const QString &line)
// line.
ignoreLines(1, ParseStateEntryFileName);
- return;
+ return true;
}
// Parses a symlink target.
@@ -454,8 +470,9 @@ void CliPlugin::handleUnrar4Line(const QString &line)
handleUnrar4Entry();
m_parseState = ParseStateEntryFileName;
- return;
+ return true;
}
+ return true;
}
void CliPlugin::handleUnrar4Entry()
diff --git a/plugins/clirarplugin/cliplugin.h b/plugins/clirarplugin/cliplugin.h
index dc7b48c..7340e6f 100644
--- a/plugins/clirarplugin/cliplugin.h
+++ b/plugins/clirarplugin/cliplugin.h
@@ -49,15 +49,16 @@ private:
ParseStateLinkTarget
} m_parseState;
- void handleUnrar5Line(const QString &line);
+ bool handleUnrar5Line(const QString &line);
void handleUnrar5Entry();
- void handleUnrar4Line(const QString &line);
+ bool handleUnrar4Line(const QString &line);
void handleUnrar4Entry();
void ignoreLines(int lines, ParseState nextState);
QStringList m_unrar4Details;
QHash<QString, QString> m_unrar5Details;
+ QString m_unrarVersion;
bool m_isUnrar5;
bool m_isPasswordProtected;
bool m_isSolid;