Fix writing GPG signatures with trailing newline

Make sure we don't produce a spurious empty line at the end.

Bug: 564428
Change-Id: Ib991d93fbd052baca65d32a7842f07f9ddeb8130
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/CommitBuilderTest.java

@@ -13,7 +13,7 @@ import static java.nio.charset.StandardCharsets.US_ASCII;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertThrows;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -24,18 +24,8 @@ import org.junit.Test;
 
 public class CommitBuilderTest {
 
-	private void assertGpgSignatureStringOutcome(String signature,
-			String expectedOutcome) throws IOException {
-		ByteArrayOutputStream out = new ByteArrayOutputStream();
-		CommitBuilder.writeGpgSignatureString(signature, out);
-		String formatted_signature = new String(out.toByteArray(), US_ASCII);
-		assertEquals(expectedOutcome, formatted_signature);
-	}
-
-	@Test
-	public void writeGpgSignatureString_1() throws Exception {
 	// @formatter:off
-		String signature = "-----BEGIN PGP SIGNATURE-----\n" +
+	private static final String SIGNATURE = "-----BEGIN PGP SIGNATURE-----\n" +
 			"Version: BCPG v1.60\n" +
 			"\n" +
 			"iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
@@ -46,7 +36,8 @@ public class CommitBuilderTest {
 			"IQEKkjnA+lhejjK1rv+ulq4kGZJFKGYWYYhRDwFg5PTkzhudhN2SGUq5Wxq1Eg4=\n" +
 			"=b9OI\n" +
 			"-----END PGP SIGNATURE-----";
-		String expectedOutcome = "-----BEGIN PGP SIGNATURE-----\n" +
+
+	private static final String EXPECTED = "-----BEGIN PGP SIGNATURE-----\n" +
 			" Version: BCPG v1.60\n" +
 			" \n" +
 			" iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
@@ -58,23 +49,48 @@ public class CommitBuilderTest {
 			" =b9OI\n" +
 			" -----END PGP SIGNATURE-----";
 	// @formatter:on
-		assertGpgSignatureStringOutcome(signature, expectedOutcome);
+
+	private void assertGpgSignatureStringOutcome(String signature,
+			String expectedOutcome) throws IOException {
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		CommitBuilder.writeGpgSignatureString(signature, out);
+		String formatted_signature = new String(out.toByteArray(), US_ASCII);
+		assertEquals(expectedOutcome, formatted_signature);
+	}
+
+	@Test
+	public void writeGpgSignatureString() throws Exception {
+		assertGpgSignatureStringOutcome(SIGNATURE, EXPECTED);
+	}
+
+	@Test
+	public void writeGpgSignatureStringTrailingLF() throws Exception {
+		assertGpgSignatureStringOutcome(SIGNATURE + '\n', EXPECTED);
+	}
+
+	@Test
+	public void writeGpgSignatureStringCRLF() throws Exception {
+		assertGpgSignatureStringOutcome(SIGNATURE.replaceAll("\n", "\r\n"),
+				EXPECTED);
+	}
+
+	@Test
+	public void writeGpgSignatureStringTrailingCRLF() throws Exception {
+		assertGpgSignatureStringOutcome(
+				SIGNATURE.replaceAll("\n", "\r\n") + "\r\n", EXPECTED);
 	}
 
 	@Test
 	public void writeGpgSignatureString_failsForNonAscii() throws Exception {
 		String signature = "Ü Ä";
-		try {
+		IllegalArgumentException e = assertThrows(
-			CommitBuilder.writeGpgSignatureString(signature,
+				IllegalArgumentException.class,
-					new ByteArrayOutputStream());
+				() -> CommitBuilder.writeGpgSignatureString(signature,
-			fail("Exception expected");
+						new ByteArrayOutputStream()));
-		} catch (IllegalArgumentException e) {
-			// good
 		String message = MessageFormat.format(JGitText.get().notASCIIString,
 				signature);
 		assertEquals(message, e.getMessage());
 	}
-	}
 
 	@Test
 	public void writeGpgSignatureString_oneLineNotModified() throws Exception {

org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java

@@ -361,7 +361,9 @@ public class CommitBuilder {
 	 * header</a>.
 	 * <p>
 	 * CRLF and CR will be sanitized to LF and signature will have a hanging
-	 * indent of one space starting with line two.
+	 * indent of one space starting with line two. A trailing line break is
+	 * <em>not</em> written; the caller is supposed to terminate the GPG
+	 * signature header by writing a single newline.
 	 * </p>
 	 *
 	 * @param in
@@ -375,22 +377,24 @@ public class CommitBuilder {
 	 */
 	static void writeGpgSignatureString(String in, OutputStream out)
 			throws IOException, IllegalArgumentException {
-		for (int i = 0; i < in.length(); ++i) {
+		int length = in.length();
+		for (int i = 0; i < length; ++i) {
 			char ch = in.charAt(i);
 			switch (ch) {
 			case '\r':
-				if (i + 1 < in.length() && in.charAt(i + 1) == '\n') {
+				if (i + 1 < length && in.charAt(i + 1) == '\n') {
-					out.write('\n');
-					out.write(' ');
 					++i;
-				} else {
+				}
+				if (i + 1 < length) {
 					out.write('\n');
 					out.write(' ');
 				}
 				break;
 			case '\n':
+				if (i + 1 < length) {
 					out.write('\n');
 					out.write(' ');
+				}
 				break;
 			default:
 				// sanity check