Add protocol v2 support in http

Teach UploadPack to support protocol v2 with non-bidirectional pipes,
and add support to the HTTP protocol for v2. This is only activated if
the repository's config has "protocol.version" equal to 2.

Change-Id: I093a14acd2c3850b8b98e14936a716958f35a848
Helped-by: Matthias Sohn <matthias.sohn@sap.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Jonathan Nieder <jrn@google.com>

6 files changed, 157 insertions, 7 deletions

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
index 3e9c1fefac..ca6b749e75 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
@@ -76,6 +76,7 @@ import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.InternalHttpServerGlue;
+import org.eclipse.jgit.transport.PacketLineOut;
 import org.eclipse.jgit.transport.RefAdvertiser.PacketLineOutRefAdvertiser;
 import org.eclipse.jgit.transport.ServiceMayNotContinueException;
 import org.eclipse.jgit.transport.UploadPack;
@@ -117,6 +118,25 @@ class UploadPackServlet extends HttpServlet {
 				up.setBiDirectionalPipe(false);
 				up.sendAdvertisedRefs(pck);
 			} finally {
+				// TODO(jonathantanmy): Move responsibility for closing the
+				// RevWalk to UploadPack, either by making it AutoCloseable
+				// or by making sendAdvertisedRefs clean up after itself.
+				up.getRevWalk().close();
+			}
+		}
+
+		@Override
+		protected void respond(HttpServletRequest req,
+				PacketLineOut pckOut, String serviceName) throws IOException,
+				ServiceNotEnabledException, ServiceNotAuthorizedException {
+			UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
+			try {
+				up.setBiDirectionalPipe(false);
+				up.sendAdvertisedRefs(new PacketLineOutRefAdvertiser(pckOut), serviceName);
+			} finally {
+				// TODO(jonathantanmy): Move responsibility for closing the
+				// RevWalk to UploadPack, either by making it AutoCloseable
+				// or by making sendAdvertisedRefs clean up after itself.
 				up.getRevWalk().close();
 			}
 		}
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/resolver/DefaultUploadPackFactory.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/resolver/DefaultUploadPackFactory.java
index 30edc15166..a69fab0afd 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/resolver/DefaultUploadPackFactory.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/resolver/DefaultUploadPackFactory.java
@@ -43,6 +43,8 @@
 
 package org.eclipse.jgit.http.server.resolver;
 
+import java.util.Arrays;
+
 import javax.servlet.http.HttpServletRequest;
 
 import org.eclipse.jgit.lib.Config;
@@ -73,9 +75,16 @@ public class DefaultUploadPackFactory implements
 	@Override
 	public UploadPack create(HttpServletRequest req, Repository db)
 			throws ServiceNotEnabledException, ServiceNotAuthorizedException {
-		if (db.getConfig().get(ServiceConfig::new).enabled)
-			return new UploadPack(db);
-		else
+		if (db.getConfig().get(ServiceConfig::new).enabled) {
+			UploadPack up = new UploadPack(db);
+			String header = req.getHeader("Git-Protocol"); //$NON-NLS-1$
+			if (header != null) {
+				String[] params = header.split(":"); //$NON-NLS-1$
+				up.setExtraParameters(Arrays.asList(params));
+			}
+			return up;
+		} else {
 			throw new ServiceNotEnabledException();
+		}
 	}
 }
diff --git a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF
index d83402bded..e7795eb506 100644
--- a/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.http.test/META-INF/MANIFEST.MF
@@ -9,8 +9,8 @@ Bundle-Localization: plugin
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
 Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
  javax.servlet.http;version="[2.5.0,3.2.0)",
- org.apache.commons.codec;version="[1.6.0, 2.0.0)",
- org.apache.commons.codec.binary;version="[1.6.0, 2.0.0)",
+ org.apache.commons.codec;version="[1.6.0,2.0.0)",
+ org.apache.commons.codec.binary;version="[1.6.0,2.0.0)",
  org.eclipse.jetty.continuation;version="[9.4.5,10.0.0)",
  org.eclipse.jetty.http;version="[9.4.5,10.0.0)",
  org.eclipse.jetty.io;version="[9.4.5,10.0.0)",
