Merge "http: Allow specifying a custom error handler for UploadPack"

4 files changed, 119 insertions, 27 deletions

diff --git a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF
index d4ce149acb..bad1d8a869 100644
--- a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF
+++ b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF
@@ -18,6 +18,7 @@ Bundle-ActivationPolicy: lazy
 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.eclipse.jgit.annotations;version="[5.5.0,5.7.0)",
  org.eclipse.jgit.errors;version="[5.6.0,5.7.0)",
  org.eclipse.jgit.internal.storage.dfs;version="[5.6.0,5.7.0)",
  org.eclipse.jgit.internal.storage.file;version="[5.6.0,5.7.0)",
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitFilter.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitFilter.java
index 1c5e7ec598..e9462eeb4c 100644
--- a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitFilter.java
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/GitFilter.java
@@ -92,6 +92,8 @@ public class GitFilter extends MetaFilter {
 
 	private UploadPackFactory<HttpServletRequest> uploadPackFactory = new DefaultUploadPackFactory();
 
+	private UploadPackErrorHandler uploadPackErrorHandler;
+
 	private ReceivePackFactory<HttpServletRequest> receivePackFactory = new DefaultReceivePackFactory();
 
 	private final List<Filter> uploadPackFilters = new LinkedList<>();
@@ -150,6 +152,17 @@ public class GitFilter extends MetaFilter {
 	}
 
 	/**
+	 * Set a custom error handler for git-upload-pack.
+	 *
+	 * @param h
+	 *            A custom error handler for git-upload-pack.
+	 */
+	public void setUploadPackErrorHandler(UploadPackErrorHandler h) {
+		assertNotInitialized();
+		this.uploadPackErrorHandler = h;
+	}
+
+	/**
 	 * Add upload-pack filter
 	 *
 	 * @param filter
@@ -212,7 +225,7 @@ public class GitFilter extends MetaFilter {
 			b = b.through(new UploadPackServlet.Factory(uploadPackFactory));
 			for (Filter f : uploadPackFilters)
 				b = b.through(f);
-			b.with(new UploadPackServlet());
+			b.with(new UploadPackServlet(uploadPackErrorHandler));
 		}
 
 		if (receivePackFactory != ReceivePackFactory.DISABLED) {
diff --git a/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java
new file mode 100644
index 0000000000..03be0873b0
--- /dev/null
+++ b/org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java
@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2019, Google LLC  and others
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Distribution License v. 1.0 which is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+package org.eclipse.jgit.http.server;
+
+import java.io.IOException;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jgit.transport.ServiceMayNotContinueException;
+import org.eclipse.jgit.transport.UploadPack;
+
+/**
+ * Handle git-upload-pack errors.
+ *
+ * <p>
+ * This is an entry point for customizing an error handler for git-upload-pack.
+ * Right before calling {@link UploadPack#uploadWithExceptionPropagation}, JGit
+ * will call this handler if specified through {@link GitFilter}. The
+ * implementation of this handler is responsible for calling
+ * {@link UploadPackRunnable} and handling exceptions for clients.
+ *
+ * <p>
+ * If a custom handler is not specified, JGit will use the default error
+ * handler.
+ *
+ * @since 5.6
+ */
+public interface UploadPackErrorHandler {
+	/**
+	 * @param req
+	 *            The HTTP request
+	 * @param rsp
+	 *            The HTTP response
+	 * @param r
+	 *            A continuation that handles a git-upload-pack request.
+	 * @throws IOException
+	 */
+	void upload(HttpServletRequest req, HttpServletResponse rsp,
+			UploadPackRunnable r) throws IOException;
+
+	/** Process a git-upload-pack request. */
+	public interface UploadPackRunnable {
+		/**
+		 * See {@link UploadPack#uploadWithExceptionPropagation}.
+		 *
+		 * @throws ServiceMayNotContinueException
+		 * @throws IOException
+		 */
+		void upload() throws ServiceMayNotContinueException, IOException;
+	}
+}
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 0f4037144a..54561e0cfc 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,8 @@ 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.http.server.UploadPackErrorHandler.UploadPackRunnable;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.InternalHttpServerGlue;
 import org.eclipse.jgit.transport.PacketLineOut;
@@ -181,53 +182,71 @@ class UploadPackServlet extends HttpServlet {
 		}
 	}
 
+	private final UploadPackErrorHandler handler;
+
+	UploadPackServlet(@Nullable UploadPackErrorHandler handler) {
+		this.handler = handler != null ? handler
+				: this::defaultUploadPackHandler;
+	}
+
 	/** {@inheritDoc} */
 	@Override
-	public void doPost(final HttpServletRequest req,
-			final HttpServletResponse rsp) throws IOException {
+	public void doPost(HttpServletRequest req, HttpServletResponse rsp)
+			throws IOException {
 		if (!UPLOAD_PACK_REQUEST_TYPE.equals(req.getContentType())) {
 			rsp.sendError(SC_UNSUPPORTED_MEDIA_TYPE);
 			return;
 		}
 
-		SmartOutputStream out = new SmartOutputStream(req, rsp, false) {
-			@Override
-			public void flush() throws IOException {
-				doFlush();
-			}
-		};
+		UploadPackRunnable r = () -> {
+			UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
+			@SuppressWarnings("resource")
+			SmartOutputStream out = new SmartOutputStream(req, rsp, false) {
+				@Override
+				public void flush() throws IOException {
+					doFlush();
+				}
+			};
 
-		UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
-		try {
 			up.setBiDirectionalPipe(false);
 			rsp.setContentType(UPLOAD_PACK_RESULT_TYPE);
 
-			up.upload(getInputStream(req), out, null);
-			out.close();
-
-		} catch (ServiceMayNotContinueException e) {
-			if (e.isOutput()) {
+			try {
+				up.upload(getInputStream(req), out, null);
+				out.close();
+			} catch (ServiceMayNotContinueException e) {
+				if (e.isOutput()) {
+					consumeRequestBody(req);
+					out.close();
+				}
+				throw e;
+			} catch (UploadPackInternalServerErrorException e) {
+				// Special case exception, error message was sent to client.
+				log(up.getRepository(), e.getCause());
 				consumeRequestBody(req);
 				out.close();
-			} else if (!rsp.isCommitted()) {
-				rsp.reset();
-				sendError(req, rsp, e.getStatusCode(), e.getMessage());
 			}
-			return;
+		};
 
-		} catch (UploadPackInternalServerErrorException e) {
-			// Special case exception, error message was sent to client.
-			log(up.getRepository(), e.getCause());
-			consumeRequestBody(req);
-			out.close();
+		handler.upload(req, rsp, r);
+	}
 
+	private void defaultUploadPackHandler(HttpServletRequest req,
+			HttpServletResponse rsp, UploadPackRunnable r) throws IOException {
+		try {
+			r.upload();
+		} catch (ServiceMayNotContinueException e) {
+			if (!e.isOutput() && !rsp.isCommitted()) {
+				rsp.reset();
+				sendError(req, rsp, e.getStatusCode(), e.getMessage());
+			}
 		} catch (Throwable e) {
+			UploadPack up = (UploadPack) req.getAttribute(ATTRIBUTE_HANDLER);
 			log(up.getRepository(), e);
 			if (!rsp.isCommitted()) {
 				rsp.reset();
 				sendError(req, rsp, SC_INTERNAL_SERVER_ERROR);
 			}
-			return;
 		}
 	}