diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java index 1b3519206..7578c6e3e 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java @@ -51,13 +51,6 @@ import org.junit.Test; /** Tests for base receive-pack utilities. */ public class BaseReceivePackTest { - @Test - public void chomp() { - assertEquals("foo", BaseReceivePack.chomp("foo")); - assertEquals("foo", BaseReceivePack.chomp("foo\n")); - assertEquals("foo\n", BaseReceivePack.chomp("foo\n\n")); - } - @Test public void parseCommand() throws Exception { String o = "0000000000000000000000000000000000000000"; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java index 6e49f47ff..8fdf386ec 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java @@ -87,6 +87,30 @@ public class PushCertificateParserTest { + "0020-----END PGP SIGNATURE-----\n" + "0012push-cert-end\n"; + // Same push certificate, with all trailing newlines stripped. + // (Note that the canonical signed payload is the same, so the same signature + // is still valid.) + private static final String INPUT_NO_NEWLINES = "001bcertificate version 0.1" + + "0040pusher Dave Borowitz 1433954361 -0700" + + "0023pushee git://localhost/repo.git" + + "0029nonce 1433954361-bde756572d665bba81d8" + + "0004" + + "00670000000000000000000000000000000000000000" + + " 6c2b981a177396fb47345b7df3e4d3f854c6bea7" + + " refs/heads/master" + + "0021-----BEGIN PGP SIGNATURE-----" + + "0015Version: GnuPG v1" + + "0004" + + "0044iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa" + + "00449tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7" + + "0044htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V" + + "00444ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG" + + "0044IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY" + + "0044+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=" + + "0009=XFeC" + + "001f-----END PGP SIGNATURE-----" + + "0011push-cert-end"; + private Repository db; @Before @@ -115,11 +139,10 @@ public class PushCertificateParserTest { ObjectId newId = ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); String line = oldId.name() + " " + newId.name() + " refs/heads/master"; - String rawLine = line + "\n"; - ReceiveCommand cmd = BaseReceivePack.parseCommand(line); - parser.addCommand(cmd, rawLine); - parser.addCommand(rawLine); + + parser.addCommand(cmd); + parser.addCommand(line); assertNull(parser.build()); } @@ -132,8 +155,8 @@ public class PushCertificateParserTest { assertNull(parser.build()); parser.receiveHeader(pckIn, false); - parser.addCommand(pckIn.readStringRaw()); - assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw()); + parser.addCommand(pckIn.readString()); + assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readString()); parser.receiveSignature(pckIn); assertNull(parser.build()); } @@ -162,8 +185,46 @@ public class PushCertificateParserTest { PushCertificateParser parser = new PushCertificateParser(db, newEnabledConfig()); parser.receiveHeader(pckIn, false); - parser.addCommand(pckIn.readStringRaw()); - assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw()); + parser.addCommand(pckIn.readString()); + assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readString()); + parser.receiveSignature(pckIn); + + PushCertificate cert = parser.build(); + assertEquals("0.1", cert.getVersion()); + assertEquals("Dave Borowitz", cert.getPusherIdent().getName()); + assertEquals("dborowitz@google.com", + cert.getPusherIdent().getEmailAddress()); + assertEquals(1433954361000L, cert.getPusherIdent().getWhen().getTime()); + assertEquals(-7 * 60, cert.getPusherIdent().getTimeZoneOffset()); + assertEquals("git://localhost/repo.git", cert.getPushee()); + assertEquals("1433954361-bde756572d665bba81d8", cert.getNonce()); + + assertNotEquals(cert.getNonce(), parser.getAdvertiseNonce()); + assertEquals(PushCertificate.NonceStatus.BAD, cert.getNonceStatus()); + + assertEquals(1, cert.getCommands().size()); + ReceiveCommand cmd = cert.getCommands().get(0); + assertEquals("refs/heads/master", cmd.getRefName()); + assertEquals(ObjectId.zeroId(), cmd.getOldId()); + assertEquals("6c2b981a177396fb47345b7df3e4d3f854c6bea7", + cmd.getNewId().name()); + + assertEquals(concatPacketLines(INPUT, 0, 6), cert.toText()); + + String signature = concatPacketLines(INPUT, 6, 17); + assertTrue(signature.startsWith(PushCertificateParser.BEGIN_SIGNATURE)); + assertTrue(signature.endsWith(PushCertificateParser.END_SIGNATURE + "\n")); + assertEquals(signature, cert.getSignature()); + } + + @Test + public void parseCertFromPktLineNoNewlines() throws Exception { + PacketLineIn pckIn = newPacketLineIn(INPUT_NO_NEWLINES); + PushCertificateParser parser = + new PushCertificateParser(db, newEnabledConfig()); + parser.receiveHeader(pckIn, false); + parser.addCommand(pckIn.readString()); + assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readString()); parser.receiveSignature(pckIn); PushCertificate cert = parser.build(); @@ -186,11 +247,12 @@ public class PushCertificateParserTest { assertEquals("6c2b981a177396fb47345b7df3e4d3f854c6bea7", cmd.getNewId().name()); + // Canonical signed payload has reinserted newlines. assertEquals(concatPacketLines(INPUT, 0, 6), cert.toText()); String signature = concatPacketLines(INPUT, 6, 17); assertTrue(signature.startsWith(PushCertificateParser.BEGIN_SIGNATURE)); - assertTrue(signature.endsWith(PushCertificateParser.END_SIGNATURE)); + assertTrue(signature.endsWith(PushCertificateParser.END_SIGNATURE + "\n")); assertEquals(signature, cert.getSignature()); } @@ -203,6 +265,15 @@ public class PushCertificateParserTest { assertEquals("line 2\nline 3\n", concatPacketLines(input, 1, 4)); } + @Test + public void testConcatPacketLinesInsertsNewlines() throws Exception { + String input = "000bline 1\n000aline 2000bline 3\n"; + assertEquals("line 1\n", concatPacketLines(input, 0, 1)); + assertEquals("line 1\nline 2\n", concatPacketLines(input, 0, 2)); + assertEquals("line 2\nline 3\n", concatPacketLines(input, 1, 3)); + assertEquals("line 2\nline 3\n", concatPacketLines(input, 1, 4)); + } + private static String concatPacketLines(String input, int begin, int end) throws IOException { StringBuilder result = new StringBuilder(); @@ -211,12 +282,12 @@ public class PushCertificateParserTest { while (i < end) { String line; try { - line = pckIn.readStringRaw(); + line = pckIn.readString(); } catch (EOFException e) { break; } if (++i > begin) { - result.append(line); + result.append(line).append('\n'); } } return result.toString(); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 819f77c06..36d335503 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1066,18 +1066,17 @@ public abstract class BaseReceivePack { PushCertificateParser certParser = getPushCertificateParser(); FirstLine firstLine = null; for (;;) { - String rawLine; + String line; try { - rawLine = pckIn.readStringRaw(); + line = pckIn.readString(); } catch (EOFException eof) { if (commands.isEmpty()) return; throw eof; } - if (rawLine == PacketLineIn.END) { + if (line == PacketLineIn.END) { break; } - String line = chomp(rawLine); if (line.length() >= 48 && line.startsWith("shallow ")) { //$NON-NLS-1$ clientShallowCommits.add(ObjectId.fromString(line.substring(8, 48))); @@ -1095,7 +1094,7 @@ public abstract class BaseReceivePack { } } - if (rawLine.equals(PushCertificateParser.BEGIN_SIGNATURE)) { + if (line.equals(PushCertificateParser.BEGIN_SIGNATURE)) { certParser.receiveSignature(pckIn); continue; } @@ -1114,21 +1113,11 @@ public abstract class BaseReceivePack { } commands.add(cmd); if (certParser.enabled()) { - // Must use raw line with optional newline so signed payload can be - // reconstructed. - certParser.addCommand(cmd, rawLine); + certParser.addCommand(cmd); } } } - static String chomp(String line) { - if (line != null && !line.isEmpty() - && line.charAt(line.length() - 1) == '\n') { - return line.substring(0, line.length() - 1); - } - return line; - } - static ReceiveCommand parseCommand(String line) throws PackProtocolException { if (line == null || line.length() < 83) { throw new PackProtocolException( diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java index 6dc4153d1..fdc70adc8 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java @@ -85,12 +85,11 @@ public class PushCertificate { private final String nonce; private final NonceStatus nonceStatus; private final List commands; - private final String rawCommands; private final String signature; PushCertificate(String version, PushCertificateIdent pusher, String pushee, String nonce, NonceStatus nonceStatus, List commands, - String rawCommands, String signature) { + String signature) { if (version == null || version.isEmpty()) { throw new IllegalArgumentException(MessageFormat.format( JGitText.get().pushCertificateInvalidField, VERSION)); @@ -112,8 +111,7 @@ public class PushCertificate { JGitText.get().pushCertificateInvalidField, "nonce status")); //$NON-NLS-1$ } - if (commands == null || commands.isEmpty() - || rawCommands == null || rawCommands.isEmpty()) { + if (commands == null || commands.isEmpty()) { throw new IllegalArgumentException(MessageFormat.format( JGitText.get().pushCertificateInvalidField, "command")); //$NON-NLS-1$ @@ -123,7 +121,7 @@ public class PushCertificate { JGitText.get().pushCertificateInvalidSignature); } if (!signature.startsWith(PushCertificateParser.BEGIN_SIGNATURE) - || !signature.endsWith(PushCertificateParser.END_SIGNATURE)) { + || !signature.endsWith(PushCertificateParser.END_SIGNATURE + '\n')) { throw new IllegalArgumentException( JGitText.get().pushCertificateInvalidSignature); } @@ -133,7 +131,6 @@ public class PushCertificate { this.nonce = nonce; this.nonceStatus = nonceStatus; this.commands = commands; - this.rawCommands = rawCommands; this.signature = signature; } @@ -209,14 +206,18 @@ public class PushCertificate { * @since 4.1 */ public String toText() { - return new StringBuilder() + StringBuilder sb = new StringBuilder() .append(VERSION).append(' ').append(version).append('\n') .append(PUSHER).append(' ').append(getPusher()) .append('\n') .append(PUSHEE).append(' ').append(pushee).append('\n') .append(NONCE).append(' ').append(nonce).append('\n') - .append('\n') - .append(rawCommands) - .toString(); + .append('\n'); + for (ReceiveCommand cmd : commands) { + sb.append(cmd.getOldId().name()) + .append(' ').append(cmd.getNewId().name()) + .append(' ').append(cmd.getRefName()).append('\n'); + } + return sb.toString(); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java index 661a0f094..b34fb9fc6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java @@ -42,7 +42,6 @@ */ package org.eclipse.jgit.transport; -import static org.eclipse.jgit.transport.BaseReceivePack.chomp; import static org.eclipse.jgit.transport.BaseReceivePack.parseCommand; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_PUSH_CERT; @@ -66,9 +65,9 @@ import org.eclipse.jgit.transport.PushCertificate.NonceStatus; */ public class PushCertificateParser { static final String BEGIN_SIGNATURE = - "-----BEGIN PGP SIGNATURE-----\n"; //$NON-NLS-1$ + "-----BEGIN PGP SIGNATURE-----"; //$NON-NLS-1$ static final String END_SIGNATURE = - "-----END PGP SIGNATURE-----\n"; //$NON-NLS-1$ + "-----END PGP SIGNATURE-----"; //$NON-NLS-1$ static final String VERSION = "certificate version"; //$NON-NLS-1$ @@ -80,7 +79,7 @@ public class PushCertificateParser { private static final String VERSION_0_1 = "0.1"; //$NON-NLS-1$ - private static final String END_CERT = "push-cert-end\n"; //$NON-NLS-1$ + private static final String END_CERT = "push-cert-end"; //$NON-NLS-1$ private boolean received; private String version; @@ -112,7 +111,6 @@ public class PushCertificateParser { private final NonceGenerator nonceGenerator; private final List commands; - private final StringBuilder rawCommands; PushCertificateParser(Repository into, SignedPushConfig cfg) { if (cfg != null) { @@ -124,7 +122,6 @@ public class PushCertificateParser { } db = into; commands = new ArrayList<>(); - rawCommands = new StringBuilder(); } /** @@ -139,8 +136,7 @@ public class PushCertificateParser { } try { return new PushCertificate(version, pusher, pushee, receivedNonce, - nonceStatus, Collections.unmodifiableList(commands), - rawCommands.toString(), signature); + nonceStatus, Collections.unmodifiableList(commands), signature); } catch (IllegalArgumentException e) { throw new IOException(e.getMessage(), e); } @@ -244,9 +240,9 @@ public class PushCertificateParser { * Read the PGP signature. *

* This method assumes the line - * {@code "-----BEGIN PGP SIGNATURE-----\n"} has already been parsed, - * and continues parsing until an {@code "-----END PGP SIGNATURE-----\n"} is - * found, followed by {@code "push-cert-end\n"}. + * {@code "-----BEGIN PGP SIGNATURE-----"} has already been parsed, + * and continues parsing until an {@code "-----END PGP SIGNATURE-----"} is + * found, followed by {@code "push-cert-end"}. * * @param pckIn * where we read the signature from. @@ -257,13 +253,13 @@ public class PushCertificateParser { public void receiveSignature(PacketLineIn pckIn) throws IOException { received = true; try { - StringBuilder sig = new StringBuilder(BEGIN_SIGNATURE); + StringBuilder sig = new StringBuilder(BEGIN_SIGNATURE).append('\n'); String line; - while (!(line = pckIn.readStringRaw()).equals(END_SIGNATURE)) { - sig.append(line); + while (!(line = pckIn.readString()).equals(END_SIGNATURE)) { + sig.append(line).append('\n'); } - signature = sig.append(END_SIGNATURE).toString(); - if (!pckIn.readStringRaw().equals(END_CERT)) { + signature = sig.append(END_SIGNATURE).append('\n').toString(); + if (!pckIn.readString().equals(END_CERT)) { throw new PackProtocolException( JGitText.get().pushCertificateInvalidSignature); } @@ -278,28 +274,23 @@ public class PushCertificateParser { * * @param cmd * the command. - * @param rawLine - * the exact line read from the wire that produced this - * command, including trailing newline if present. * @since 4.1 */ - public void addCommand(ReceiveCommand cmd, String rawLine) { + public void addCommand(ReceiveCommand cmd) { commands.add(cmd); - rawCommands.append(rawLine); } /** * Add a command to the signature. * - * @param rawLine - * the exact line read from the wire that produced this - * command, including trailing newline if present. + * @param line + * the line read from the wire that produced this + * command, with optional trailing newline already trimmed. * @throws PackProtocolException * if the raw line cannot be parsed to a command. * @since 4.0 */ - public void addCommand(String rawLine) throws PackProtocolException { - commands.add(parseCommand(chomp(rawLine))); - rawCommands.append(rawLine); + public void addCommand(String line) throws PackProtocolException { + commands.add(parseCommand(line)); } }