From abedaf0d3854f319d0df839851c0e95909aec06a Mon Sep 17 00:00:00 2001 From: Masaya Suzuki Date: Tue, 6 Aug 2019 12:02:23 -0700 Subject: [PATCH] http: Allow specifying a custom error handler for UploadPack By abstracting the error handler, this lets a user customize the error handler for UploadPack. A customized error handler can show a custom error message to the clients based on the exception thrown from the hook, create a monitoring system for server errors, or do custom logging. Change-Id: Idd3b87d6bd471fef807c0cf1183e904b2886157e Signed-off-by: Masaya Suzuki --- .../META-INF/MANIFEST.MF | 1 + .../eclipse/jgit/http/server/GitFilter.java | 15 +++- .../http/server/UploadPackErrorHandler.java | 59 +++++++++++++++ .../jgit/http/server/UploadPackServlet.java | 71 ++++++++++++------- 4 files changed, 119 insertions(+), 27 deletions(-) create mode 100644 org.eclipse.jgit.http.server/src/org/eclipse/jgit/http/server/UploadPackErrorHandler.java diff --git a/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF b/org.eclipse.jgit.http.server/META-INF/MANIFEST.MF index d4ce149ac..bad1d8a86 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 1c5e7ec59..e9462eeb4 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 uploadPackFactory = new DefaultUploadPackFactory(); + private UploadPackErrorHandler uploadPackErrorHandler; + private ReceivePackFactory receivePackFactory = new DefaultReceivePackFactory(); private final List uploadPackFilters = new LinkedList<>(); @@ -149,6 +151,17 @@ public void setUploadPackFactory(UploadPackFactory f) { this.uploadPackFactory = f != null ? f : (UploadPackFactory)UploadPackFactory.DISABLED; } + /** + * 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 * @@ -212,7 +225,7 @@ public void init(FilterConfig filterConfig) throws ServletException { 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 000000000..03be0873b --- /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. + * + *

+ * 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. + * + *

+ * 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 0f4037144..54561e0cf 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.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 @@ public void destroy() { } } + 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()) { + } + }; + + 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()); } - return; - - } catch (UploadPackInternalServerErrorException e) { - // Special case exception, error message was sent to client. - log(up.getRepository(), e.getCause()); - consumeRequestBody(req); - out.close(); - } 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; } }