peek into file before open

* check file open, providing nice and early error * explicit check for perfparser (magic number QPERFSTREAM) opening directly without the need to fall back to file name * explicit check for V1 perf data which isn't supported by perfparser * explicit check for unknown file type
KDAB · Nov 14, 2023 · 0c43808 · 0c43808
1 parent 930909c
commit 0c43808
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 16 deletions.
diff --git a/src/parsers/perf/perfparser.cpp b/src/parsers/perf/perfparser.cpp
@@ -1476,6 +1476,7 @@ PerfParser::~PerfParser() = default;
 
 bool PerfParser::initParserArgs(const QString& path)
 {
+    // check for common file issues
     const auto info = QFileInfo(path);
     if (!info.exists()) {
         emit parsingFailed(tr("File '%1' does not exist.").arg(path));
@@ -1490,16 +1491,32 @@ bool PerfParser::initParserArgs(const QString& path)
         return false;
     }
 
+    // peek into file header
+    const auto filename = decompressIfNeeded(path);
+    QFile file(filename);
+    file.open(QIODevice::ReadOnly);
+    if (file.peek(8) != "PERFILE2" && file.peek(11) != "QPERFSTREAM") {
+        if (file.peek(8) == "PERFFILE") {
+            emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, tr("Unsupported V1 perf data")));
+        } else {
+            emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, tr("File format unknown")));
+        }
+        file.close();
+        return false;
+    }
+    file.close();
+
+    // check perfparser and set initial values
     auto parserBinary = Util::perfParserBinaryPath();
     if (parserBinary.isEmpty()) {
         emit parsingFailed(tr("Failed to find hotspot-perfparser binary."));
         return false;
     }
 
-    auto parserArgs = [this](const QString& filename) {
+    auto parserArgs = [](const QString& filename) {
         const auto settings = Settings::instance();
-        QStringList parserArgs = {QStringLiteral("--input"), decompressIfNeeded(filename),
-                                  QStringLiteral("--max-frames"), QStringLiteral("1024")};
+        QStringList parserArgs = {QStringLiteral("--input"), filename, QStringLiteral("--max-frames"),
+                                  QStringLiteral("1024")};
         const auto sysroot = settings->sysroot();
         if (!sysroot.isEmpty()) {
             parserArgs += {QStringLiteral("--sysroot"), sysroot};
@@ -1531,7 +1548,7 @@ bool PerfParser::initParserArgs(const QString& path)
         return parserArgs;
     };
 
-    m_parserArgs = parserArgs(path);
+    m_parserArgs = parserArgs(filename);
     m_parserBinary = parserBinary;
     return true;
 }
@@ -1584,22 +1601,23 @@ void PerfParser::startParseFile(const QString& path)
             emit parsingFinished();
         };
 
-        if (path.endsWith(QLatin1String(".perfparser"))) {
-            QFile file(path);
-            if (!file.open(QIODevice::ReadOnly)) {
-                emit parsingFailed(tr("Failed to open file %1: %2").arg(path, file.errorString()));
-                return;
-            }
+        // note: file is always readable and in supported format here,
+        //        already validated in initParserArgs()
+        QFile file(path);
+        file.open(QIODevice::ReadOnly);
+        if (file.peek(11) == "QPERFSTREAM") {
             d.setInput(&file);
             while (!file.atEnd() && !d.stopRequested) {
                 if (!d.tryParse()) {
-                    emit parsingFailed(tr("Failed to parse file"));
+                    // TODO: provide reason
+                    emit parsingFailed(tr("Failed to parse file %1: %2").arg(path, QStringLiteral("Unknown reason")));
                     return;
                 }
             }
             finalize();
             return;
         }
+        file.close();
 
         QProcess process;
         process.setProcessEnvironment(perfparserEnvironment(debuginfodUrls));

diff --git a/tests/integrationtests/tst_perfparser.cpp b/tests/integrationtests/tst_perfparser.cpp
@@ -226,11 +226,15 @@ private slots:
         QTest::addColumn<QString>("errorMessagePart");
         QTest::addColumn<int>("waitTime");
 
-        QTest::addRow("pre-exported perfparser") << QFINDTESTDATA("file_content/true.perfparser") << QString() << 2000;
-        QTest::addRow("invalid data") << QFINDTESTDATA("tst_perfparser.cpp") << QStringLiteral("invalid perf data file")
-                                      << 1000;
-        QTest::addRow("PERF v1") << QFINDTESTDATA("file_content/perf.data.true.v1")
-                                 << QStringLiteral("invalid perf data file") << 1000;
+        const auto perfData = QFINDTESTDATA("file_content/true.perfparser");
+        QTest::addRow("pre-exported perfparser") << perfData << QString() << 2000;
+        const auto perfDataSomeName = QStringLiteral("fruitper");
+        QFile::copy(perfData, perfDataSomeName); // we can ignore errors (file exist) here
+        QTest::addRow("pre-exported perfparser \"bad extension\"") << perfDataSomeName << QString() << 2000;
+        QTest::addRow("no expected magic header")
+            << QFINDTESTDATA("tst_perfparser.cpp") << QStringLiteral("File format unknown") << 1000;
+        QTest::addRow("PERF v1") << QFINDTESTDATA("file_content/perf.data.true.v1") << QStringLiteral("V1 perf data")
+                                 << 1000;
 
         // TODO: check why we need this long waittime
         QTest::addRow("PERF v2") << QFINDTESTDATA("file_content/perf.data.true.v2") << QString() << 9000;
@@ -272,6 +276,7 @@ private slots:
             QCOMPARE(parsingFinishedSpy.count(), 0);
             const auto message = parsingFailedSpy.takeFirst().at(0).toString();
             QVERIFY(message.contains(errorMessagePart));
+            QVERIFY(message.contains(perfFile));
         }
     }