JENKINS-28640: Quotation marks in commit msg:

Only related to the accumulated strategy. Temporary fix for the bug with quotation marks in the commit message leading to merge failure because of not properly escaped and formatted command line arguments. Fix is temporary replacing " (quotation marks) with ' (plings). The squashed strategy is not affected, as the git command automatically generated the commit message, where as the plugin creates a look-a-like squash message, slightly modified, is using accumulated strategy. Other improvements: - fixed hard-coded dependency on the maven-hpi-plugin in the pom file, new version works now - cleaned few tests from wrong comments, and not clearly formatted error messages if the tests failed - renamed test suite for commit message prolems related to quotation marks to special chars test suite - shared a useful method from one test suite through test utils class that can pretty print console output from the job to use for verification in the tests.
jenkinsci · Jul 7, 2015 · a48f690 · a48f690
1 parent cbd24a9
commit a48f690
Show file tree

Hide file tree

Showing 16 changed files with 666 additions and 151 deletions.
diff --git a/pom.xml b/pom.xml
@@ -122,7 +122,6 @@
             <plugin>
                 <groupId>org.jenkins-ci.tools</groupId>
                 <artifactId>maven-hpi-plugin</artifactId>
-                <version>1.109</version> <!-- Older version have some problems with writing the manifest file -->
                 <configuration>
                     <pluginVersionDescription>${buildNumber}</pluginVersionDescription>
                     <compatibleSinceVersion>2.0</compatibleSinceVersion>

