Merge changes I5381e110,I5534b560

* changes:
  Discard request HTTP bodies for status code <400
  Ensure all smart HTTP errors are sent to clients

5 files changed, 268 insertions, 22 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 3d2aff174d..8bd1704bc4 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
@@ -49,11 +49,11 @@ import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
-import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -184,24 +184,27 @@ public class GitSmartHttpTools {
 			pck.writeString("# service=" + svc + "\n");
 			pck.end();
 			pck.writeString("ERR " + textForGit);
-			send(res, infoRefsResultType(svc), buf.toByteArray());
+			send(req, res, infoRefsResultType(svc), buf.toByteArray());
 		} else if (isUploadPack(req)) {
 			pck.writeString("ERR " + textForGit);
-			send(res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
+			send(req, res, UPLOAD_PACK_RESULT_TYPE, buf.toByteArray());
 		} else if (isReceivePack(req)) {
 			pck.writeString("ERR " + textForGit);
-			send(res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
+			send(req, res, RECEIVE_PACK_RESULT_TYPE, buf.toByteArray());
 		} else {
+			if (httpStatus < 400)
+				ServletUtils.consumeRequestBody(req);
 			res.sendError(httpStatus);
 		}
 	}
 
-	private static void send(HttpServletResponse res, String type, byte[] buf)
-			throws IOException {
+	private static void send(HttpServletRequest req, HttpServletResponse res,
+			String type, byte[] buf) throws IOException {
+		ServletUtils.consumeRequestBody(req);
 		res.setStatus(HttpServletResponse.SC_OK);
 		res.setContentType(type);
 		res.setContentLength(buf.length);
-		ServletOutputStream os = res.getOutputStream();
+		OutputStream os = res.getOutputStream();
 		try {
 			os.write(buf);
 		} finally {
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
index 6af28ba0d3..c84d52b695 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ReceivePackServlet.java
@@ -52,6 +52,7 @@ import static org.eclipse.jgit.http.server.GitSmartHttpTools.RECEIVE_PACK_REQUES
 import static org.eclipse.jgit.http.server.GitSmartHttpTools.RECEIVE_PACK_RESULT_TYPE;
 import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
 import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
+import static org.eclipse.jgit.http.server.ServletUtils.consumeRequestBody;
 import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
 import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
 
@@ -158,17 +159,18 @@ class ReceivePackServlet extends HttpServlet {
 			return;
 		}
 
+		SmartOutputStream out = new SmartOutputStream(req, rsp) {
+			@Override
+			public void flush() throws IOException {
+				doFlush();
+			}
+		};
+
 		ReceivePack rp = (ReceivePack) req.getAttribute(ATTRIBUTE_HANDLER);
 		try {
 			rp.setBiDirectionalPipe(false);
 			rsp.setContentType(RECEIVE_PACK_RESULT_TYPE);
 
-			final SmartOutputStream out = new SmartOutputStream(req, rsp) {
-				@Override
-				public void flush() throws IOException {
-					doFlush();
-				}
-			};
 			rp.receive(getInputStream(req), out, null);
 			out.close();
 		} catch (UnpackException e) {
@@ -176,8 +178,10 @@ class ReceivePackServlet extends HttpServlet {
 			getServletContext().log(
 					HttpServerText.get().internalErrorDuringReceivePack,
 					e.getCause());
+			consumeRequestBody(req);
+			out.close();
 
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			getServletContext().log(HttpServerText.get().internalErrorDuringReceivePack, e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java
index 2114655875..91fb8cce9a 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/ServletUtils.java
@@ -119,6 +119,50 @@ public final class ServletUtils {
 	}
 
 	/**
+	 * Consume the entire request body, if one was supplied.
+	 *
+	 * @param req
+	 *            the request whose body must be consumed.
+	 */
+	public static void consumeRequestBody(HttpServletRequest req) {
+		if (0 < req.getContentLength() || isChunked(req)) {
+			try {
+				consumeRequestBody(req.getInputStream());
+			} catch (IOException e) {
+				// Ignore any errors obtaining the input stream.
+			}
+		}
+	}
+
+	private static boolean isChunked(HttpServletRequest req) {
+		return "chunked".equals(req.getHeader("Transfer-Encoding"));
+	}
+
+	/**
+	 * Consume the rest of the input stream and discard it.
+	 *
+	 * @param in
+	 *            the stream to discard, closed if not null.
+	 */
+	public static void consumeRequestBody(InputStream in) {
+		if (in == null)
+			return;
+		try {
+			while (0 < in.skip(2048) || 0 <= in.read()) {
+				// Discard until EOF.
+			}
+		} catch (IOException err) {
+			// Discard IOException during read or skip.
+		} finally {
+			try {
+				in.close();
+			} catch (IOException err) {
+				// Discard IOException during close of input stream.
+			}
+		}
+	}
+
+	/**
 	 * Send a plain text response to a {@code GET} or {@code HEAD} HTTP request.
 	 * <p>
 	 * The text response is encoded in the Git character encoding, UTF-8.
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 c7891dfc77..15ef2c7eac 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
@@ -52,6 +52,7 @@ import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK_REQUEST
 import static org.eclipse.jgit.http.server.GitSmartHttpTools.UPLOAD_PACK_RESULT_TYPE;
 import static org.eclipse.jgit.http.server.GitSmartHttpTools.sendError;
 import static org.eclipse.jgit.http.server.ServletUtils.ATTRIBUTE_HANDLER;
+import static org.eclipse.jgit.http.server.ServletUtils.consumeRequestBody;
 import static org.eclipse.jgit.http.server.ServletUtils.getInputStream;
 import static org.eclipse.jgit.http.server.ServletUtils.getRepository;
 
@@ -160,22 +161,26 @@ class UploadPackServlet extends HttpServlet {
 			return;
 		}
 
+		SmartOutputStream out = new SmartOutputStream(req, rsp) {
+			@Override
+			public void flush() throws IOException {
+				doFlush();
+			}
+		};
+
 		UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
 		try {
 			up.setBiDirectionalPipe(false);
 			rsp.setContentType(UPLOAD_PACK_RESULT_TYPE);
 
-			final SmartOutputStream out = new SmartOutputStream(req, rsp) {
-				@Override
-				public void flush() throws IOException {
-					doFlush();
-				}
-			};
 			up.upload(getInputStream(req), out, null);
 			out.close();
 
 		} catch (UploadPackMayNotContinueException e) {
-			if (!e.isOutput() && !rsp.isCommitted()) {
+			if (e.isOutput()) {
+				consumeRequestBody(req);
+				out.close();
+			} else if (!rsp.isCommitted()) {
 				rsp.reset();
 				sendError(req, rsp, SC_FORBIDDEN, e.getMessage());
 			}
@@ -186,8 +191,10 @@ class UploadPackServlet extends HttpServlet {
 			getServletContext().log(
 					HttpServerText.get().internalErrorDuringUploadPack,
 					e.getCause());
+			consumeRequestBody(req);
+			out.close();
 
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();
diff --git a/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/ProtocolErrorTest.java b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/ProtocolErrorTest.java
new file mode 100644
index 0000000000..8cb9e087da
--- /dev/null
+++ b/org.eclipse.jgit.http.test/tst/org/eclipse/jgit/http/test/ProtocolErrorTest.java
@@ -0,0 +1,188 @@
+/*
+ * Copyright (C) 2010, 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.http.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.eclipse.jgit.JGitText;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.http.server.GitServlet;
+import org.eclipse.jgit.http.server.GitSmartHttpTools;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.junit.http.HttpTestCase;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevBlob;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.storage.file.FileRepository;
+import org.eclipse.jgit.transport.PacketLineIn;
+import org.eclipse.jgit.transport.PacketLineOut;
+import org.eclipse.jgit.transport.URIish;
+import org.eclipse.jgit.transport.resolver.RepositoryResolver;
+import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException;
+import org.eclipse.jgit.util.NB;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ProtocolErrorTest extends HttpTestCase {
+	private FileRepository remoteRepository;
+
+	private URIish remoteURI;
+
+	private RevBlob a_blob;
+
+	@Before
+	public void setUp() throws Exception {
+		super.setUp();
+
+		final TestRepository<FileRepository> src = createTestRepository();
+		final String srcName = src.getRepository().getDirectory().getName();
+
+		ServletContextHandler app = server.addContext("/git");
+		GitServlet gs = new GitServlet();
+		gs.setRepositoryResolver(new RepositoryResolver<HttpServletRequest>() {
+			public Repository open(HttpServletRequest req, String name)
+					throws RepositoryNotFoundException,
+					ServiceNotEnabledException {
+				if (!name.equals(srcName))
+					throw new RepositoryNotFoundException(name);
+
+				final Repository db = src.getRepository();
+				db.incrementOpen();
+				return db;
+			}
+		});
+		app.addServlet(new ServletHolder(gs), "/*");
+
+		server.setUp();
+
+		remoteRepository = src.getRepository();
+		remoteURI = toURIish(app, srcName);
+
+		FileBasedConfig cfg = remoteRepository.getConfig();
+		cfg.setBoolean("http", null, "receivepack", true);
+		cfg.save();
+
+		a_blob = src.blob("a");
+	}
+
+	@Test
+	public void testPush_UnpackError_TruncatedPack() throws Exception {
+		StringBuilder sb = new StringBuilder();
+		sb.append(ObjectId.zeroId().name());
+		sb.append(' ');
+		sb.append(a_blob.name());
+		sb.append(' ');
+		sb.append("refs/objects/A");
+		sb.append('\0');
+		sb.append("report-status");
+
+		ByteArrayOutputStream reqbuf = new ByteArrayOutputStream();
+		PacketLineOut reqpck = new PacketLineOut(reqbuf);
+		reqpck.writeString(sb.toString());
+		reqpck.end();
+
+		packHeader(reqbuf, 1);
+
+		byte[] reqbin = reqbuf.toByteArray();
+
+		URL u = new URL(remoteURI.toString() + "/git-receive-pack");
+		HttpURLConnection c = (HttpURLConnection) u.openConnection();
+		try {
+			c.setRequestMethod("POST");
+			c.setDoOutput(true);
+			c.setRequestProperty("Content-Type",
+					GitSmartHttpTools.RECEIVE_PACK_REQUEST_TYPE);
+			c.setFixedLengthStreamingMode(reqbin.length);
+			OutputStream out = c.getOutputStream();
+			try {
+				out.write(reqbin);
+			} finally {
+				out.close();
+			}
+
+			assertEquals(200, c.getResponseCode());
+			assertEquals(GitSmartHttpTools.RECEIVE_PACK_RESULT_TYPE,
+					c.getContentType());
+
+			InputStream rawin = c.getInputStream();
+			try {
+				PacketLineIn pckin = new PacketLineIn(rawin);
+				assertEquals("unpack error "
+						+ JGitText.get().packfileIsTruncated,
+						pckin.readString());
+				assertEquals("ng refs/objects/A n/a (unpacker error)",
+						pckin.readString());
+				assertSame(PacketLineIn.END, pckin.readString());
+			} finally {
+				rawin.close();
+			}
+		} finally {
+			c.disconnect();
+		}
+	}
+
+	private void packHeader(ByteArrayOutputStream tinyPack, int cnt)
+			throws IOException {
+		final byte[] hdr = new byte[8];
+		NB.encodeInt32(hdr, 0, 2);
+		NB.encodeInt32(hdr, 4, cnt);
+
+		tinyPack.write(Constants.PACK_SIGNATURE);
+		tinyPack.write(hdr, 0, 8);
+	}
+}