PackCertificateParser: return null if nothing was received

Add test for this case in both the enabled and disabled cases.

Change-Id: If9d12192a2dc9f9dd1eac9844b5a7b0edadc0b34

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 7de933396795b05449e196d2c7b21ece0c583db8..9c157c3379133119884444e83e4a6a300d8fa4a3 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java
@@ -43,55 +43,124 @@
 package org.eclipse.jgit.transport;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.ByteArrayInputStream;
 import java.io.EOFException;
 import java.io.IOException;
 
+import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
 import org.junit.Test;
 
 /** Test for push certificate parsing. */
 public class PushCertificateParserTest {
-       @Test
-       public void parseCertFromPktLine() throws Exception {
-               // Example push certificate generated by C git 2.2.0.
-               String input = "001ccertificate version 0.1\n"
-               + "0041pusher Dave Borowitz <dborowitz@google.com> 1433954361 -0700\n"
-               + "0024pushee git://localhost/repo.git\n"
-               + "002anonce 1433954361-bde756572d665bba81d8\n"
-               + "0005\n"
-               + "00680000000000000000000000000000000000000000"
-               + " 6c2b981a177396fb47345b7df3e4d3f854c6bea7"
-               + " refs/heads/master\n"
-               + "0022-----BEGIN PGP SIGNATURE-----\n"
-               + "0016Version: GnuPG v1\n"
-               + "0005\n"
-               + "0045iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n"
-               + "00459tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n"
-               + "0045htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n"
-               + "00454ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n"
-               + "0045IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n"
-               + "0045+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n"
-               + "000a=XFeC\n"
-               + "0020-----END PGP SIGNATURE-----\n"
-               + "0012push-cert-end\n";
+       // Example push certificate generated by C git 2.2.0.
+       private static final String INPUT = "001ccertificate version 0.1\n"
+                       + "0041pusher Dave Borowitz <dborowitz@google.com> 1433954361 -0700\n"
+                       + "0024pushee git://localhost/repo.git\n"
+                       + "002anonce 1433954361-bde756572d665bba81d8\n"
+                       + "0005\n"
+                       + "00680000000000000000000000000000000000000000"
+                       + " 6c2b981a177396fb47345b7df3e4d3f854c6bea7"
+                       + " refs/heads/master\n"
+                       + "0022-----BEGIN PGP SIGNATURE-----\n"
+                       + "0016Version: GnuPG v1\n"
+                       + "0005\n"
+                       + "0045iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n"
+                       + "00459tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n"
+                       + "0045htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n"
+                       + "00454ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n"
+                       + "0045IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n"
+                       + "0045+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n"
+                       + "000a=XFeC\n"
+                       + "0020-----END PGP SIGNATURE-----\n"
+                       + "0012push-cert-end\n";
 
-               PacketLineIn pckIn = newPacketLineIn(input);
+       private Repository db;
+
+       @Before
+       public void setUp() {
+               db = new InMemoryRepository(new DfsRepositoryDescription("repo"));
+       }
+
+       private static SignedPushConfig newEnabledConfig() {
                Config cfg = new Config();
                cfg.setString("receive", null, "certnonceseed", "sekret");
-               Repository db = new InMemoryRepository(
-                               new DfsRepositoryDescription("repo"));
+               return SignedPushConfig.KEY.parse(cfg);
+       }
+
+       private static SignedPushConfig newDisabledConfig() {
+               return SignedPushConfig.KEY.parse(new Config());
+       }
+
+       @Test
+       public void noCert() throws Exception {
+               PushCertificateParser parser =
+                               new PushCertificateParser(db, newEnabledConfig());
+               assertTrue(parser.enabled());
+               assertNull(parser.build());
+
+               ObjectId oldId = ObjectId.zeroId();
+               ObjectId newId =
+                               ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
+               String rawLine =
+                               oldId.name() + " " + newId.name() + " refs/heads/master";
+               ReceiveCommand cmd = BaseReceivePack.parseCommand(rawLine);
+
+               parser.addCommand(cmd, rawLine);
+               parser.addCommand(rawLine);
+               assertNull(parser.build());
+       }
 
-               PushCertificateParser parser = new PushCertificateParser(
-                               db, new SignedPushConfig(cfg));
+       @Test
+       public void disabled() throws Exception {
+               PacketLineIn pckIn = newPacketLineIn(INPUT);
+               PushCertificateParser parser =
+                               new PushCertificateParser(db, newDisabledConfig());
+               assertFalse(parser.enabled());
+               assertNull(parser.build());
+
+               parser.receiveHeader(pckIn, false);
+               parser.addCommand(pckIn.readStringRaw());
+               assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw());
+               parser.receiveSignature(pckIn);
+               assertNull(parser.build());
+       }
+
+       @Test
+       public void disabledParserStillRequiresCorrectSyntax() throws Exception {
+               PacketLineIn pckIn = newPacketLineIn("001ccertificate version XYZ\n");
+               PushCertificateParser parser =
+                               new PushCertificateParser(db, newDisabledConfig());
+               assertFalse(parser.enabled());
+               try {
+                       parser.receiveHeader(pckIn, false);
+                       fail("Expected PackProtocolException");
+               } catch (PackProtocolException e) {
+                       assertEquals(
+                                       "Push certificate has missing or invalid value for certificate"
+                                               + " version: XYZ",
+                                       e.getMessage());
+               }
+               assertNull(parser.build());
+       }
+
+       @Test
+       public void parseCertFromPktLine() throws Exception {
+               PacketLineIn pckIn = newPacketLineIn(INPUT);
+               PushCertificateParser parser =
+                               new PushCertificateParser(db, newEnabledConfig());
                parser.receiveHeader(pckIn, false);
                parser.addCommand(pckIn.readStringRaw());
                assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw());
@@ -117,9 +186,9 @@ public class PushCertificateParserTest {
                assertEquals("6c2b981a177396fb47345b7df3e4d3f854c6bea7",
                                cmd.getNewId().name());
 
-               assertEquals(concatPacketLines(input, 0, 6), cert.toText());
+               assertEquals(concatPacketLines(INPUT, 0, 6), cert.toText());
 
-               String signature = concatPacketLines(input, 6, 17);
+               String signature = concatPacketLines(INPUT, 6, 17);
                assertTrue(signature.startsWith(PushCertificateParser.BEGIN_SIGNATURE));
                assertTrue(signature.endsWith(PushCertificateParser.END_SIGNATURE));
                assertEquals(signature, cert.getSignature());

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 54eb747e9b74f2364e319b3dd32b182b0b2bce2b..37e5d3cd3c0dcb6c4719173fd2326de86e3d0923 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -260,7 +260,8 @@ public abstract class BaseReceivePack {
         * <p>
         * Only valid after commands are read from the wire.
         *
-        * @return the parsed certificate, or null if push certificates are disabled.
+        * @return the parsed certificate, or null if push certificates are disabled
+        *         or no cert was presented by the client.
         * @throws IOException if the certificate was present but invalid.
         * @since 4.1
         */

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 cb6a8598b3fcd9c01a98126f5de3afaee3103bf4..04871c7f9a788aa13490cf23b13b42b53bfba9c0 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java
@@ -82,6 +82,7 @@ public class PushCertificateParser {
 
        private static final String END_CERT = "push-cert-end\n"; //$NON-NLS-1$
 
+       private boolean received;
        private String version;
        private PushCertificateIdent pusher;
        private String pushee;
@@ -135,7 +136,7 @@ public class PushCertificateParser {
         * @since 4.1
         */
        public PushCertificate build() throws IOException {
-               if (nonceGenerator == null) {
+               if (!received || nonceGenerator == null) {
                        return null;
                }
                try {
@@ -210,6 +211,7 @@ public class PushCertificateParser {
         */
        public void receiveHeader(PacketLineIn pckIn, boolean stateless)
                        throws IOException {
+               received = true;
                try {
                        version = parseHeader(pckIn, VERSION);
                        if (!version.equals(VERSION_0_1)) {
@@ -255,6 +257,7 @@ public class PushCertificateParser {
         * @since 4.0
         */
        public void receiveSignature(PacketLineIn pckIn) throws IOException {
+               received = true;
                try {
                        StringBuilder sig = new StringBuilder(BEGIN_SIGNATURE);
                        String line;