Merge changes I8277fd45,I7ac4e0ae,Ib475dfc0,Ib26adf95

* changes:
  Try to send HTTP error messages over sideband
  Extract the capability parsing logic in {Upload,Receive}Pack
  Make capability strings in BasePack{Fetch,Push}Connection public
  Fix a typo in "capabilities" in ReceivePack

org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitSmartHttpTools.java

@@ -43,6 +43,12 @@
 
 package org.eclipse.jgit.http.server;
 
+import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
+import static org.eclipse.jgit.transport.BasePackFetchConnection.OPTION_SIDE_BAND;
+import static org.eclipse.jgit.transport.BasePackFetchConnection.OPTION_SIDE_BAND_64K;
+import static org.eclipse.jgit.transport.BasePackPushConnection.CAPABILITY_SIDE_BAND_64K;
+import static org.eclipse.jgit.transport.SideBandOutputStream.CH_ERROR;
+import static org.eclipse.jgit.transport.SideBandOutputStream.SMALL_BUF;
 import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
 import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
 import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
@@ -58,7 +64,12 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 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.
@@ -141,6 +152,11 @@ public class GitSmartHttpTools {
 	 * to a Git protocol client using an HTTP 200 OK response with the error
 	 * embedded in the payload. If the request was not issued by a Git client,
 	 * an HTTP response code is returned instead.
+	 * <p>
+	 * This method may only be called before handing off the request to
+	 * {@link UploadPack#upload(java.io.InputStream, OutputStream, OutputStream)}
+	 * or
+	 * {@link ReceivePack#receive(java.io.InputStream, OutputStream, OutputStream)}.
 	 *
 	 * @param req
 	 *            current request.
@@ -176,26 +192,119 @@ public class GitSmartHttpTools {
 			}
 		}
 
+		if (isInfoRefs(req)) {
+			sendInfoRefsError(req, res, textForGit);
+		} else if (isUploadPack(req)) {
+			sendUploadPackError(req, res, textForGit);
+		} else if (isReceivePack(req)) {
+			sendReceivePackError(req, res, textForGit);
+		} else {
+			if (httpStatus < 400)
+				ServletUtils.consumeRequestBody(req);
+			res.sendError(httpStatus);
+		}
+	}
+
+	private static void sendInfoRefsError(HttpServletRequest req,
+			HttpServletResponse res, String textForGit) throws IOException {
 		ByteArrayOutputStream buf = new ByteArrayOutputStream(128);
 		PacketLineOut pck = new PacketLineOut(buf);
-
-		if (isInfoRefs(req)) {
 		String svc = req.getParameter("service");
 		pck.writeString("# service=" + svc + "\n");
 		pck.end();
 		pck.writeString("ERR " + textForGit);
 		send(req, res, infoRefsResultType(svc), buf.toByteArray());
-		} else if (isUploadPack(req)) {
-			pck.writeString("ERR " + textForGit);
+	}
+
+	private static void sendUploadPackError(HttpServletRequest req,
+			HttpServletResponse res, String textForGit) throws IOException {
+		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);
 		send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
-		} else if (isReceivePack(req)) {
-			pck.writeString("ERR " + textForGit);
+	}
+
+	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()).readStringRaw();
+			UploadPack.FirstLine parsed = new UploadPack.FirstLine(line);
+			return (parsed.getOptions().contains(OPTION_SIDE_BAND)
+					|| parsed.getOptions().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);
+		PacketLineOut pckOut = new PacketLineOut(buf);
+
+		boolean sideband;
+		ReceivePack rp = (ReceivePack) req.getAttribute(ATTRIBUTE_HANDLER);
+		if (rp != null) {
+			try {
+				sideband = rp.isSideBand();
+			} catch (RequestNotYetReadException e) {
+				sideband = isReceivePackSideBand(req);
+			}
+		} else
+			sideband = isReceivePackSideBand(req);
+
+		if (sideband)
+			writeSideBand(buf, textForGit);
+		else
+			writePacket(pckOut, textForGit);
 		send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
-		} else {
-			if (httpStatus < 400)
-				ServletUtils.consumeRequestBody(req);
-			res.sendError(httpStatus);
 	}
