Merge pull request #14912 from NissimHadar/21185-protectNitpickFromBadFilename

Case 21185: protect nitpick from bad filename

tools/CMakeLists.txt

@@ -19,8 +19,8 @@ function(check_test name)
 endfunction()
 
 if (BUILD_TOOLS)
-    # Allow different tools for production builds
+    # Allow different tools for stable builds
-    if (RELEASE_TYPE STREQUAL "PRODUCTION")
+    if (STABLE_BUILD)
         set(ALL_TOOLS 
             udt-test 
             vhacd-util

tools/nitpick/src/AWSInterface.cpp

@@ -27,13 +27,30 @@ void AWSInterface::createWebPageFromResults(const QString& testResults,
                                             const QString& workingDirectory,
                                             QCheckBox* updateAWSCheckBox,
                                             QLineEdit* urlLineEdit) {
-    _testResults = testResults;
     _workingDirectory = workingDirectory;
 
+    // Verify filename is in correct format 
+    // For example `D:/tt/TestResults--2019-02-10_17-30-57(local)[DESKTOP-6BO62Q9].zip`
+    QStringList parts = testResults.split('/');
+    QString zipFilename = parts[parts.length() - 1];
+
+    QStringList zipFolderNameParts = zipFilename.split(QRegExp("[\\(\\)\\[\\]]"), QString::SkipEmptyParts);
+
+    if (!QRegularExpression("TestResults--\\d{4}(-\\d\\d){2}_\\d\\d(-\\d\\d){2}").match(zipFolderNameParts[0]).hasMatch() ||
+        !QRegularExpression("\\w").match(zipFolderNameParts[1]).hasMatch() ||                                                 // build (local, build number or PR number)
+        !QRegularExpression("\\w").match(zipFolderNameParts[2]).hasMatch()                                                  // machine name
+    ) {
+        QMessageBox::critical(0, "Filename is in wrong format", "'" + zipFilename + "' is not in nitpick format");
+        return;
+    }
+
+    _testResults = testResults;
+
     _urlLineEdit = urlLineEdit;
     _urlLineEdit->setEnabled(false);
 
-    extractTestFailuresFromZippedFolder();
+    QString zipFilenameWithoutExtension = zipFilename.split('.')[0];
+    extractTestFailuresFromZippedFolder(_workingDirectory + "/" + zipFilenameWithoutExtension);
     createHTMLFile();
 
     if (updateAWSCheckBox->isChecked()) {
@@ -44,14 +61,12 @@ void AWSInterface::createWebPageFromResults(const QString& testResults,
     }
 }
 
-void AWSInterface::extractTestFailuresFromZippedFolder() {
+void AWSInterface::extractTestFailuresFromZippedFolder(const QString& folderName) {
     // For a test results zip file called `D:/tt/TestResults--2018-10-02_16-54-11(9426)[DESKTOP-PMKNLSQ].zip`
     //   the folder will be called `TestResults--2018-10-02_16-54-11(9426)[DESKTOP-PMKNLSQ]`
     //   and, this folder will be in the working directory
-    QStringList parts = _testResults.split('/');
+    if (QDir(folderName).exists()) {
-    QString zipFolderName = _workingDirectory + "/" + parts[parts.length() - 1].split('.')[0];
+        QDir dir = folderName;
-    if (QDir(zipFolderName).exists()) {
-        QDir dir = zipFolderName;
         dir.removeRecursively();
     }
 

tools/nitpick/src/AWSInterface.h

@@ -30,7 +30,7 @@ public:
                                   QCheckBox* updateAWSCheckBox,
                                   QLineEdit* urlLineEdit);
 
-    void extractTestFailuresFromZippedFolder();
+    void extractTestFailuresFromZippedFolder(const QString& folderName);
     void createHTMLFile();
 
     void startHTMLpage(QTextStream& stream);

tools/nitpick/src/Nitpick.cpp

@@ -40,7 +40,7 @@ Nitpick::Nitpick(QWidget* parent) : QMainWindow(parent) {
  
     _ui.plainTextEdit->setReadOnly(true);
 
-   setWindowTitle("Nitpick - v2.1.1");
+   setWindowTitle("Nitpick - v2.1.2");
 }
 
 Nitpick::~Nitpick() {

tools/nitpick/src/Test.cpp

@@ -835,11 +835,16 @@ void Test::createRecursiveScript(const QString& directory, bool interactiveMode)
         << endl;
     textStream << "Script.include(PATH_TO_THE_REPO_PATH_UTILS_FILE);" << endl << endl;
 
-    textStream << "if (typeof nitpick === 'undefined') nitpick = createNitpick(Script.resolvePath(\".\"));" << endl;
+    // The 'depth' variable is used to signal when to start running the recursive scripts
-    textStream << "if (typeof testsRootPath === 'undefined') testsRootPath = nitpick.getTestsRootPath();" << endl << endl;
+    textStream << "if (typeof depth === 'undefined') {" << endl; 
-
+    textStream << "   depth = 0;" << endl;
+    textStream << "   nitpick = createNitpick(Script.resolvePath(\".\"));" << endl;
+    textStream << "   testsRootPath = nitpick.getTestsRootPath();" << endl << endl;
     textStream << "   nitpick.enableRecursive();" << endl;
-    textStream << "nitpick.enableAuto();" << endl << endl;
+    textStream << "   nitpick.enableAuto();" << endl;
+    textStream << "} else {" << endl;
+    textStream << "   depth++" << endl;
+    textStream << "}" << endl << endl;
 
     // Now include the test scripts
     for (int i = 0; i < directories.length(); ++i) {
@@ -847,8 +852,9 @@ void Test::createRecursiveScript(const QString& directory, bool interactiveMode)
     }
 
     textStream << endl;
-    textStream << "if (typeof runningRecursive === 'undefined') {" << endl; 
+    textStream << "if (depth > 0) {" << endl;
-    textStream << "   runningRecursive = true;" << endl;
+    textStream << "   depth--;" << endl;
+    textStream << "} else {" << endl;
     textStream << "   nitpick.runRecursive();" << endl;
     textStream << "}" << endl << endl;
 
@@ -1091,7 +1097,7 @@ void Test::setTestRailCreateMode(TestRailCreateMode testRailCreateMode) {
 
 void Test::createWebPage(QCheckBox* updateAWSCheckBox, QLineEdit* urlLineEdit) {
     QString testResults = QFileDialog::getOpenFileName(nullptr, "Please select the zipped test results to update from", nullptr,
-                                                       "Zipped Test Results (*.zip)");
+                                                       "Zipped Test Results (TestResults--*.zip)");
     if (testResults.isNull()) {
         return;
     }

tools/nitpick/src/TestRunnerDesktop.cpp

@@ -554,7 +554,7 @@ void TestRunnerDesktop::evaluateResults() {
     nitpick->startTestsEvaluation(false, true, _snapshotFolder, _branch, _user);
 }
 
-void TestRunnerDesktop::automaticTestRunEvaluationComplete(QString zippedFolder, int numberOfFailures) {
+void TestRunnerDesktop::automaticTestRunEvaluationComplete(const QString& zippedFolder, int numberOfFailures) {
     addBuildNumberToResults(zippedFolder);
     restoreHighFidelityAppDataFolder();
 
@@ -580,14 +580,19 @@ void TestRunnerDesktop::automaticTestRunEvaluationComplete(QString zippedFolder,
     _runNow->setEnabled(true);
 }
 
-void TestRunnerDesktop::addBuildNumberToResults(QString zippedFolderName) {
+void TestRunnerDesktop::addBuildNumberToResults(const QString& zippedFolderName) {
-    QString augmentedFilename;
+    QString augmentedFilename { zippedFolderName };
     if (!_runLatest->isChecked()) {
-        augmentedFilename = zippedFolderName.replace("local", getPRNumberFromURL(_url->text()));
+        augmentedFilename.replace("local", getPRNumberFromURL(_url->text()));
     } else {
-        augmentedFilename = zippedFolderName.replace("local", _buildInformation.build);
+        augmentedFilename.replace("local", _buildInformation.build);
+    }
+
+    if (!QFile::rename(zippedFolderName, augmentedFilename)) {
+        QMessageBox::critical(0, "Internal error: " + QString(__FILE__) + ":" + QString::number(__LINE__), "Could not rename '" + zippedFolderName + "' to '" + augmentedFilename);
+        exit(-1);
+    
     }
-    QFile::rename(zippedFolderName, augmentedFilename);
 }
 
 void TestRunnerDesktop::restoreHighFidelityAppDataFolder() {

tools/nitpick/src/TestRunnerDesktop.h

@@ -61,8 +61,8 @@ public:
     void runInterfaceWithTestScript();
 
     void evaluateResults();
-    void automaticTestRunEvaluationComplete(QString zippedFolderName, int numberOfFailures);
+    void automaticTestRunEvaluationComplete(const QString& zippedFolderName, int numberOfFailures);
-    void addBuildNumberToResults(QString zippedFolderName);
+    void addBuildNumberToResults(const QString& zippedFolderName);
 
     void copyFolder(const QString& source, const QString& destination);