FirstWant: tighten first-want line validation

First-want line parsing accepts lines with an optional whitespace, when
the spec is strict requiring a white space.

Validate the line enforcing that there is a white space between oid and
capabilities list.

Change-Id: I45ada67030e0720f9b402c298be18c7518c799b1
Signed-off-by: Ivan Frade <ifrade@google.com>

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
index 572ec8bae587285aa5e1352485d3633692f79ee3..627079eac193d53f7be83e20e0307bfcf96525e1 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/internal/transport/parser/FirstWantTest.java
@@ -44,17 +44,20 @@ package org.eclipse.jgit.internal.transport.parser;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
+import org.eclipse.jgit.errors.PackProtocolException;
 import org.junit.Test;
 
 public class FirstWantTest {
 
        @Test
-       public void testFirstWantWithOptions() {
+       public void testFirstWantWithOptions() throws PackProtocolException {
                String line = "want b9d4d1eb2f93058814480eae9e1b67550f46ee38 "
                                + "no-progress include-tag ofs-delta agent=JGit/unknown";
 
@@ -69,7 +72,7 @@ public class FirstWantTest {
        }
 
        @Test
-       public void testFirstWantWithoutOptions() {
+       public void testFirstWantWithoutOptions() throws PackProtocolException {
                String line = "want b9d4d1eb2f93058814480eae9e1b67550f46ee38";
 
                FirstWant r = FirstWant.fromLine(line);
@@ -77,4 +80,42 @@ public class FirstWantTest {
                                r.getLine());
                assertTrue(r.getCapabilities().isEmpty());
        }
+
+       private String makeFirstWantLine(String capability) {
+               return String.format("want b9d4d1eb2f93058814480eae9e1b67550f46ee38 %s", capability);
+       }
+
+       @Test
+       public void testFirstWantNoWhitespace() {
+               try {
+                       FirstWant.fromLine(
+                                       "want b9d4d1eb2f93058814480eae9e1b67550f400000capability");
+                       fail("Accepting first want line without SP between oid and first capability");
+               } catch (PackProtocolException e) {
+                       // pass
+               }
+       }
+
+       @Test
+       public void testFirstWantOnlyWhitespace() throws PackProtocolException {
+               FirstWant r = FirstWant
+                               .fromLine("want b9d4d1eb2f93058814480eae9e1b67550f46ee38 ");
+               assertEquals("want b9d4d1eb2f93058814480eae9e1b67550f46ee38",
+                               r.getLine());
+       }
+
+       @Test
+       public void testFirstWantValidCapabilityNames()
+                       throws PackProtocolException {
+               List<String> validNames = Arrays.asList(
+                               "c", "cap", "C", "CAP", "1", "1cap", "cap-64k_test",
+                               "-", "-cap",
+                               "_", "_cap", "agent=pack.age/Version");
+
+               for (String capability: validNames) {
+                       FirstWant r = FirstWant.fromLine(makeFirstWantLine(capability));
+                       assertEquals(r.getCapabilities().size(), 1);
+                       assertTrue(r.getCapabilities().contains(capability));
+               }
+       }
 }

diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index 40915b7b1f6da41fb9acafb0e6cea09fc2ede8ee..b3f7b89c9bbec01600f5dbbd0fba0724257f2d65 100644 (file)
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -771,6 +771,7 @@ URINotSupported=URI not supported: {0}
 userConfigFileInvalid=User config file {0} invalid {1}
 validatingGitModules=Validating .gitmodules files
 walkFailure=Walk failure.
+wantNoSpaceWithCapabilities=No space between oid and first capability in first want line
 wantNotValid=want {0} not valid
 weeksAgo={0} weeks ago
 windowSizeMustBeLesserThanLimit=Window size must be < limit

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index 7a9d523de580269bde3bf6d50d21349820e0ac60..9f0e3e0c2a45096c78857fc96a66a3dadff048c3 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -832,6 +832,7 @@ public class JGitText extends TranslationBundle {
        /***/ public String userConfigFileInvalid;
        /***/ public String validatingGitModules;
        /***/ public String walkFailure;
+       /***/ public String wantNoSpaceWithCapabilities;
        /***/ public String wantNotValid;
        /***/ public String weeksAgo;
        /***/ public String windowSizeMustBeLesserThanLimit;

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
index ea0fbed1d6d1c11576e051039c3ee956e41b3541..1ac9b18874a634b101d68e670e36461f3545db65 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/transport/parser/FirstWant.java
@@ -42,10 +42,14 @@
  */
 package org.eclipse.jgit.internal.transport.parser;
 
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
+import org.eclipse.jgit.errors.PackProtocolException;
+import org.eclipse.jgit.internal.JGitText;
+
 /**
  * In the pack negotiation phase (protocol v0/v1), the client sends a list of
  * wants. The first "want" line is special, as it (can) have a list of
@@ -74,20 +78,21 @@ public class FirstWant {
         * @param line
         *            line from the client.
         * @return an instance of FirstWant
+        * @throws PackProtocolException
+        *             if the line doesn't follow the protocol format.
         */
-       public static FirstWant fromLine(String line) {
+       public static FirstWant fromLine(String line) throws PackProtocolException {
                String wantLine;
                Set<String> capabilities;
 
                if (line.length() > 45) {
-                       final HashSet<String> opts = new HashSet<>();
                        String opt = line.substring(45);
-                       if (opt.startsWith(" ")) { //$NON-NLS-1$
-                               opt = opt.substring(1);
-                       }
-                       for (String c : opt.split(" ")) { //$NON-NLS-1$
-                               opts.add(c);
+                       if (!opt.startsWith(" ")) { //$NON-NLS-1$
+                               throw new PackProtocolException(JGitText.get().wantNoSpaceWithCapabilities);
                        }
+                       opt = opt.substring(1);
+                       HashSet<String> opts = new HashSet<>(
+                                       Arrays.asList(opt.split(" "))); //$NON-NLS-1$
                        wantLine = line.substring(0, 45);
                        capabilities = Collections.unmodifiableSet(opts);
                } else {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 8d2158d5605da060106277bd31b8c3105b005bf6..82e174548b9e230bded69002d179f425c0b3f601 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -69,6 +69,7 @@ import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.io.UncheckedIOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -193,7 +194,11 @@ public class UploadPack {
                 *            line from the client.
                 */
                public FirstLine(String line) {
-                       firstWant = FirstWant.fromLine(line);
+                       try {
+                               firstWant = FirstWant.fromLine(line);
+                       } catch (PackProtocolException e) {
+                               throw new UncheckedIOException(e);
+                       }
                }
 
                /** @return non-capabilities part of the line. */