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>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/CommitBuilderTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/CommitBuilderTest.java
index 26294c7714f84fe716d6f33564ed46efed74e89d..dee58f9cfcdc2e2568b4447f09117f98859382b1 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/CommitBuilderTest.java
+++ b/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,6 +24,32 @@ import org.junit.Test;
 
 public class CommitBuilderTest {
 
+       // @formatter:off
+       private static final String SIGNATURE = "-----BEGIN PGP SIGNATURE-----\n" +
+                       "Version: BCPG v1.60\n" +
+                       "\n" +
+                       "iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
+                       "opvBwqyt6OX8lgNwTwBGHFNt8LdmDCCmKoq/XwkNi3ARVjLhe3gBcKXNoavvPk2Z\n" +
+                       "gIg5ChevGkU4afWCOMLVEYnkCBGw2+86XhrK1P7gTHEk1Rd+Yv1ZRDJBY+fFO7yz\n" +
+                       "uSBuF5RpEY2sJiIvp27Gub/rY3B5NTR/feO/z+b9oiP/fMUhpRwG5KuWUsn9NPjw\n" +
+                       "3tvbgawYpU/2UnS+xnavMY4t2fjRYjsoxndPLb2MUX8X7vC7FgWLBlmI/rquLZVM\n" +
+                       "IQEKkjnA+lhejjK1rv+ulq4kGZJFKGYWYYhRDwFg5PTkzhudhN2SGUq5Wxq1Eg4=\n" +
+                       "=b9OI\n" +
+                       "-----END PGP SIGNATURE-----";
+
+       private static final String EXPECTED = "-----BEGIN PGP SIGNATURE-----\n" +
+                       " Version: BCPG v1.60\n" +
+                       " \n" +
+                       " iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
+                       " opvBwqyt6OX8lgNwTwBGHFNt8LdmDCCmKoq/XwkNi3ARVjLhe3gBcKXNoavvPk2Z\n" +
+                       " gIg5ChevGkU4afWCOMLVEYnkCBGw2+86XhrK1P7gTHEk1Rd+Yv1ZRDJBY+fFO7yz\n" +
+                       " uSBuF5RpEY2sJiIvp27Gub/rY3B5NTR/feO/z+b9oiP/fMUhpRwG5KuWUsn9NPjw\n" +
+                       " 3tvbgawYpU/2UnS+xnavMY4t2fjRYjsoxndPLb2MUX8X7vC7FgWLBlmI/rquLZVM\n" +
+                       " IQEKkjnA+lhejjK1rv+ulq4kGZJFKGYWYYhRDwFg5PTkzhudhN2SGUq5Wxq1Eg4=\n" +
+                       " =b9OI\n" +
+                       " -----END PGP SIGNATURE-----";
+       // @formatter:on
+
        private void assertGpgSignatureStringOutcome(String signature,
                        String expectedOutcome) throws IOException {
                ByteArrayOutputStream out = new ByteArrayOutputStream();
@@ -33,47 +59,37 @@ public class CommitBuilderTest {
        }
 
        @Test
-       public void writeGpgSignatureString_1() throws Exception {
-               // @formatter:off
-               String signature = "-----BEGIN PGP SIGNATURE-----\n" +
-                               "Version: BCPG v1.60\n" +
-                               "\n" +
-                               "iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
-                               "opvBwqyt6OX8lgNwTwBGHFNt8LdmDCCmKoq/XwkNi3ARVjLhe3gBcKXNoavvPk2Z\n" +
-                               "gIg5ChevGkU4afWCOMLVEYnkCBGw2+86XhrK1P7gTHEk1Rd+Yv1ZRDJBY+fFO7yz\n" +
-                               "uSBuF5RpEY2sJiIvp27Gub/rY3B5NTR/feO/z+b9oiP/fMUhpRwG5KuWUsn9NPjw\n" +
-                               "3tvbgawYpU/2UnS+xnavMY4t2fjRYjsoxndPLb2MUX8X7vC7FgWLBlmI/rquLZVM\n" +
-                               "IQEKkjnA+lhejjK1rv+ulq4kGZJFKGYWYYhRDwFg5PTkzhudhN2SGUq5Wxq1Eg4=\n" +
-                               "=b9OI\n" +
-                               "-----END PGP SIGNATURE-----";
-               String expectedOutcome = "-----BEGIN PGP SIGNATURE-----\n" +
-                               " Version: BCPG v1.60\n" +
-                               " \n" +
-                               " iQEcBAABCAAGBQJb9cVhAAoJEKX+6Axg/6TZeFsH/0CY0WX/z7U8+7S5giFX4wH4\n" +
-                               " opvBwqyt6OX8lgNwTwBGHFNt8LdmDCCmKoq/XwkNi3ARVjLhe3gBcKXNoavvPk2Z\n" +
-                               " gIg5ChevGkU4afWCOMLVEYnkCBGw2+86XhrK1P7gTHEk1Rd+Yv1ZRDJBY+fFO7yz\n" +
-                               " uSBuF5RpEY2sJiIvp27Gub/rY3B5NTR/feO/z+b9oiP/fMUhpRwG5KuWUsn9NPjw\n" +
-                               " 3tvbgawYpU/2UnS+xnavMY4t2fjRYjsoxndPLb2MUX8X7vC7FgWLBlmI/rquLZVM\n" +
-                               " IQEKkjnA+lhejjK1rv+ulq4kGZJFKGYWYYhRDwFg5PTkzhudhN2SGUq5Wxq1Eg4=\n" +
-                               " =b9OI\n" +
-                               " -----END PGP SIGNATURE-----";
-               // @formatter:on
-               assertGpgSignatureStringOutcome(signature, expectedOutcome);
+       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 {
-                       CommitBuilder.writeGpgSignatureString(signature,
-                                       new ByteArrayOutputStream());
-                       fail("Exception expected");
-               } catch (IllegalArgumentException e) {
-                       // good
-                       String message = MessageFormat.format(JGitText.get().notASCIIString,
-                                       signature);
-                       assertEquals(message, e.getMessage());
-               }
+               IllegalArgumentException e = assertThrows(
+                               IllegalArgumentException.class,
+                               () -> CommitBuilder.writeGpgSignatureString(signature,
+                                               new ByteArrayOutputStream()));
+               String message = MessageFormat.format(JGitText.get().notASCIIString,
+                               signature);
+               assertEquals(message, e.getMessage());
        }
 
        @Test

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java
index 66d7d51bdff0cc2fd4386b96ae4754ef81a2608c..4f93fda49fec46fbc1465050a85195a6c070afbd 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/CommitBuilder.java
+++ b/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') {
-                                       out.write('\n');
-                                       out.write(' ');
+                               if (i + 1 < length && in.charAt(i + 1) == '\n') {
                                        ++i;
-                               } else {
+                               }
+                               if (i + 1 < length) {
                                        out.write('\n');
                                        out.write(' ');
                                }
                                break;
                        case '\n':
-                               out.write('\n');
-                               out.write(' ');
+                               if (i + 1 < length) {
+                                       out.write('\n');
+                                       out.write(' ');
+                               }
                                break;
                        default:
                                // sanity check