+
+	private static boolean isReceivePackSideBand(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 a ReceivePack, or it might not have read any of the request.
+			// So, cheat and read the first line.
+			String line = new PacketLineIn(req.getInputStream()).readStringRaw();
+			ReceivePack.FirstLine parsed = new ReceivePack.FirstLine(line);
+			return parsed.getCapabilities().contains(CAPABILITY_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 writeSideBand(OutputStream out, String textForGit)
+			throws IOException {
+		OutputStream msg = new SideBandOutputStream(CH_ERROR, SMALL_BUF, out);
+		msg.write(Constants.encode("error: " + textForGit));
+		msg.flush();
+	}
+
+	private static void writePacket(PacketLineOut pckOut, String textForGit)
+			throws IOException {
+		pckOut.writeString("error: " + textForGit);
 	}
 
 	private static void send(HttpServletRequest req, HttpServletResponse res,

org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackFetchConnection.java

@@ -120,25 +120,35 @@ public abstract class BasePackFetchConnection extends BasePackConnection
 	 */
 	protected static final int MIN_CLIENT_BUFFER = 2 * 32 * 46 + 8;
 
-	static final String OPTION_INCLUDE_TAG = "include-tag";
+	/** Include tags if we are also including the referenced objects. */
+	public static final String OPTION_INCLUDE_TAG = "include-tag";
 
-	static final String OPTION_MULTI_ACK = "multi_ack";
+	/** Mutli-ACK support for improved negotiation. */
+	public static final String OPTION_MULTI_ACK = "multi_ack";
 
-	static final String OPTION_MULTI_ACK_DETAILED = "multi_ack_detailed";
+	/** Mutli-ACK detailed support for improved negotiation. */
+	public static final String OPTION_MULTI_ACK_DETAILED = "multi_ack_detailed";
 
-	static final String OPTION_THIN_PACK = "thin-pack";
+	/** The client supports packs with deltas but not their bases. */
+	public static final String OPTION_THIN_PACK = "thin-pack";
 
-	static final String OPTION_SIDE_BAND = "side-band";
+	/** The client supports using the side-band for progress messages. */
+	public static final String OPTION_SIDE_BAND = "side-band";
 
-	static final String OPTION_SIDE_BAND_64K = "side-band-64k";
+	/** The client supports using the 64K side-band for progress messages. */
+	public static final String OPTION_SIDE_BAND_64K = "side-band-64k";
 
-	static final String OPTION_OFS_DELTA = "ofs-delta";
+	/** The client supports packs with OFS deltas. */
+	public static final String OPTION_OFS_DELTA = "ofs-delta";
 
-	static final String OPTION_SHALLOW = "shallow";
+	/** The client supports shallow fetches. */
+	public static final String OPTION_SHALLOW = "shallow";
 
-	static final String OPTION_NO_PROGRESS = "no-progress";
+	/** The client does not want progress messages and will ignore them. */
+	public static final String OPTION_NO_PROGRESS = "no-progress";
 
-	static final String OPTION_NO_DONE = "no-done";
+	/** The client supports receiving a pack before it has sent "done". */
+	public static final String OPTION_NO_DONE = "no-done";
 
 	static enum MultiAck {
 		OFF, CONTINUE, DETAILED;

org.eclipse.jgit/src/org/eclipse/jgit/transport/BasePackPushConnection.java

@@ -83,13 +83,17 @@ import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
  */
 public abstract class BasePackPushConnection extends BasePackConnection implements
 		PushConnection {
-	static final String CAPABILITY_REPORT_STATUS = "report-status";
+	/** The client expects a status report after the server processes the pack. */
+	public static final String CAPABILITY_REPORT_STATUS = "report-status";
 
-	static final String CAPABILITY_DELETE_REFS = "delete-refs";
+	/** The server supports deleting refs. */
+	public static final String CAPABILITY_DELETE_REFS = "delete-refs";
 
-	static final String CAPABILITY_OFS_DELTA = "ofs-delta";
+	/** The server supports packs with OFS deltas. */
+	public static final String CAPABILITY_OFS_DELTA = "ofs-delta";
 
-	static final String CAPABILITY_SIDE_BAND_64K = "side-band-64k";
+	/** The client supports using the 64K side-band for progress messages. */
+	public static final String CAPABILITY_SIDE_BAND_64K = "side-band-64k";
 
 	private final boolean thinPack;
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceivePack.java

@@ -98,6 +98,39 @@ import org.eclipse.jgit.util.io.TimeoutOutputStream;
  * Implements the server side of a push connection, receiving objects.
  */
 public class ReceivePack {
+	/** Data in the first line of a request, the line itself plus capabilities. */
+	public static class FirstLine {
+		private final String line;
+		private final Set<String> capabilities;
+
+		/**
+		 * Parse the first line of a receive-pack request.
+		 *
+		 * @param line
+		 *            line from the client.
+		 */
+		public FirstLine(String line) {
+			final HashSet<String> caps = new HashSet<String>();
+			final int nul = line.indexOf('\0');
+			if (nul >= 0) {
+				for (String c : line.substring(nul + 1).split(" "))
+					caps.add(c);
+			}
+			this.line = line.substring(0, nul);
+			this.capabilities = Collections.unmodifiableSet(caps);
+		}
+
+		/** @return non-capabilities part of the line. */
+		public String getLine() {
+			return line;
+		}
+
+		/** @return capabilities parsed from the line. */
+		public Set<String> getCapabilities() {
+			return capabilities;
+		}
+	}
+
 	/** Database we write the stored objects into. */
 	private final Repository db;
 
@@ -175,7 +208,7 @@ public class ReceivePack {
 	private Set<ObjectId> advertisedHaves;
 
 	/** Capabilities requested by the client. */
-	private Set<String> enabledCapablities;
+	private Set<String> enabledCapabilities;
 
 	/** Commands to execute, as received by the client. */
 	private List<ReceiveCommand> commands;
@@ -616,6 +649,23 @@ public class ReceivePack {
 		maxObjectSizeLimit = limit;
 	}
 
+	/**
+	 * Check whether the client expects a side-band stream.
+	 *
+	 * @return true if the client has advertised a side-band capability, false
+	 *     otherwise.
+	 * @throws RequestNotYetReadException
+	 *             if the client's request has not yet been read from the wire, so
+	 *             we do not know if they expect side-band. Note that the client
+	 *             may have already written the request, it just has not been
+	 *             read.
+	 */
+	public boolean isSideBand() throws RequestNotYetReadException {
+		if (enabledCapabilities == null)
+			throw new RequestNotYetReadException();
+		return enabledCapabilities.contains(CAPABILITY_SIDE_BAND_64K);
+	}
+
 	/** @return all of the command received by the current request. */
 	public List<ReceiveCommand> getAllCommands() {
 		return Collections.unmodifiableList(commands);
@@ -713,7 +763,6 @@ public class ReceivePack {
 			pckOut = new PacketLineOut(rawOut);
 			pckOut.setFlushOnEnd(false);
 
-			enabledCapablities = new HashSet<String>();
 			commands = new ArrayList<ReceiveCommand>();
 
 			service();
@@ -753,7 +802,7 @@ public class ReceivePack {
 				pckIn = null;
 				pckOut = null;
 				refs = null;
-				enabledCapablities = null;
+				enabledCapabilities = null;
 				commands = null;
 				if (timer != null) {
 					try {
@@ -891,12 +940,9 @@ public class ReceivePack {
 				break;
 
 			if (commands.isEmpty()) {
-				final int nul = line.indexOf('\0');
-				if (nul >= 0) {
-					for (String c : line.substring(nul + 1).split(" "))
-						enabledCapablities.add(c);
-					line = line.substring(0, nul);
-				}
+				final FirstLine firstLine = new FirstLine(line);
+				enabledCapabilities = firstLine.getCapabilities();
+				line = firstLine.getLine();
 			}
 
 			if (line.length() < 83) {
@@ -919,9 +965,9 @@ public class ReceivePack {
 	}
 
 	private void enableCapabilities() {
-		reportStatus = enabledCapablities.contains(CAPABILITY_REPORT_STATUS);
+		reportStatus = enabledCapabilities.contains(CAPABILITY_REPORT_STATUS);
 
-		sideBand = enabledCapablities.contains(CAPABILITY_SIDE_BAND_64K);
+		sideBand = enabledCapabilities.contains(CAPABILITY_SIDE_BAND_64K);
 		if (sideBand) {
 			OutputStream out = rawOut;
 

org.eclipse.jgit/src/org/eclipse/jgit/transport/RequestNotYetReadException.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2012, Google Inc.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   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.
+ *
+ * - 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.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package org.eclipse.jgit.transport;
+
+/** Indicates that a client request has not yet been read from the wire. */
+public class RequestNotYetReadException extends IllegalStateException {
+	private static final long serialVersionUID = 1L;
+
+	/** Initialize with no message. */
+	public RequestNotYetReadException() {
+		// Do not set a message.
+	}
+
+	/**
+	 * @param msg
+	 *            a message explaining the state. This message should not
+	 *            be shown to an end-user.
+	 */
+	public RequestNotYetReadException(String msg) {
+		super(msg);
+	}
+}

org.eclipse.jgit/src/org/eclipse/jgit/transport/SideBandOutputStream.java

@@ -55,16 +55,21 @@ import org.eclipse.jgit.JGitText;
  * This stream is buffered at packet sizes, so the caller doesn't need to wrap
  * it in yet another buffered stream.
  */
-class SideBandOutputStream extends OutputStream {
-	static final int CH_DATA = SideBandInputStream.CH_DATA;
+public class SideBandOutputStream extends OutputStream {
+	/** Channel used for pack data. */
+	public static final int CH_DATA = SideBandInputStream.CH_DATA;
 
-	static final int CH_PROGRESS = SideBandInputStream.CH_PROGRESS;
+	/** Channel used for progress messages. */
+	public static final int CH_PROGRESS = SideBandInputStream.CH_PROGRESS;
 
-	static final int CH_ERROR = SideBandInputStream.CH_ERROR;
+	/** Channel used for error messages. */
+	public static final int CH_ERROR = SideBandInputStream.CH_ERROR;
 
-	static final int SMALL_BUF = 1000;
+	/** Default buffer size for a small amount of data. */
+	public static final int SMALL_BUF = 1000;
 
-	static final int MAX_BUF = 65520;
+	/** Maximum buffer size for a single packet of sideband data. */
+	public static final int MAX_BUF = 65520;
 
 	static final int HDR_SIZE = 5;
 
@@ -95,7 +100,7 @@ class SideBandOutputStream extends OutputStream {
 	 *            stream that the packets are written onto. This stream should
 	 *            be attached to a SideBandInputStream on the remote side.
 	 */
-	SideBandOutputStream(final int chan, final int sz, final OutputStream os) {
+	public SideBandOutputStream(final int chan, final int sz, final OutputStream os) {
 		if (chan <= 0 || chan > 255)
 			throw new IllegalArgumentException(MessageFormat.format(JGitText.get().channelMustBeInRange0_255, chan));
 		if (sz <= HDR_SIZE)

org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java

@@ -118,6 +118,44 @@ public class UploadPack {
 		ANY;
 	}
 
+	/** Data in the first line of a request, the line itself plus options. */
+	public static class FirstLine {
+		private final String line;
+		private final Set<String> options;
+
+		/**
+		 * Parse the first line of a receive-pack request.
+		 *
+		 * @param line
+		 *            line from the client.
+		 */
+		public FirstLine(String line) {
+			if (line.length() > 45) {
+				final HashSet<String> opts = new HashSet<String>();
+				String opt = line.substring(45);
+				if (opt.startsWith(" "))
+					opt = opt.substring(1);
+				for (String c : opt.split(" "))
+					opts.add(c);
+				this.line = line.substring(0, 45);
+				this.options = Collections.unmodifiableSet(opts);
+			} else {
+				this.line = line;
+				this.options = Collections.emptySet();
+			}
+		}
+
+		/** @return non-capabilities part of the line. */
+		public String getLine() {
+			return line;
+		}
+
+		/** @return options parsed from the line. */
+		public Set<String> getOptions() {
+			return options;
+		}
+	}
+
 	/** Database we read the objects from. */
 	private final Repository db;
 
@@ -167,7 +205,7 @@ public class UploadPack {
 	private PreUploadHook preUploadHook = PreUploadHook.NULL;
 
 	/** Capabilities requested by the client. */
-	private final Set<String> options = new HashSet<String>();
+	private Set<String> options;
 
 	/** Raw ObjectIds the client has asked for, before validating them. */
 	private final Set<ObjectId> wantIds = new HashSet<ObjectId>();
@@ -426,6 +464,24 @@ public class UploadPack {
 		this.logger = logger;
 	}
 
+	/**
+	 * Check whether the client expects a side-band stream.
+	 *
+	 * @return true if the client has advertised a side-band capability, false
+	 *     otherwise.
+	 * @throws RequestNotYetReadException
+	 *             if the client's request has not yet been read from the wire, so
+	 *             we do not know if they expect side-band. Note that the client
+	 *             may have already written the request, it just has not been
+	 *             read.
+	 */
+	public boolean isSideBand() throws RequestNotYetReadException {
+		if (options == null)
+			throw new RequestNotYetReadException();
+		return (options.contains(OPTION_SIDE_BAND)
+				|| options.contains(OPTION_SIDE_BAND_64K));
+	}
+
 	/**
 	 * Execute the upload task on the socket.
 	 *
@@ -664,12 +720,9 @@ public class UploadPack {
 				throw new PackProtocolException(MessageFormat.format(JGitText.get().expectedGot, "want", line));
 
 			if (isFirst && line.length() > 45) {
-				String opt = line.substring(45);
-				if (opt.startsWith(" "))
-					opt = opt.substring(1);
-				for (String c : opt.split(" "))
-					options.add(c);
-				line = line.substring(0, 45);
+				final FirstLine firstLine = new FirstLine(line);
+				options = firstLine.getOptions();
+				line = firstLine.getLine();
 			}
 
 			wantIds.add(ObjectId.fromString(line.substring(5)));