Ensure all smart HTTP errors are sent to clients

Error messages are typically short, below the 32 KiB in-memory buffer
size of the SmartOutputStream. When an error is queued up for sending
to a client and an exception is thrown up into the servlet handler we
discarded the message and sent nothing to the client, as the messages
were stuck inside of the SmartOutputStream buffer.

Hoist the creation of the output stream above the invocation of try
block of the service, and use close() in the few catch blocks that
assume there are buffered messages ready for transmission. This will
ensure errors from unpacking a stream in ReceivePack are sent off to
a client correctly, as previously these were causing no status report
to arrive at the client side as the data was stuck in the buffer.

Change-Id: I5534b560697731121f48979ae077aa7c95b8e39c

2 files changed, 21 insertions, 15 deletions

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..27bee85d82 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
@@ -158,17 +158,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 +177,9 @@ class ReceivePackServlet extends HttpServlet {
 			getServletContext().log(
 					HttpServerText.get().internalErrorDuringReceivePack,
 					e.getCause());
+			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/UploadPackServlet.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackServlet.java
index c7891dfc77..33bfff6d47 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
@@ -160,22 +160,25 @@ 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()) {
+				out.close();
+			} else if (!rsp.isCommitted()) {
 				rsp.reset();
 				sendError(req, rsp, SC_FORBIDDEN, e.getMessage());
 			}
@@ -186,8 +189,9 @@ class UploadPackServlet extends HttpServlet {
 			getServletContext().log(
 					HttpServerText.get().internalErrorDuringUploadPack,
 					e.getCause());
+			out.close();
 
-		} catch (IOException e) {
+		} catch (Throwable e) {
 			getServletContext().log(HttpServerText.get().internalErrorDuringUploadPack, e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();