summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRagnar Thomsen <rthomsen6@gmail.com>2016-11-15 16:13:00 (GMT)
committerRagnar Thomsen <rthomsen6@gmail.com>2016-11-15 16:13:00 (GMT)
commit702468a11f12d20b828c54785e92bdea7d254185 (patch)
tree87ee57bdd94e2e5b2f10f3717b2b556b2ea297ec
parentf399eaa2df9f08eb7ae90aa1937f0999227cfa47 (diff)
Improve error handling for CliPlugins
This commit removes the ExtractionFailedPatterns regexp in CliProperties. Instead we take care of the error handling in the individual plugins by adding a pure virtual readExtractLine() method to CliInterface (similar to readListLine()). This way we can provide custom error messages to Part, instead of the generic "Extraction failed due to an unexpected error" (this message was also used for ListJob). Differential Revision: D3142
-rw-r--r--kerfuffle/cliinterface.cpp15
-rw-r--r--kerfuffle/cliinterface.h1
-rw-r--r--kerfuffle/cliproperties.cpp10
-rw-r--r--kerfuffle/cliproperties.h3
-rw-r--r--plugins/cli7zplugin/cliplugin.cpp21
-rw-r--r--plugins/cli7zplugin/cliplugin.h1
-rw-r--r--plugins/clirarplugin/cliplugin.cpp31
-rw-r--r--plugins/clirarplugin/cliplugin.h1
-rw-r--r--plugins/cliunarchiverplugin/cliplugin.cpp31
-rw-r--r--plugins/cliunarchiverplugin/cliplugin.h1
-rw-r--r--plugins/clizipplugin/cliplugin.cpp19
-rw-r--r--plugins/clizipplugin/cliplugin.h1
12 files changed, 92 insertions, 43 deletions
diff --git a/kerfuffle/cliinterface.cpp b/kerfuffle/cliinterface.cpp
index f7988ba..9dc7bc9 100644
--- a/kerfuffle/cliinterface.cpp
+++ b/kerfuffle/cliinterface.cpp
@@ -767,7 +767,6 @@ void CliInterface::readStdout(bool handleAll)
bool foundErrorMessage =
(wrongPasswordMessage ||
m_cliProps->isDiskFullMsg(QLatin1String(lines.last())) ||
- m_cliProps->isExtractionFailedMsg(QLatin1String(lines.last())) ||
m_cliProps->isfileExistsMsg(QLatin1String(lines.last()))) ||
m_cliProps->isPasswordPrompt(QLatin1String(lines.last()));
@@ -869,15 +868,11 @@ bool CliInterface::handleLine(const QString& line)
return false;
}
- if (m_cliProps->isExtractionFailedMsg(line)) {
- qCWarning(ARK) << "Error in extraction:" << line;
- emit error(i18n("Extraction failed because of an unexpected error."));
- return false;
- }
-
if (handleFileExistsMessage(line)) {
return true;
}
+
+ return readExtractLine(line);
}
if (m_operationMode == List) {
@@ -907,12 +902,6 @@ bool CliInterface::handleLine(const QString& line)
return false;
}
- if (m_cliProps->isExtractionFailedMsg(line)) {
- qCWarning(ARK) << "Error in extraction!!";
- emit error(i18n("Extraction failed because of an unexpected error."));
- return false;
- }
-
if (m_cliProps->isCorruptArchiveMsg(line)) {
qCWarning(ARK) << "Archive corrupt";
setCorrupt(true);
diff --git a/kerfuffle/cliinterface.h b/kerfuffle/cliinterface.h
index 6d456b0..122d03c 100644
--- a/kerfuffle/cliinterface.h
+++ b/kerfuffle/cliinterface.h
@@ -71,6 +71,7 @@ public:
virtual void resetParsing() = 0;
virtual bool readListLine(const QString &line) = 0;
+ virtual bool readExtractLine(const QString &line) = 0;
bool doKill() Q_DECL_OVERRIDE;
bool doSuspend() Q_DECL_OVERRIDE;
bool doResume() Q_DECL_OVERRIDE;
diff --git a/kerfuffle/cliproperties.cpp b/kerfuffle/cliproperties.cpp
index d9db2a7..726d754 100644
--- a/kerfuffle/cliproperties.cpp
+++ b/kerfuffle/cliproperties.cpp
@@ -338,16 +338,6 @@ bool CliProperties::isFileExistsFileName(const QString &line)
return false;
}
-bool CliProperties::isExtractionFailedMsg(const QString &line)
-{
- foreach(const QString &rx, m_extractionFailedPatterns) {
- if (QRegularExpression(rx).match(line).hasMatch()) {
- return true;
- }
- }
- return false;
-}
-
bool CliProperties::isCorruptArchiveMsg(const QString &line)
{
foreach(const QString &rx, m_corruptArchivePatterns) {
diff --git a/kerfuffle/cliproperties.h b/kerfuffle/cliproperties.h
index 4955a97..2574434 100644
--- a/kerfuffle/cliproperties.h
+++ b/kerfuffle/cliproperties.h
@@ -67,7 +67,6 @@ class KERFUFFLE_EXPORT CliProperties: public QObject
Q_PROPERTY(QStringList testPassedPatterns MEMBER m_testPassedPatterns)
Q_PROPERTY(QStringList fileExistsPatterns MEMBER m_fileExistsPatterns)
Q_PROPERTY(QStringList fileExistsFileName MEMBER m_fileExistsFileName)
- Q_PROPERTY(QStringList extractionFailedPatterns MEMBER m_extractionFailedPatterns)
Q_PROPERTY(QStringList corruptArchivePatterns MEMBER m_corruptArchivePatterns)
Q_PROPERTY(QStringList diskFullPatterns MEMBER m_diskFullPatterns)
@@ -99,7 +98,6 @@ public:
bool isTestPassedMsg(const QString &line);
bool isfileExistsMsg(const QString &line);
bool isFileExistsFileName(const QString &line);
- bool isExtractionFailedMsg(const QString &line);
bool isCorruptArchiveMsg(const QString &line);
bool isDiskFullMsg(const QString &line);
@@ -139,7 +137,6 @@ private:
QStringList m_testPassedPatterns;
QStringList m_fileExistsPatterns;
QStringList m_fileExistsFileName;
- QStringList m_extractionFailedPatterns;
QStringList m_corruptArchivePatterns;
QStringList m_diskFullPatterns;
diff --git a/plugins/cli7zplugin/cliplugin.cpp b/plugins/cli7zplugin/cliplugin.cpp
index 1121c65..985bc9b 100644
--- a/plugins/cli7zplugin/cliplugin.cpp
+++ b/plugins/cli7zplugin/cliplugin.cpp
@@ -30,6 +30,7 @@
#include <QDir>
#include <QRegularExpression>
+#include <KLocalizedString>
#include <KPluginFactory>
using namespace Kerfuffle;
@@ -108,8 +109,6 @@ void CliPlugin::setupCliProperties()
QStringLiteral("A"), //Overwrite all
QStringLiteral("S"), //Autoskip
QStringLiteral("Q")}); //Cancel
- m_cliProps->setProperty("extractionFailedPatterns", QStringList{QStringLiteral("ERROR: E_FAIL"),
- QStringLiteral("Open ERROR: Can not open the file as \\[7z\\] archive")});
m_cliProps->setProperty("corruptArchivePatterns", QStringList{QStringLiteral("Unexpected end of archive"),
QStringLiteral("Headers Error")});
m_cliProps->setProperty("diskFullPatterns", QStringList{QStringLiteral("No space left on device")});
@@ -123,6 +122,12 @@ bool CliPlugin::readListLine(const QString& line)
static const QLatin1String entryInfoDelimiter("----------");
const QRegularExpression rxComment(QStringLiteral("Comment = .+$"));
+ const QRegularExpression rxListFailed(QStringLiteral("Open ERROR: Can not open the file as \\[7z\\] archive"));
+ if (rxListFailed.match(line).hasMatch()) {
+ emit error(i18n("Listing the archive failed."));
+ return false;
+ }
+
if (m_parseState == ParseStateTitle) {
const QRegularExpression rxVersionLine(QStringLiteral("^p7zip Version ([\\d\\.]+) .*$"));
@@ -268,6 +273,18 @@ bool CliPlugin::readListLine(const QString& line)
return true;
}
+bool CliPlugin::readExtractLine(const QString &line)
+{
+ QRegularExpression rx(QStringLiteral("ERROR: E_FAIL"));
+
+ if (rx.match(line).hasMatch()) {
+ emit error(i18n("Extraction failed."));
+ return false;
+ }
+
+ return true;
+}
+
void CliPlugin::handleMethods(const QStringList &methods)
{
foreach (const QString &method, methods) {
diff --git a/plugins/cli7zplugin/cliplugin.h b/plugins/cli7zplugin/cliplugin.h
index d8cbfa7..2e8a13c 100644
--- a/plugins/cli7zplugin/cliplugin.h
+++ b/plugins/cli7zplugin/cliplugin.h
@@ -38,6 +38,7 @@ public:
virtual void resetParsing() Q_DECL_OVERRIDE;
virtual bool readListLine(const QString &line) Q_DECL_OVERRIDE;
+ virtual bool readExtractLine(const QString &line) Q_DECL_OVERRIDE;
private:
enum ArchiveType {
diff --git a/plugins/clirarplugin/cliplugin.cpp b/plugins/clirarplugin/cliplugin.cpp
index cce5052..d0eb730 100644
--- a/plugins/clirarplugin/cliplugin.cpp
+++ b/plugins/clirarplugin/cliplugin.cpp
@@ -120,8 +120,6 @@ void CliPlugin::setupCliProperties()
QStringLiteral("A"), //Overwrite all
QStringLiteral("E"), //Autoskip
QStringLiteral("Q")}); //Cancel
- m_cliProps->setProperty("extractionFailedPatterns", QStringList{QStringLiteral("CRC failed"),
- QStringLiteral("Cannot find volume")});
m_cliProps->setProperty("corruptArchivePatterns", QStringList{QStringLiteral("Unexpected end of archive"),
QStringLiteral("the file header is corrupt")});
m_cliProps->setProperty("diskFullPatterns", QStringList{QStringLiteral("No space left on device")});
@@ -175,6 +173,12 @@ bool CliPlugin::readListLine(const QString &line)
bool CliPlugin::handleUnrar5Line(const QString &line)
{
+ const QRegularExpression rxVolume(QStringLiteral("Cannot find volume "));
+ if (rxVolume.match(line).hasMatch()) {
+ emit error(i18n("Failed to find all archive volumes."));
+ return false;
+ }
+
// Parses the comment field.
if (m_parseState == ParseStateComment) {
@@ -309,6 +313,12 @@ void CliPlugin::handleUnrar5Entry()
bool CliPlugin::handleUnrar4Line(const QString &line)
{
+ const QRegularExpression rxVolume(QStringLiteral("Cannot find volume "));
+ if (rxVolume.match(line).hasMatch()) {
+ emit error(i18n("Failed to find all archive volumes."));
+ return false;
+ }
+
// Parses the comment field.
if (m_parseState == ParseStateComment) {
@@ -543,6 +553,23 @@ void CliPlugin::handleUnrar4Entry()
emit entry(e);
}
+bool CliPlugin::readExtractLine(const QString &line)
+{
+ const QRegularExpression rxCRC(QStringLiteral("CRC failed"));
+ if (rxCRC.match(line).hasMatch()) {
+ emit error(i18n("One or more wrong checksums"));
+ return false;
+ }
+
+ const QRegularExpression rxVolume(QStringLiteral("Cannot find volume "));
+ if (rxVolume.match(line).hasMatch()) {
+ emit error(i18n("Failed to find all archive volumes."));
+ return false;
+ }
+
+ return true;
+}
+
void CliPlugin::ignoreLines(int lines, ParseState nextState)
{
m_remainingIgnoreLines = lines;
diff --git a/plugins/clirarplugin/cliplugin.h b/plugins/clirarplugin/cliplugin.h
index fe70880..59d5e52 100644
--- a/plugins/clirarplugin/cliplugin.h
+++ b/plugins/clirarplugin/cliplugin.h
@@ -37,6 +37,7 @@ public:
virtual void resetParsing() Q_DECL_OVERRIDE;
virtual bool readListLine(const QString &line) Q_DECL_OVERRIDE;
+ virtual bool readExtractLine(const QString &line) Q_DECL_OVERRIDE;
private:
diff --git a/plugins/cliunarchiverplugin/cliplugin.cpp b/plugins/cliunarchiverplugin/cliplugin.cpp
index 542431e..50ba5de 100644
--- a/plugins/cliunarchiverplugin/cliplugin.cpp
+++ b/plugins/cliunarchiverplugin/cliplugin.cpp
@@ -91,14 +91,28 @@ void CliPlugin::setupCliProperties()
QStringLiteral("$Password")});
m_cliProps->setProperty("passwordPromptPatterns", QStringList{QStringLiteral("This archive requires a password to unpack. Use the -p option to provide one.")});
-
- m_cliProps->setProperty("extractionFailedPatterns", QStringList{QStringLiteral("Failed! \\((.+)\\)$"),
- QStringLiteral("Segmentation fault$")});
}
bool CliPlugin::readListLine(const QString &line)
{
- Q_UNUSED(line)
+ const QRegularExpression rx(QStringLiteral("Failed! \\((.+)\\)$"));
+
+ if (rx.match(line).hasMatch()) {
+ emit error(i18n("Listing the archive failed."));
+ return false;
+ }
+
+ return true;
+}
+
+bool CliPlugin::readExtractLine(const QString &line)
+{
+ const QRegularExpression rx(QStringLiteral("Failed! \\((.+)\\)$"));
+
+ if (rx.match(line).hasMatch()) {
+ emit error(i18n("Extraction failed."));
+ return false;
+ }
return true;
}
@@ -127,15 +141,6 @@ bool CliPlugin::handleLine(const QString& line)
m_jsonOutput += line + QLatin1Char('\n');
}
- // TODO: is this check really needed?
- if (m_operationMode == Copy) {
- if (m_cliProps->isExtractionFailedMsg(line)) {
- qCWarning(ARK) << "Error in extraction:" << line;
- emit error(i18n("Extraction failed because of an unexpected error."));
- return false;
- }
- }
-
if (m_operationMode == List) {
// This can only be an header-encrypted archive.
if (m_cliProps->isPasswordPrompt(line)) {
diff --git a/plugins/cliunarchiverplugin/cliplugin.h b/plugins/cliunarchiverplugin/cliplugin.h
index 5e8c8f8..641de16 100644
--- a/plugins/cliunarchiverplugin/cliplugin.h
+++ b/plugins/cliunarchiverplugin/cliplugin.h
@@ -38,6 +38,7 @@ public:
virtual bool extractFiles(const QVector<Kerfuffle::Archive::Entry*> &files, const QString &destinationDirectory, const Kerfuffle::ExtractionOptions &options) Q_DECL_OVERRIDE;
virtual void resetParsing() Q_DECL_OVERRIDE;
virtual bool readListLine(const QString &line) Q_DECL_OVERRIDE;
+ virtual bool readExtractLine(const QString &line) Q_DECL_OVERRIDE;
/**
* Fill the lsar's json output all in once (useful for unit testing).
diff --git a/plugins/clizipplugin/cliplugin.cpp b/plugins/clizipplugin/cliplugin.cpp
index 33ba408..980b320 100644
--- a/plugins/clizipplugin/cliplugin.cpp
+++ b/plugins/clizipplugin/cliplugin.cpp
@@ -188,6 +188,25 @@ bool CliPlugin::readListLine(const QString &line)
return true;
}
+bool CliPlugin::readExtractLine(const QString &line)
+{
+ const QRegularExpression rxUnsupCompMethod(QStringLiteral("unsupported compression method (\\d+)"));
+ const QRegularExpression rxUnsupEncMethod(QStringLiteral("need PK compat. v\\d\\.\\d \\(can do v\\d\\.\\d\\)"));
+
+ QRegularExpressionMatch unsupCompMethodMatch = rxUnsupCompMethod.match(line);
+ if (unsupCompMethodMatch.hasMatch()) {
+ emit error(i18n("Extraction failed due to unsupported compression method (%1).", unsupCompMethodMatch.captured(1)));
+ return false;
+ }
+
+ if (QRegularExpression(rxUnsupEncMethod).match(line).hasMatch()) {
+ emit error(i18n("Extraction failed due to unsupported encryption method."));
+ return false;
+ }
+
+ return true;
+}
+
bool CliPlugin::moveFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions &options)
{
qCDebug(ARK) << "Moving" << files.count() << "file(s) to destination:" << destination;
diff --git a/plugins/clizipplugin/cliplugin.h b/plugins/clizipplugin/cliplugin.h
index 995d3d9..c76f9d8 100644
--- a/plugins/clizipplugin/cliplugin.h
+++ b/plugins/clizipplugin/cliplugin.h
@@ -39,6 +39,7 @@ public:
virtual void resetParsing() Q_DECL_OVERRIDE;
virtual QString escapeFileName(const QString &fileName) const Q_DECL_OVERRIDE;
virtual bool readListLine(const QString &line) Q_DECL_OVERRIDE;
+ virtual bool readExtractLine(const QString &line) Q_DECL_OVERRIDE;
virtual bool moveFiles(const QVector<Archive::Entry*> &files, Archive::Entry *destination, const CompressionOptions& options) Q_DECL_OVERRIDE;
virtual int moveRequiredSignals() const Q_DECL_OVERRIDE;