diff --git a/...in/java/org/jenkinsci/plugins/pretestedintegration/scm/git/AccumulatedCommitStrategy.java b/...in/java/org/jenkinsci/plugins/pretestedintegration/scm/git/AccumulatedCommitStrategy.java
@@ -122,7 +122,9 @@ public void integrate(AbstractBuild<?,?> build, Launcher launcher, BuildListener
             // Merge and commit must be splitted in two steps, to allow setting author which only is supported on commit command, not merge.
             logger.info("Starting accumulated merge (no-ff) - without commit:");
             listener.getLogger().println(String.format(LOG_PREFIX + "Starting accumulated merge (no-ff) - without commit:"));
-            exitCodeMerge = gitbridge.git(build, launcher, listener, out, "merge", "-m", String.format("%s%n%s", headerLine, commits), commit, "--no-ff", "--no-commit");
+            String commitMsg = String.format("%s%n%s", headerLine, commits);
+            String modifiedCommitMsg = commitMsg.replaceAll("\"","'");
+            exitCodeMerge = gitbridge.git(build, launcher, listener, out, "merge", "--no-ff", "--no-commit", "-m", modifiedCommitMsg, commit);
             logger.info("Accumulated merge done");
             listener.getLogger().println(String.format(LOG_PREFIX + "Accumulated merge done"));
         } catch (Exception ex) {

diff --git a/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java b/src/main/java/org/jenkinsci/plugins/pretestedintegration/scm/git/GitBridge.java
@@ -186,7 +186,7 @@ private ProcStarter buildCommand(AbstractBuild<?, ?> build, Launcher launcher, T
         b.add(cmds);
         listener.getLogger().println(String.format("%s %s", PretestedIntegrationBuildWrapper.LOG_PREFIX, b.toStringWithQuote() ));
         logger.exiting("GitBridge", "buildCommand");// Generated code DONT TOUCH! Bookmark: b2de8fe32eb583d6dac86f020b66bfa4
-		return launcher.launch().pwd(resolveWorkspace(build, listener)).cmds(b);
+        return launcher.launch().pwd(resolveWorkspace(build, listener)).cmds(b);
     }
 
     /**
@@ -209,7 +209,7 @@ public int git(AbstractBuild<?, ?> build, Launcher launcher, TaskListener listen
     }
 
     /**
-     * Invoke a command with mercurial
+     * Invoke a command with git
      *
      * @param build
      * @param launcher

diff --git a/...m/git/CommitMessagesWithDoubleQuotes.java → ...m/git/CommitMessagesWithSpecialChars.java b/...m/git/CommitMessagesWithDoubleQuotes.java → ...m/git/CommitMessagesWithSpecialChars.java
diff --git a/...g/jenkinsci/plugins/pretestedintegration/integration/scm/git/CustomIntegrationBranch.java b/...g/jenkinsci/plugins/pretestedintegration/integration/scm/git/CustomIntegrationBranch.java
@@ -50,40 +50,6 @@
 public class CustomIntegrationBranch extends StaticGitRepositoryTestBase {
 
 
-    /**
-     * Pretty prints console output from build log
-     *
-     * @param build
-     * @param buildname - descriptive build name included in the output
-     * @return boolean - true matched console like text, else false
-     * @throws IOException
-     * @throws SAXException
-     */
-    private boolean printAndReturnConsoleOfBuild(FreeStyleBuild build, String buildname) throws IOException, SAXException {
-        // this outputs loft of HTML garbage... so pretty printing after:
-        String console = jenkinsRule.createWebClient().getPage(build, "console").asXml();
-        System.out.println("************************************************************************");
-        System.out.println("* Relevant part of Jenkins build console (captured with regexp)");
-        System.out.println(String.format("* Build %s CONSOLE:", buildname));
-
-        // the pattern we want to search for
-        Pattern p = Pattern.compile("<link rel=\"stylesheet\" type=\"text/css\" href=\"/jenkins/descriptor/hudson.console.ExpandableDetailsNote/style.css\"/>"
-                + ".*<pre>(.*)</pre>.*</td>.*</tr>.*</tbody>.*</table>", Pattern.DOTALL);
-        Matcher m = p.matcher(console);
-        // if we find a match, get the group
-        if (m.find()) {
-            // get the matching group
-            String capturedText = m.group(1);
-
-            // print the group
-            System.out.format("'%s'\n", capturedText);
-            return true;
-        } else {
-            System.out.format("Didn't match any relevant part of the console");
-            return false;
-        }
-    }
-
     /**
      * Tests that we use the author of the last commit when integrating - using
      * the Squash strategy
@@ -110,7 +76,7 @@ public void customIntegrationBranchSquashStrategy() throws Exception {
 
         RunList<FreeStyleBuild> builds = project.getBuilds();
         for (FreeStyleBuild b : builds) {
-            assertTrue("Could not match console output for pretty printing", printAndReturnConsoleOfBuild(b, String.format("%s", b.getNumber())));
+            assertTrue("Could not match console output for pretty printing", TestUtilsFactory.printAndReturnConsoleOfBuild(b, String.format("%s", b.getNumber()), jenkinsRule));
             Result result = b.getResult();
             assertNotNull("Build result was null.", result);
             assertTrue("Build was not a success - that is expected in this test", result.isBetterOrEqualTo(Result.SUCCESS));
@@ -183,10 +149,10 @@ public void customIntegrationBranchSquashStrategy() throws Exception {
         assertTrue("The last commit on integration branch was not an accumulated commit. Didn't find the text 'Squashed commit'", commitFullMessage.contains("Squashed commit"));
         assertTrue("The squashed commit message, doesn't contain the SHA from the git log from the first of the included commits", commitFullMessage.contains("commit c1449e075f528974c63eef81109d0632eaada0c7"));
         assertTrue("The squashed commit message, didn't contain expected date string from the git log from the first of the included commits", commitFullMessage.contains("Wed Jun 3 14:03:46 2015 +0200"));
-        assertTrue("4", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
+        assertTrue("The squashed commit message, didn't contain part of the original commit messages.", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
         assertTrue("The squashed commit message, doesn't contain the SHA from the git log from the second of the included commits", commitFullMessage.contains("commit 70353ce6771866f29c38b4460b3f74f9024f8ce2"));
         assertTrue("The squashed commit message, didn't contain expected date string from the git log from the seond of the included commits", commitFullMessage.contains("Wed Jun 3 14:03:46 2015 +0200"));
-        assertTrue("7", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
+        assertTrue("The squashed commit message, didn't contain part of the original commit messages.", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
 
         // Verify that the collection and gathering of accumulated commit message doesn't collect much information
         // like commits from other branches:
@@ -223,7 +189,7 @@ public void customIntegrationBranchAccumulatedStrategy() throws Exception {
 
         RunList<FreeStyleBuild> builds = project.getBuilds();
         for (FreeStyleBuild b : builds) {
-            assertTrue("Could not match console output for pretty printing", printAndReturnConsoleOfBuild(b, String.format("%s", b.getNumber())));
+            assertTrue("Could not match console output for pretty printing", TestUtilsFactory.printAndReturnConsoleOfBuild(b, String.format("%s", b.getNumber()), jenkinsRule));
             Result result = b.getResult();
             assertNotNull("Build result was null.", result);
             assertTrue("Build was not a success - that is expected in this test", result.isBetterOrEqualTo(Result.SUCCESS));
@@ -299,10 +265,10 @@ public void customIntegrationBranchAccumulatedStrategy() throws Exception {
         assertTrue("The last commit on integration branch was not an accumulated commit. Didn't find the text 'Accumulated commit'", commitFullMessage.contains("Accumulated commit"));
         assertTrue("The accumulated commit message, doesn't contain the SHA from the git log from the first of the included commits", commitFullMessage.contains("commit c1449e075f528974c63eef81109d0632eaada0c7"));
         assertTrue("The accumulated commit message, didn't contain expected date string from the git log from the first of the included commits", commitFullMessage.contains("Wed Jun 3 14:03:46 2015 +0200"));
-        assertTrue("4", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
+        assertTrue("The squashed commit message, didn't contain part of the original commit messages.", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
         assertTrue("The accumulated commit message, doesn't contain the SHA from the git log from the second of the included commits", commitFullMessage.contains("commit 70353ce6771866f29c38b4460b3f74f9024f8ce2"));
         assertTrue("The accumulated commit message, didn't contain expected date string from the git log from the seond of the included commits", commitFullMessage.contains("Wed Jun 3 14:03:46 2015 +0200"));
-        assertTrue("7", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
+        assertTrue("The squashed commit message, didn't contain part of the original commit messages.", commitFullMessage.contains("Added a second line from myDevelopmentBranch in test commit log file."));
 
         // Verify that the collection and gathering of accumulated commit message doesn't collect much information
         // like commits from other branches:

diff --git a/...nkinsci/plugins/pretestedintegration/integration/scm/git/StaticGitRepositoryTestBase.java b/...nkinsci/plugins/pretestedintegration/integration/scm/git/StaticGitRepositoryTestBase.java
@@ -50,6 +50,8 @@ public StaticGitRepositoryTestBase() {
         testMethodName_vs_staticGitRepoName.put(    "commitMessagesWithDoubleQuotesAccumulatedLinux",           "commitMessagesWithDoubleQuotes_linux");
         testMethodName_vs_staticGitRepoName.put(    "commitMessagesWithDoubleQuotesSquashedWindows",            "commitMessagesWithDoubleQuotes_windows");
         testMethodName_vs_staticGitRepoName.put(    "commitMessagesWithDoubleQuotesAccumulatedWindows",         "commitMessagesWithDoubleQuotes_windows");
+        testMethodName_vs_staticGitRepoName.put(    "commitMessagesWithDoubleQuotesSingleQuotesMadeAccumulatedWindows",         "commitMessagesWithDoubleQuotesSingleQuotesMade_windows");
+        testMethodName_vs_staticGitRepoName.put(    "commitMessagesWithDoubleQuotesSingleQuotesMadeAccumulatedWindows_customerSuppliedRepo",         "JENKINS-28640");
         testMethodName_vs_staticGitRepoName.put(    "authorOfLastCommitUsedIfMoreThanOneCommitSquashStrategy",         "useAuthorOfLastCommit");
         testMethodName_vs_staticGitRepoName.put(    "authorOfLastCommitUsedIfMoreThanOneCommitAccumulatedStrategy",    "useAuthorOfLastCommit");
         testMethodName_vs_staticGitRepoName.put(    "customIntegrationBranchSquashStrategy",         "customIntegrationBranch");

diff --git a/...java/org/jenkinsci/plugins/pretestedintegration/integration/scm/git/TestUtilsFactory.java b/...java/org/jenkinsci/plugins/pretestedintegration/integration/scm/git/TestUtilsFactory.java
@@ -5,6 +5,7 @@
  */
 package org.jenkinsci.plugins.pretestedintegration.integration.scm.git;
 
+import hudson.model.FreeStyleBuild;
 import hudson.model.FreeStyleProject;
 import hudson.plugins.git.BranchSpec;
 import hudson.plugins.git.GitSCM;
@@ -24,6 +25,8 @@
 import java.io.FileReader;
 import java.io.IOException;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 import org.apache.commons.io.FileUtils;
@@ -43,6 +46,7 @@
 import org.jenkinsci.plugins.pretestedintegration.scm.git.GitBridge;
 import org.jenkinsci.plugins.pretestedintegration.scm.git.SquashCommitStrategy;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.xml.sax.SAXException;
 
 /**
  *
@@ -795,5 +799,39 @@ public static void unzipFunction(String destinationFolder, String zipFile) {
             e.printStackTrace();
         }
     }
+
+    /**
+     * Pretty prints console output from build log
+     *
+     * @param build
+     * @param buildname - descriptive build name included in the output
+     * @return boolean - true matched console like text, else false
+     * @throws IOException
+     * @throws SAXException
+     */
+    public static boolean printAndReturnConsoleOfBuild(FreeStyleBuild build, String buildname, JenkinsRule jenkinsRule) throws IOException, SAXException {
+        // this outputs loft of HTML garbage... so pretty printing after:
+        String console = jenkinsRule.createWebClient().getPage(build, "console").asXml();
+        System.out.println("************************************************************************");
+        System.out.println("* Relevant part of Jenkins build console (captured with regexp)");
+        System.out.println(String.format("* Build %s CONSOLE:", buildname));
+
+        // the pattern we want to search for
+        Pattern p = Pattern.compile("<link rel=\"stylesheet\" type=\"text/css\" href=\"/jenkins/descriptor/hudson.console.ExpandableDetailsNote/style.css\"/>"
+                + ".*<pre>(.*)</pre>.*</td>.*</tr>.*</tbody>.*</table>", Pattern.DOTALL);
+        Matcher m = p.matcher(console);
+        // if we find a match, get the group
+        if (m.find()) {
+            // get the matching group
+            String capturedText = m.group(1);
+
+            // print the group
+            System.out.format("'%s'\n", capturedText);
+            return true;
+        } else {
+            System.out.format("Didn't match any relevant part of the console");
+            return false;
+        }
+    }
 
 }
