Merge "Accept Change-Id even if footer contains not well-formed entries" into stable-2.3

Author: msohn

org.eclipse.jgit.test/tst/org/eclipse/jgit/util/ChangeIdUtilTest.java

@@ -642,29 +642,58 @@ public void testIndexOfChangeId() {
 		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
 				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
 				"\n"));
+		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n\n\n",
+				"\n"));
+		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+										+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n \n \n",
+								"\n"));
+		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+				"\n"));
+
+		// leading whitespace is rejected by Gerrit
+		assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ " Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+				"\n"));
+		assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "\t Change-Id:  I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+				"\n"));
+
+		assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "Change-Id: \n", "\n"));
+		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701 \n",
+				"\n"));
+		assertEquals(12, ChangeIdUtil.indexOfChangeId("x\n" + "\n"
+				+ "Bug 4711\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+				"\n"));
+		assertEquals(56, ChangeIdUtil.indexOfChangeId("x\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+				+ "\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
+				"\n"));
+		assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+				+ "\n" + "x\n", "\n"));
+		assertEquals(-1, ChangeIdUtil.indexOfChangeId("x\n\n"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n"
+				+ "\n" + "x\n", "\n"));
 		assertEquals(5, ChangeIdUtil.indexOfChangeId("x\r\n" + "\r\n"
 				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r\n",
 				"\r\n"));
 		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
 				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
 				"\r"));
+		assertEquals(3, ChangeIdUtil.indexOfChangeId("x\r" + "\r"
+				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\r",
+				"\r"));
 		assertEquals(8, ChangeIdUtil.indexOfChangeId("x\ny\n\nz\n" + "\n"
 				+ "Change-Id: I3b7e4e16b503ce00f07ba6ad01d97a356dad7701\n",
 				"\n"));
 	}
 
-	@Test
-	public void testIndexOfFirstFooterLine() {
-		assertEquals(
-				2,
-				ChangeIdUtil.indexOfFirstFooterLine(new String[] { "a", "",
-						"Bug: 42", "Signed-Off-By: j.developer@a.com" }));
-		assertEquals(
-				3,
-				ChangeIdUtil.indexOfFirstFooterLine(new String[] { "a",
-						"Bug: 42", "", "Signed-Off-By: j.developer@a.com" }));
-	}
-
 	private void hookDoesNotModify(final String in) throws Exception {
 		assertEquals(in, call(in));
 	}

org.eclipse.jgit/src/org/eclipse/jgit/util/ChangeIdUtil.java

@@ -125,9 +125,14 @@ public static ObjectId computeChangeId(final ObjectId treeId,
 	private static final Pattern footerPattern = Pattern
 			.compile("(^[a-zA-Z0-9-]+:(?!//).*$)"); //$NON-NLS-1$
 
+	private static final Pattern changeIdPattern = Pattern
+			.compile("(^" + CHANGE_ID + " *I[a-f0-9]{40}$)"); //$NON-NLS-1$ //$NON-NLS-2$
+
 	private static final Pattern includeInFooterPattern = Pattern
 			.compile("^[ \\[].*$"); //$NON-NLS-1$
 
+	private static final Pattern trailingWhitespace = Pattern.compile("\\s+$");
+
 	/**
 	 * Find the right place to insert a Change-Id and return it.
 	 * <p>
@@ -209,8 +214,11 @@ public static String insertId(String message, ObjectId changeId,
 	}
 
 	/**
-	 * Find the index in the String {@code} message} where the Change-Id entry
-	 * begins
+	 * Return the index in the String {@code message} where the Change-Id entry
+	 * in the footer begins. If there are more than one entries matching the
+	 * pattern, return the index of the last one in the last section. Because of
+	 * Bug: 400818 we release the constraint here that a footer must contain
+	 * only lines matching {@code footerPattern}.
 	 *
 	 * @param message
 	 * @param delimiter
@@ -221,14 +229,32 @@ public static String insertId(String message, ObjectId changeId,
 	 */
 	public static int indexOfChangeId(String message, String delimiter) {
 		String[] lines = message.split(delimiter);
-		int footerFirstLine = indexOfFirstFooterLine(lines);
-		if (footerFirstLine == lines.length)
-			return -1;
+		int indexOfChangeIdLine = 0;
+		boolean inFooter = false;
+		for (int i = lines.length - 1; i >= 0; --i) {
+			if (!inFooter && isEmptyLine(lines[i]))
+				continue;
+			inFooter = true;
+			if (changeIdPattern.matcher(trimRight(lines[i])).matches()) {
+				indexOfChangeIdLine = i;
+				break;
+			} else if (isEmptyLine(lines[i]) || i == 0)
+				return -1;
+		}
+		int indexOfChangeIdLineinString = 0;
+		for (int i = 0; i < indexOfChangeIdLine; ++i)
+			indexOfChangeIdLineinString += lines[i].length()
+					+ delimiter.length();
+		return indexOfChangeIdLineinString
+				+ lines[indexOfChangeIdLine].indexOf(CHANGE_ID);
+	}
+
+	private static boolean isEmptyLine(String line) {
+		return line.trim().length() == 0;
+	}
 
-		int indexOfFooter = 0;
-		for (int i = 0; i < footerFirstLine; ++i)
-			indexOfFooter += lines[i].length() + delimiter.length();
-		return message.indexOf(CHANGE_ID, indexOfFooter);
+	private static String trimRight(String s) {
+		return trailingWhitespace.matcher(s).replaceAll(""); //$NON-NLS-1$
 	}
 
 	/**