@@ -44,6 +44,7 @@ Import-Package: javax.servlet;version="[2.5.0,3.2.0)",
  org.eclipse.jgit.transport.http.apache;version="[5.0.0,5.1.0)",
  org.eclipse.jgit.transport.resolver;version="[5.0.0,5.1.0)",
  org.eclipse.jgit.util;version="[5.0.0,5.1.0)",
+ org.hamcrest;version="[1.1.0,2.0.0)",
  org.hamcrest.core;version="[1.1.0,2.0.0)",
  org.junit;version="[4.12,5.0.0)",
  org.junit.runner;version="[4.12,5.0.0)",
diff --git a/org.eclipse.jgit.http.test/pom.xml b/org.eclipse.jgit.http.test/pom.xml
index 6eef6cea56..06eb0e89e7 100644
--- a/org.eclipse.jgit.http.test/pom.xml
+++ b/org.eclipse.jgit.http.test/pom.xml
@@ -84,6 +84,13 @@
     </dependency>
 
     <dependency>
+      <groupId>org.hamcrest</groupId>
+      <artifactId>hamcrest-library</artifactId>
+      <scope>test</scope>
+      <version>[1.1.0,2.0.0)</version>
+    </dependency>
+
+    <dependency>
       <groupId>org.eclipse.jgit</groupId>
       <artifactId>org.eclipse.jgit.junit.http</artifactId>
       <version>${project.version}</version>
diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/HttpClientTests.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/HttpClientTests.java
index 035501ffc1..ef059bf2a3 100644
--- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/HttpClientTests.java
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/HttpClientTests.java
@@ -43,15 +43,20 @@
 
 package org.eclipse.jgit.http.test;
 
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.theInstance;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.File;
+import java.io.OutputStream;
 import java.net.URI;
+import java.net.URL;
 import java.util.List;
 
 import javax.servlet.http.HttpServletRequest;
@@ -75,9 +80,13 @@ import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.lib.StoredConfig;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.transport.FetchConnection;
+import org.eclipse.jgit.transport.PacketLineIn;
+import org.eclipse.jgit.transport.PacketLineOut;
 import org.eclipse.jgit.transport.Transport;
 import org.eclipse.jgit.transport.URIish;
 import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.eclipse.jgit.transport.http.HttpConnection;
+import org.eclipse.jgit.transport.http.JDKHttpConnectionFactory;
 import org.eclipse.jgit.transport.resolver.RepositoryResolver;
 import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
 import org.junit.Before;
@@ -345,4 +354,82 @@ public class HttpClientTests extends HttpTestCase {
 			assertNotNull(head);
 		}
 	}