diff --git a/src/test/resources/JENKINS-28640.md b/src/test/resources/JENKINS-28640.md
@@ -0,0 +1,14 @@
+# Test repository with commits for issue JENKINS-28640
+
+Problem is double quotes in commit messages, are wrongly escaped in commands.
+
+Issue [JENKINS-28640 Quotationmarks in commit message leads to merge failure](https://issues.jenkins-ci.org/browse/JENKINS-28640) report a possible bug, if using double quotes in a commit message.
+
+An earlier issue, namely the [JENKINS-27662](https://issues.jenkins-ci.org/browse/JENKINS-27662) didn't find such problems. It can be related to either our test environment or the way we created the commit messges in those test repositories used in the functional tests.
+
+
+This repository was created by one of the users of the plugin, a customer of Praqma, and supplies a test example that can reproduce the problem.
+
+There is no script for reproducing the repository, it was created under Windows.
+
+The repository have been converted to a bare git repository, to use it with the tests.
diff --git a/src/test/resources/JENKINS-28640.zip b/src/test/resources/JENKINS-28640.zip
diff --git a/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade.md b/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade.md
@@ -0,0 +1,9 @@
+# Commit message with double quotes made with single quotes
+
+
+Issue [JENKINS-28640 Quotationmarks in commit message leads to merge failure](https://issues.jenkins-ci.org/browse/JENKINS-28640) report a possible bug, if using double quotes in a commit message.
+
+An earlier issue, namely the [JENKINS-27662](https://issues.jenkins-ci.org/browse/JENKINS-27662) didn't find such problems. It can be related to either our test environment or the way we created the commit messges in those test repositories used in the functional tests.
+
+
+A new set of test repositories have been created, but on different Windows environments for more variations.
diff --git a/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux-repo_description.log b/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux-repo_description.log
@@ -0,0 +1,21 @@
+# Repository view and commits
+
+Git version:
+git version 1.9.1
+
+After initial commit on master:
+-------------------------------
+* [31m97e8cea[m -[33m (HEAD, master)[m Initial commit - added README [32m(0 seconds ago) [1;34m<Praqma Support>[m
+
+After the two commit on dev branch:
+-----------------------------------
+* [31m7921dd6[m -[33m (HEAD, origin/dev/JENKINS-27662_doublequotes, dev/JENKINS-27662_doublequotes)[m This is a commit message with double quotes, and =, eg. "test quotes". [32m(0 seconds ago) [1;34m<Praqma Support>[m
+* [31m7d5b38a[m -[33m[m Added test commit log file [32m(0 seconds ago) [1;34m<Praqma Support>[m
+* [31m97e8cea[m -[33m (origin/master, master)[m Initial commit - added README [32m(0 seconds ago) [1;34m<Praqma Support>[m
+
+Final repository, view dev branch after push to ready-branch:
+------------------------------------------------------------------------------
+* [31m7921dd6[m -[33m (HEAD, origin/ready/JENKINS-27662_doublequotes, origin/dev/JENKINS-27662_doublequotes, dev/JENKINS-27662_doublequotes)[m This is a commit message with double quotes, and =, eg. "test quotes". [32m(0 seconds ago) [1;34m<Praqma Support>[m
+* [31m7d5b38a[m -[33m[m Added test commit log file [32m(0 seconds ago) [1;34m<Praqma Support>[m
+* [31m97e8cea[m -[33m (origin/master, master)[m Initial commit - added README [32m(0 seconds ago) [1;34m<Praqma Support>[m
+
diff --git a/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux.sh b/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux.sh
@@ -0,0 +1,84 @@
+#!/bin/bash
+
+# setting names and stuff
+if [ -z "$1" ]; then
+	VERSION=""
+else
+	VERSION="_$1"
+fi
+NAME=commitMessagesWithDoubleQuotesSingleQuotesMade_linux
+REPO_NAME=$NAME$VERSION # used for manual testing of script and re-runs
+WORK_DIR=`pwd`
+
+LOG=$WORK_DIR/$REPO_NAME-repo_description.log
+echo "# Repository view and commits" >> $LOG
+echo "" >> $LOG
+echo "Git version:" >> $LOG
+git --version >> $LOG
+echo "" >> $LOG
+
+
+mkdir -v $REPO_NAME.git
+cd $REPO_NAME.git
+git init --bare
+
+cd $WORK_DIR
+git clone $REPO_NAME.git
+cd $REPO_NAME
+git config user.name "Praqma Support"
+git config user.email "support@praqma.net"
+
+touch README.md
+echo "# README of repository $REPO_NAME" >> README.md
+echo "" >> README.md
+echo "This is a test repository for functional tests." >> README.md
+git add README.md
+git commit -m "Initial commit - added README"
+
+
+echo "After initial commit on master:" >> $LOG
+echo "-------------------------------" >> $LOG
+git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative >> $LOG
+echo "" >> $LOG
+echo "" >> $LOG
+
+git push origin master
+
+# custom parts
+BN=JENKINS-27662_doublequotes
+git checkout -b dev/$BN
+touch testCommit.log
+echo "# Test commit log" >> testCommit.log
+echo "" >> testCommit.log
+echo "Used for adding lines to commit something during tests.\n" >> testCommit.log
+git add testCommit.log
+git commit -m "Added test commit log file"
+
+# Problematic commit message with double quotes
+
+echo "Added a new line to this file, to commit something. Commit message will have double quotes" >> testCommit.log
+git add testCommit.log
+git commit -m 'This is a commit message with double quotes, and =, eg. "test quotes".'
+git push origin dev/$BN
+
+echo "After the two commit on dev branch:" >> $LOG
+echo "-----------------------------------" >> $LOG
+git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative >> $LOG
+echo "" >> $LOG
+echo "" >> $LOG
+
+# also push to ready branch, so integration can start during the test
+git push origin dev/$BN:ready/$BN
+
+echo "Final repository, view dev branch after push to ready-branch:" >> $LOG
+echo "------------------------------------------------------------------------------" >> $LOG
+git log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr) %C(bold blue)<%an>%Creset' --abbrev-commit --date=relative >> $LOG
+echo "" >> $LOG
+echo "" >> $LOG
+
+# Post process
+
+cd $WORK_DIR
+zip -r $NAME$VERSION.zip $REPO_NAME.git
+rm -rf $REPO_NAME.git $REPO_NAME
+
diff --git a/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux.zip b/src/test/resources/commitMessagesWithDoubleQuotesSingleQuotesMade_linux.zip