Merge changes I1f968649,Ied678797

* changes:
  UploadPackServlet: Use uploadWithExceptionPropagation
  GitSmartHttpTools: Do not use sideband when sending an error

3 files changed, 13 insertions, 41 deletions

diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
index 5e09d012d7..5077e83b36 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java
@@ -48,8 +48,6 @@ import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
 import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SIDE_BAND_64K;
-import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND;
-import static org.eclipse.jgit.transport.GitProtocolConstants.OPTION_SIDE_BAND_64K;
 import static org.eclipse.jgit.transport.SideBandOutputStream.CH_ERROR;
 import static org.eclipse.jgit.transport.SideBandOutputStream.SMALL_BUF;
 
@@ -64,14 +62,12 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jgit.internal.transport.parser.FirstCommand;
-import org.eclipse.jgit.internal.transport.parser.FirstWant;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.transport.PacketLineIn;
 import org.eclipse.jgit.transport.PacketLineOut;
 import org.eclipse.jgit.transport.ReceivePack;
 import org.eclipse.jgit.transport.RequestNotYetReadException;
 import org.eclipse.jgit.transport.SideBandOutputStream;
-import org.eclipse.jgit.transport.UploadPack;
 
 /**
  * Utility functions for handling the Git-over-HTTP protocol.
@@ -220,44 +216,15 @@ public class GitSmartHttpTools {
 
 	private static void sendUploadPackError(HttpServletRequest req,
 			HttpServletResponse res, String textForGit) throws IOException {
+		// Do not use sideband. Sideband is acceptable only while packfile is
+		// being sent. Other places, like acknowledgement section, do not
+		// support sideband. Use an error packet.
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
 		PacketLineOut pckOut = new PacketLineOut(buf);
-
-		boolean sideband;
-		UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
-		if (up != null) {
-			try {
-				sideband = up.isSideBand();
-			} catch (RequestNotYetReadException e) {
-				sideband = isUploadPackSideBand(req);
-			}
-		} else
-			sideband = isUploadPackSideBand(req);
-
-		if (sideband)
-			writeSideBand(buf, textForGit);
-		else
-			writePacket(pckOut, textForGit);
+		writePacket(pckOut, textForGit);
 		send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
 	}
 
-	private static boolean isUploadPackSideBand(HttpServletRequest req) {
-		try {
-			// The client may be in a state where they have sent the sideband
-			// capability and are expecting a response in the sideband, but we might
-			// not have an UploadPack, or it might not have read any of the request.
-			// So, cheat and read the first line.
-			String line = new PacketLineIn(req.getInputStream()).readString();
-			FirstWant parsed = FirstWant.fromLine(line);
-			return (parsed.getCapabilities().contains(OPTION_SIDE_BAND)
-					|| parsed.getCapabilities().contains(OPTION_SIDE_BAND_64K));
-		} catch (IOException e) {
-			// Probably the connection is closed and a subsequent write will fail, but
-			// try it just in case.
-			return false;
-		}
-	}
-
 	private static void sendReceivePackError(HttpServletRequest req,
 			HttpServletResponse res, String textForGit) throws IOException {
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
@@ -308,7 +275,7 @@ public class GitSmartHttpTools {
 
 	private static void writePacket(PacketLineOut pckOut, String textForGit)
 			throws IOException {
-		pckOut.writeString("error: " + textForGit);
+		pckOut.writeString("ERR " + textForGit);
 	}
 
 	private static void send(HttpServletRequest req, HttpServletResponse res,
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 54561e0cfc..6baab5ddd7 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
@@ -70,7 +70,9 @@ import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
+
 import org.eclipse.jgit.annotations.Nullable;
+import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.http.server.UploadPackErrorHandler.UploadPackRunnable;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.InternalHttpServerGlue;
@@ -212,7 +214,8 @@ class UploadPackServlet extends HttpServlet {
 			rsp.setContentType(UPLOAD_PACK_RESULT_TYPE);
 
 			try {
-				up.upload(getInputStream(req), out, null);
+				up.uploadWithExceptionPropagation(getInputStream(req), out,
+						null);
 				out.close();
 			} catch (ServiceMayNotContinueException e) {
 				if (e.isOutput()) {
@@ -245,7 +248,9 @@ class UploadPackServlet extends HttpServlet {
 			log(up.getRepository(), e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();
-				sendError(req, rsp, SC_INTERNAL_SERVER_ERROR);
+				String msg = e instanceof PackProtocolException ? e.getMessage()
+						: null;
+				sendError(req, rsp, SC_INTERNAL_SERVER_ERROR, msg);
 			}
 		}
 	}
diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
index 99aa06b17c..b23fd28547 100644
--- a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/SmartClientSmartServerTest.java
@@ -1214,7 +1214,7 @@ public class SmartClientSmartServerTest extends AllFactoriesHttpTestCase {
 						Collections.<ObjectId> emptySet());
 				fail("Successfully served ref with value " + c.getRef(master));
 			} catch (TransportException err) {
-				assertEquals("internal server error", err.getMessage());
+				assertEquals("Internal server error", err.getMessage());
 			}
 		} finally {
 			noRefServer.tearDown();