+
+	@Test
+	public void testHttpClientWantsV2ButServerNotConfigured() throws Exception {
+		JDKHttpConnectionFactory f = new JDKHttpConnectionFactory();
+		String url = smartAuthNoneURI.toString() + "/info/refs?service=git-upload-pack";
+		HttpConnection c = f.create(new URL(url));
+		c.setRequestMethod("GET");
+		c.setRequestProperty("Git-Protocol", "version=2");
+		c.connect();
+		assertThat(c.getResponseCode(), is(200));
+
+		PacketLineIn pckIn = new PacketLineIn(c.getInputStream());
+
+		// Check that we get a v0 response.
+		assertThat(pckIn.readString(), is("# service=git-upload-pack"));
+		assertThat(pckIn.readString(), theInstance(PacketLineIn.END));
+		assertTrue(pckIn.readString().matches("[0-9a-f]{40} HEAD.*"));
+	}
+
+	@Test
+	public void testV2HttpFirstResponse() throws Exception {
+		remoteRepository.getRepository().getConfig().setInt(
+				"protocol", null, "version", 2);
+
+		JDKHttpConnectionFactory f = new JDKHttpConnectionFactory();
+		String url = smartAuthNoneURI.toString() + "/info/refs?service=git-upload-pack";
+		HttpConnection c = f.create(new URL(url));
+		c.setRequestMethod("GET");
+		c.setRequestProperty("Git-Protocol", "version=2");
+		c.connect();
+		assertThat(c.getResponseCode(), is(200));
+
+		PacketLineIn pckIn = new PacketLineIn(c.getInputStream());
+		assertThat(pckIn.readString(), is("version 2"));
+
+		// What remains are capabilities - ensure that all of them are
+		// non-empty strings, and that we see END at the end.
+		String s;
+		while ((s = pckIn.readString()) != PacketLineIn.END) {
+			assertTrue(!s.isEmpty());
+		}
+	}
+
+	@Test
+	public void testV2HttpSubsequentResponse() throws Exception {
+		remoteRepository.getRepository().getConfig().setInt(
+				"protocol", null, "version", 2);
+
+		JDKHttpConnectionFactory f = new JDKHttpConnectionFactory();
+		String url = smartAuthNoneURI.toString() + "/git-upload-pack";
+		HttpConnection c = f.create(new URL(url));
+		c.setRequestMethod("POST");
+		c.setRequestProperty("Content-Type", "application/x-git-upload-pack-request");
+		c.setRequestProperty("Git-Protocol", "version=2");
+		c.setDoOutput(true);
+		c.connect();
+
+		// Test ls-refs to verify that everything is connected
+		// properly. Tests for other commands go in
+		// UploadPackTest.java.
+
+		OutputStream os = c.getOutputStream();
+		PacketLineOut pckOut = new PacketLineOut(os);
+		pckOut.writeString("command=ls-refs");
+		pckOut.writeDelim();
+		pckOut.end();
+		os.close();
+
+		PacketLineIn pckIn = new PacketLineIn(c.getInputStream());
+
+		// Just check that we get what looks like a ref advertisement.
+		String s;
+		while ((s = pckIn.readString()) != PacketLineIn.END) {
+			assertTrue(s.matches("[0-9a-f]{40} [A-Za-z/]*"));
+		}
+
+		assertThat(c.getResponseCode(), is(200));
+	}
 }
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 82e6e62f0f..90c5b599d4 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -1147,12 +1147,34 @@ public class UploadPack {
 	 * @param adv
 	 *            the advertisement formatter.
 	 * @throws java.io.IOException
-	 *             the formatter failed to write an advertisement.
+	 *            the formatter failed to write an advertisement.
 	 * @throws org.eclipse.jgit.transport.ServiceMayNotContinueException
-	 *             the hook denied advertisement.
+	 *            the hook denied advertisement.
 	 */
 	public void sendAdvertisedRefs(RefAdvertiser adv) throws IOException,
 			ServiceMayNotContinueException {
+		sendAdvertisedRefs(adv, null);
+	}
+
+	/**
+	 * Generate an advertisement of available refs and capabilities.
+	 *
+	 * @param adv
+	 *            the advertisement formatter.
+	 * @param serviceName
+	 *            if not null, also output "# service=serviceName" followed by a
+	 *            flush packet before the advertisement. This is required
+	 *            in v0 of the HTTP protocol, described in Git's
+	 *            Documentation/technical/http-protocol.txt.
+	 * @throws java.io.IOException
+	 *            the formatter failed to write an advertisement.
+	 * @throws org.eclipse.jgit.transport.ServiceMayNotContinueException
+	 *            the hook denied advertisement.
+	 * @since 5.0
+	 */
+	public void sendAdvertisedRefs(RefAdvertiser adv,
+			@Nullable String serviceName) throws IOException,
+			ServiceMayNotContinueException {
 		if (useProtocolV2()) {
 			// The equivalent in v2 is only the capabilities
 			// advertisement.
@@ -1173,6 +1195,10 @@ public class UploadPack {
 			throw fail;
 		}
 
+		if (serviceName != null) {
+			adv.writeOne("# service=" + serviceName + '\n'); //$NON-NLS-1$
+			adv.end();
+		}
 		adv.init(db);
 		adv.advertiseCapability(OPTION_INCLUDE_TAG);
 		adv.advertiseCapability(OPTION_MULTI_ACK_DETAILED);