From 9b033a1b6da1193dbedca80c217af748ae546629 Mon Sep 17 00:00:00 2001 From: Thomas Wolf Date: Thu, 18 Jun 2020 18:27:45 +0200 Subject: [PATCH] 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 --- .../eclipse/jgit/lib/CommitBuilderTest.java | 90 +++++++++++-------- .../org/eclipse/jgit/lib/CommitBuilder.java | 20 +++-- 2 files changed, 65 insertions(+), 45 deletions(-) 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 26294c771..dee58f9cf 100644 --- 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 66d7d51bd..4f93fda49 100644 --- 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. *

* 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 + * not written; the caller is supposed to terminate the GPG + * signature header by writing a single newline. *

* * @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