BaseReceivePack: More validation during parseCommand

Change-Id: I25f3a5582a45dd0ec8f78f5daf74c2203797a184

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 98164d9335e3cb71d3ad5f571a9d4c03657c0883..1b351920631a1098d2740744d1b2e51e16ec24bb 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java
@@ -13,17 +13,17 @@
  * conditions are met:
  *
  * - Redistributions of source code must retain the above copyright
- *      notice, this list of conditions and the following disclaimer.
+ *   notice, this list of conditions and the following disclaimer.
  *
  * - Redistributions in binary form must reproduce the above
- *      copyright notice, this list of conditions and the following
- *      disclaimer in the documentation and/or other materials provided
- *      with the distribution.
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
  *
  * - Neither the name of the Eclipse Foundation, Inc. nor the
- *      names of its contributors may be used to endorse or promote
- *      products derived from this software without specific prior
- *      written permission.
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
  *
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
  * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
@@ -43,7 +43,9 @@
 package org.eclipse.jgit.transport;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
+import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.junit.Test;
 
@@ -57,14 +59,34 @@ public class BaseReceivePackTest {
        }
 
        @Test
-       public void parseCommand() {
-               String input = "0000000000000000000000000000000000000000"
-                               + " deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"
-                               + " refs/heads/master";
-               ReceiveCommand cmd = BaseReceivePack.parseCommand(input);
+       public void parseCommand() throws Exception {
+               String o = "0000000000000000000000000000000000000000";
+               String n = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef";
+               String r = "refs/heads/master";
+               ReceiveCommand cmd = BaseReceivePack.parseCommand(o + " " + n + " " + r);
                assertEquals(ObjectId.zeroId(), cmd.getOldId());
                assertEquals("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef",
                                cmd.getNewId().name());
                assertEquals("refs/heads/master", cmd.getRefName());
+
+               assertParseCommandFails(null);
+               assertParseCommandFails("");
+               assertParseCommandFails(o.substring(35) + " " + n.substring(35)
+                               + " " + r + "\n");
+               assertParseCommandFails(o + " " + n + " " + r + "\n");
+               assertParseCommandFails(o + " " + n + " " + "refs^foo");
+               assertParseCommandFails(o + " " + n.substring(10) + " " + r);
+               assertParseCommandFails(o.substring(10) + " " + n + " " + r);
+               assertParseCommandFails("X" + o.substring(1) + " " + n + " " + r);
+               assertParseCommandFails(o + " " + "X" + n.substring(1) + " " + r);
+       }
+
+       private void assertParseCommandFails(String input) {
+               try {
+                       BaseReceivePack.parseCommand(input);
+                       fail();
+               } catch (PackProtocolException e) {
+                       // Expected.
+               }
        }
 }

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 9c157c3379133119884444e83e4a6a300d8fa4a3..6e49f47ff5ec5b8d8d1bd774248e770a63fe4c09 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
@@ -114,10 +114,10 @@ public class PushCertificateParserTest {
                ObjectId oldId = ObjectId.zeroId();
                ObjectId newId =
                                ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef");
-               String rawLine =
-                               oldId.name() + " " + newId.name() + " refs/heads/master";
-               ReceiveCommand cmd = BaseReceivePack.parseCommand(rawLine);
+               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);
                assertNull(parser.build());

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 37e5d3cd3c0dcb6c4719173fd2326de86e3d0923..819f77c06fd78eaf95d93e8789ebebc5abdc02a6 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -1100,13 +1100,13 @@ public abstract class BaseReceivePack {
                                continue;
                        }
 
-                       if (line.length() < 83) {
-                               final String m = JGitText.get().errorInvalidProtocolWantedOldNewRef;
-                               sendError(m);
-                               throw new PackProtocolException(m);
+                       ReceiveCommand cmd;
+                       try {
+                               cmd = parseCommand(line);
+                       } catch (PackProtocolException e) {
+                               sendError(e.getMessage());
+                               throw e;
                        }
-
-                       final ReceiveCommand cmd = parseCommand(line);
                        if (cmd.getRefName().equals(Constants.HEAD)) {
                                cmd.setResult(Result.REJECTED_CURRENT_BRANCH);
                        } else {
@@ -1129,10 +1129,26 @@ public abstract class BaseReceivePack {
                return line;
        }
 
-       static ReceiveCommand parseCommand(String line) {
-               ObjectId oldId = ObjectId.fromString(line.substring(0, 40));
-               ObjectId newId = ObjectId.fromString(line.substring(41, 81));
+       static ReceiveCommand parseCommand(String line) throws PackProtocolException {
+          if (line == null || line.length() < 83) {
+                       throw new PackProtocolException(
+                                       JGitText.get().errorInvalidProtocolWantedOldNewRef);
+               }
+               String oldStr = line.substring(0, 40);
+               String newStr = line.substring(41, 81);
+               ObjectId oldId, newId;
+               try {
+                       oldId = ObjectId.fromString(oldStr);
+                       newId = ObjectId.fromString(newStr);
+               } catch (IllegalArgumentException e) {
+                       throw new PackProtocolException(
+                                       JGitText.get().errorInvalidProtocolWantedOldNewRef, e);
+               }
                String name = line.substring(82);
+               if (!Repository.isValidRefName(name)) {
+                       throw new PackProtocolException(
+                                       JGitText.get().errorInvalidProtocolWantedOldNewRef);
+               }
                return new ReceiveCommand(oldId, newId, name);
        }
 

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 fea8f125e16b0764ac7b65eff3209fdd0907ab98..661a0f094ad0e500f05bb08c45aec2a240d4f6c4 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java
@@ -294,9 +294,11 @@ public class PushCertificateParser {
         * @param rawLine
         *            the exact line read from the wire that produced this
         *            command, including trailing newline if present.
+        * @throws PackProtocolException
+        *             if the raw line cannot be parsed to a command.
         * @since 4.0
         */
-       public void addCommand(String rawLine) {
+       public void addCommand(String rawLine) throws PackProtocolException {
                commands.add(parseCommand(chomp(rawLine)));
                rawCommands.append(rawLine);
        }