From 11018d1438cef67c9fe80bdef904a13b72494476 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 8 May 2012 21:03:04 +0200 Subject: [PATCH 01/10] improve handling of mercurial errors --- .../sonia/scm/web/HgCGIExceptionHandler.java | 145 ++++++++++++++++++ .../main/java/sonia/scm/web/HgCGIServlet.java | 47 +++++- 2 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java new file mode 100644 index 0000000000..f76d2174f2 --- /dev/null +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java @@ -0,0 +1,145 @@ +/** + * Copyright (c) 2010, Sebastian Sdorra All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. 2. 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. 3. Neither the name of SCM-Manager; + * 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 REGENTS 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. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + + +package sonia.scm.web; + +//~--- non-JDK imports -------------------------------------------------------- + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import sonia.scm.util.IOUtil; +import sonia.scm.util.Util; +import sonia.scm.web.cgi.CGIExceptionHandler; + +//~--- JDK imports ------------------------------------------------------------ + +import java.io.IOException; +import java.io.PrintWriter; + +import java.text.MessageFormat; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * + * @author Sebastian Sdorra + */ +public class HgCGIExceptionHandler implements CGIExceptionHandler +{ + + /** Field description */ + public static final String CONTENT_TYPE_ERROR = "application/hg-error"; + + /** TODO create a bundle for error messages */ + public static final String ERROR_NOT_CONFIGURED = + "The mercurial installation on the scm-manager server seems to be not configured correctly. Please check the settings."; + + /** Field description */ + public static final String ERROR_UNKNOWN = + "There is an unknown error occurred: '{0}'"; + + /** + * the logger for HgCGIExceptionHandler + */ + private static final Logger logger = + LoggerFactory.getLogger(HgCGIExceptionHandler.class); + + //~--- methods -------------------------------------------------------------- + + /** + * Method description + * + * + * @param request + * @param response + * @param ex + */ + @Override + public void handleException(HttpServletRequest request, + HttpServletResponse response, Throwable ex) + { + if (logger.isErrorEnabled()) + { + logger.error("not able to handle mercurial request", ex); + } + + try + { + sendError(response, createErrorMessage(ex)); + } + catch (IOException ioEx) + { + logger.error("could not write error message to client", ioEx); + } + } + + /** + * Method description + * + * + * @param response + * @param message + * + * @throws IOException + */ + public void sendError(HttpServletResponse response, String message) + throws IOException + { + response.setContentType(CONTENT_TYPE_ERROR); + + PrintWriter writer = null; + + try + { + writer = response.getWriter(); + writer.println(message); + } + finally + { + IOUtil.close(writer); + } + } + + /** + * Method description + * + * + * @param ex + * + * @return + */ + private String createErrorMessage(Throwable ex) + { + return MessageFormat.format(ERROR_UNKNOWN, Util.nonNull(ex.getMessage())); + } +} diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java index 0665234ada..8921fae8ad 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java @@ -134,6 +134,7 @@ public class HgCGIServlet extends HttpServlet this.handler = handler; this.hookManager = hookManager; this.requestListenerUtil = requestListenerUtil; + this.exceptionHandler = new HgCGIExceptionHandler(); } //~--- methods -------------------------------------------------------------- @@ -170,9 +171,47 @@ public class HgCGIServlet extends HttpServlet if (repository == null) { - throw new ServletException("repository not found"); - } + if (logger.isDebugEnabled()) + { + logger.debug("no hg repository found at {}", request.getRequestURI()); + } + response.setStatus(HttpServletResponse.SC_NOT_FOUND); + } + else if (!handler.isConfigured()) + { + exceptionHandler.sendError(response, + HgCGIExceptionHandler.ERROR_NOT_CONFIGURED); + } + else + { + try + { + handleRequest(request, response, repository); + } + catch (Exception ex) + { + exceptionHandler.handleException(request, response, ex); + } + } + } + + /** + * Method description + * + * + * @param request + * @param response + * @param repository + * + * @throws IOException + * @throws ServletException + */ + private void handleRequest(HttpServletRequest request, + HttpServletResponse response, + Repository repository) + throws ServletException, IOException + { if (requestListenerUtil.callListeners(request, response, repository)) { process(request, response, repository); @@ -229,7 +268,6 @@ public class HgCGIServlet extends HttpServlet String name = repository.getName(); File directory = handler.getDirectory(repository); String pythonPath = HgUtil.getPythonPath(handler.getConfig()); - CGIExecutor executor = cgiExecutorFactory.createExecutor(configuration, getServletContext(), request, response); @@ -293,6 +331,9 @@ public class HgCGIServlet extends HttpServlet /** Field description */ private ScmConfiguration configuration; + /** Field description */ + private HgCGIExceptionHandler exceptionHandler; + /** Field description */ private HgRepositoryHandler handler; From 09c60730ce770d77641591a12e3c32786a2264eb Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 8 May 2012 21:28:41 +0200 Subject: [PATCH 02/10] improve handling of process return codes --- .../scm/web/cgi/AbstractCGIExecutor.java | 58 ++++++++++++++ .../java/sonia/scm/web/cgi/CGIExecutor.java | 18 +++++ .../scm/web/cgi/CGIStatusCodeHandler.java | 59 ++++++++++++++ .../sonia/scm/web/cgi/DefaultCGIExecutor.java | 46 ++++++----- .../web/cgi/DefaultCGIStatusCodeHandler.java | 76 +++++++++++++++++++ 5 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java create mode 100644 scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java diff --git a/scm-core/src/main/java/sonia/scm/web/cgi/AbstractCGIExecutor.java b/scm-core/src/main/java/sonia/scm/web/cgi/AbstractCGIExecutor.java index 74e23aec64..a4271e613c 100644 --- a/scm-core/src/main/java/sonia/scm/web/cgi/AbstractCGIExecutor.java +++ b/scm-core/src/main/java/sonia/scm/web/cgi/AbstractCGIExecutor.java @@ -68,6 +68,19 @@ public abstract class AbstractCGIExecutor implements CGIExecutor return environment; } + /** + * {@inheritDoc} + * + * + * @return + * @since 1.15 + */ + @Override + public CGIExceptionHandler getExceptionHandler() + { + return exceptionHandler; + } + /** * Method description * @@ -80,6 +93,19 @@ public abstract class AbstractCGIExecutor implements CGIExecutor return interpreter; } + /** + * {@inheritDoc} + * + * + * @return + * @since 1.15 + */ + @Override + public CGIStatusCodeHandler getStatusCodeHandler() + { + return statusCodeHandler; + } + /** * Method description * @@ -142,6 +168,19 @@ public abstract class AbstractCGIExecutor implements CGIExecutor this.environment = environment; } + /** + * {@inheritDoc} + * + * + * @param exceptionHandler + * @since 1.15 + */ + @Override + public void setExceptionHandler(CGIExceptionHandler exceptionHandler) + { + this.exceptionHandler = exceptionHandler; + } + /** * Method description * @@ -178,6 +217,19 @@ public abstract class AbstractCGIExecutor implements CGIExecutor this.passShellEnvironment = passShellEnvironment; } + /** + * {@inheritDoc} + * + * + * @param statusCodeHandler + * @since 1.15 + */ + @Override + public void setStatusCodeHandler(CGIStatusCodeHandler statusCodeHandler) + { + this.statusCodeHandler = statusCodeHandler; + } + /** * Method description * @@ -198,6 +250,9 @@ public abstract class AbstractCGIExecutor implements CGIExecutor /** Field description */ protected EnvList environment; + /** Field description */ + protected CGIExceptionHandler exceptionHandler; + /** Field description */ protected boolean ignoreExitCode = false; @@ -207,6 +262,9 @@ public abstract class AbstractCGIExecutor implements CGIExecutor /** Field description */ protected boolean passShellEnvironment = false; + /** Field description */ + protected CGIStatusCodeHandler statusCodeHandler; + /** Field description */ protected File workDirectory; } diff --git a/scm-core/src/main/java/sonia/scm/web/cgi/CGIExecutor.java b/scm-core/src/main/java/sonia/scm/web/cgi/CGIExecutor.java index 19951a2267..19239dcbdd 100644 --- a/scm-core/src/main/java/sonia/scm/web/cgi/CGIExecutor.java +++ b/scm-core/src/main/java/sonia/scm/web/cgi/CGIExecutor.java @@ -176,6 +176,15 @@ public interface CGIExecutor */ public String getInterpreter(); + /** + * Returns the status code handler. + * + * + * @return status code handler + * @since 1.15 + */ + public CGIStatusCodeHandler getStatusCodeHandler(); + /** * Method description * @@ -268,6 +277,15 @@ public interface CGIExecutor */ public void setPassShellEnvironment(boolean passShellEnvironment); + /** + * Sets the status code handler. + * + * + * @param statusCodeHandler the handler to set + * @since 1.15 + */ + public void setStatusCodeHandler(CGIStatusCodeHandler statusCodeHandler); + /** * Method description * diff --git a/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java b/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java new file mode 100644 index 0000000000..430dfb82e5 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java @@ -0,0 +1,59 @@ +/** + * Copyright (c) 2010, Sebastian Sdorra All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. 2. 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. 3. Neither the name of SCM-Manager; + * 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 REGENTS 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. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + + +package sonia.scm.web.cgi; + +//~--- JDK imports ------------------------------------------------------------ + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * Interface for handling return codes of processes + * executed by the {@link CGIExecutor}. + * + * @author Sebastian Sdorra + * @since 1.15 + */ +public interface CGIStatusCodeHandler +{ + + /** + * Handles the return code of the process executed by {@link CGIExecutor}. + * + * + * @param request the http request + * @param response the http response + * @param statusCode process return code + */ + public void handleStatusCode(HttpServletRequest request, + HttpServletResponse response, int statusCode); +} diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java index 9705631a9f..a936a07059 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java @@ -210,6 +210,23 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor return exceptionHandler; } + /** + * Method description + * + * + * @return + */ + @Override + public CGIStatusCodeHandler getStatusCodeHandler() + { + if (statusCodeHandler == null) + { + statusCodeHandler = new DefaultCGIStatusCodeHandler(); + } + + return statusCodeHandler; + } + /** * Method description * @@ -237,18 +254,6 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor this.contentLengthWorkaround = contentLengthWorkaround; } - /** - * Method description - * - * - * @param exceptionHandler - */ - @Override - public void setExceptionHandler(CGIExceptionHandler exceptionHandler) - { - this.exceptionHandler = exceptionHandler; - } - //~--- methods -------------------------------------------------------------- /** @@ -582,9 +587,19 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor { int exitCode = process.waitFor(); - if ((exitCode != 0) &&!ignoreExitCode) + if (!ignoreExitCode) { - logger.warn("process ends with exit code {}", exitCode); + if (logger.isTraceEnabled()) + { + logger.trace("handle status code {} with statusCodeHandler", + exitCode); + } + + getStatusCodeHandler().handleStatusCode(request, response, exitCode); + } + else if (logger.isDebugEnabled()) + { + logger.debug("ignore status code {}", exitCode); } } catch (InterruptedException ex) @@ -629,9 +644,6 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor /** Field description */ private ServletContext context; - /** Field description */ - private CGIExceptionHandler exceptionHandler; - /** Field description */ private HttpServletRequest request; diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java new file mode 100644 index 0000000000..005394b109 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java @@ -0,0 +1,76 @@ +/** + * Copyright (c) 2010, Sebastian Sdorra All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. 2. 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. 3. Neither the name of SCM-Manager; + * 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 REGENTS 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. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + + +package sonia.scm.web.cgi; + +//~--- non-JDK imports -------------------------------------------------------- + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +//~--- JDK imports ------------------------------------------------------------ + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +/** + * + * @author Sebastian Sdorra + */ +public class DefaultCGIStatusCodeHandler implements CGIStatusCodeHandler +{ + + /** + * the logger for DefaultCGIStatusCodeHandler + */ + private static final Logger logger = + LoggerFactory.getLogger(DefaultCGIStatusCodeHandler.class); + + //~--- methods -------------------------------------------------------------- + + /** + * Method description + * + * + * @param request + * @param response + * @param statusCode + */ + @Override + public void handleStatusCode(HttpServletRequest request, + HttpServletResponse response, int statusCode) + { + if (statusCode != 0) + { + logger.warn("process ends with exit code {}", statusCode); + } + } +} From 5ededccd10ac2deec313451e998c51852e6c94ee Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 8 May 2012 21:29:48 +0200 Subject: [PATCH 03/10] use execption handler to handle process interrupt exception --- .../src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java index a936a07059..227ca49286 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java @@ -604,7 +604,7 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor } catch (InterruptedException ex) { - logger.error("process interrupted", ex); + getExceptionHandler().handleException(request, response, ex); } } From e85ff1317c721dacb3c4ae16159cdc871ed9d312 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 8 May 2012 22:10:34 +0200 Subject: [PATCH 04/10] pass outputstream to status code handler to improve mercurial error handling --- .../scm/web/cgi/CGIStatusCodeHandler.java | 25 +++++- .../sonia/scm/web/HgCGIExceptionHandler.java | 77 +++++++++++++++++-- .../main/java/sonia/scm/web/HgCGIServlet.java | 2 + .../sonia/scm/web/cgi/DefaultCGIExecutor.java | 60 +++++++-------- 4 files changed, 125 insertions(+), 39 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java b/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java index 430dfb82e5..5efb6fe1e1 100644 --- a/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java +++ b/scm-core/src/main/java/sonia/scm/web/cgi/CGIStatusCodeHandler.java @@ -33,6 +33,9 @@ package sonia.scm.web.cgi; //~--- JDK imports ------------------------------------------------------------ +import java.io.IOException; +import java.io.OutputStream; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -48,12 +51,32 @@ public interface CGIStatusCodeHandler /** * Handles the return code of the process executed by {@link CGIExecutor}. + * <b>Note:</b> This method is called when the process has + * already written to the {@link OutputStream}. + * + * + * @param request the http request + * @param statusCode process return code + */ + public void handleStatusCode(HttpServletRequest request, int statusCode); + + /** + * Handles the return code of the process executed by {@link CGIExecutor}. + * <b>Note:</b> This method is only called when the process has + * not written to the {@link OutputStream}. Do not call + * {@link HttpServletResponse#getWriter()}, because there was already a call + * to {@link HttpServletResponse#getOutputStream()}. * * * @param request the http request * @param response the http response + * @param ouputStream the servlet output stream * @param statusCode process return code + * + * @throws IOException */ public void handleStatusCode(HttpServletRequest request, - HttpServletResponse response, int statusCode); + HttpServletResponse response, + OutputStream ouputStream, int statusCode) + throws IOException; } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java index f76d2174f2..4ce938d26e 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java @@ -33,16 +33,20 @@ package sonia.scm.web; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.base.Charsets; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.util.IOUtil; import sonia.scm.util.Util; import sonia.scm.web.cgi.CGIExceptionHandler; +import sonia.scm.web.cgi.CGIStatusCodeHandler; //~--- JDK imports ------------------------------------------------------------ import java.io.IOException; +import java.io.OutputStream; import java.io.PrintWriter; import java.text.MessageFormat; @@ -54,7 +58,8 @@ import javax.servlet.http.HttpServletResponse; * * @author Sebastian Sdorra */ -public class HgCGIExceptionHandler implements CGIExceptionHandler +public class HgCGIExceptionHandler + implements CGIExceptionHandler, CGIStatusCodeHandler { /** Field description */ @@ -64,6 +69,10 @@ public class HgCGIExceptionHandler implements CGIExceptionHandler public static final String ERROR_NOT_CONFIGURED = "The mercurial installation on the scm-manager server seems to be not configured correctly. Please check the settings."; + /** Field description */ + public static final String ERROR_STATUSCODE = + "Mercurial process ends with return code {0}"; + /** Field description */ public static final String ERROR_UNKNOWN = "There is an unknown error occurred: '{0}'"; @@ -93,13 +102,65 @@ public class HgCGIExceptionHandler implements CGIExceptionHandler logger.error("not able to handle mercurial request", ex); } - try + sendError(response, createErrorMessage(ex)); + } + + /** + * Method description + * + * + * @param request + * @param response + * @param output + * @param statusCode + * + * @throws IOException + */ + @Override + public void handleStatusCode(HttpServletRequest request, + HttpServletResponse response, + OutputStream output, int statusCode) + throws IOException + { + if (statusCode != 0) { - sendError(response, createErrorMessage(ex)); + response.setContentType(CONTENT_TYPE_ERROR); + + String msg = MessageFormat.format(ERROR_STATUSCODE, statusCode); + + if (logger.isWarnEnabled()) + { + logger.warn(msg); + } + + output.write(msg.getBytes(Charsets.US_ASCII)); } - catch (IOException ioEx) + else if (logger.isDebugEnabled()) { - logger.error("could not write error message to client", ioEx); + logger.debug("mercurial process ends successfully"); + } + } + + /** + * Method description + * + * + * @param request + * @param statusCode + */ + @Override + public void handleStatusCode(HttpServletRequest request, int statusCode) + { + if (statusCode != 0) + { + if (logger.isWarnEnabled()) + { + logger.error("mercurial process ends with {}", statusCode); + } + } + else if (logger.isDebugEnabled()) + { + logger.debug("mercurial process ends successfully"); } } @@ -110,10 +171,8 @@ public class HgCGIExceptionHandler implements CGIExceptionHandler * @param response * @param message * - * @throws IOException */ public void sendError(HttpServletResponse response, String message) - throws IOException { response.setContentType(CONTENT_TYPE_ERROR); @@ -124,6 +183,10 @@ public class HgCGIExceptionHandler implements CGIExceptionHandler writer = response.getWriter(); writer.println(message); } + catch (IOException ex) + { + logger.error("could not write error message to client", ex); + } finally { IOUtil.close(writer); diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java index 8921fae8ad..a364278f49 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java @@ -271,6 +271,8 @@ public class HgCGIServlet extends HttpServlet CGIExecutor executor = cgiExecutorFactory.createExecutor(configuration, getServletContext(), request, response); + executor.setExceptionHandler(exceptionHandler); + executor.setStatusCodeHandler(exceptionHandler); executor.setContentLengthWorkaround(true); executor.getEnvironment().set(ENV_REPOSITORY_NAME, name); executor.getEnvironment().set(ENV_REPOSITORY_PATH, diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java index 227ca49286..c96d981b29 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java @@ -35,6 +35,8 @@ package sonia.scm.web.cgi; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.io.ByteStreams; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -389,6 +391,7 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor { InputStream processIS = null; InputStream processES = null; + ServletOutputStream servletOS = null; try { @@ -396,13 +399,18 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor processErrorStreamAsync(processES); processServletInput(process); processIS = process.getInputStream(); - processProcessInputStream(processIS); - waitForFinish(process); + servletOS = response.getOutputStream(); + parseHeaders(processIS); + + long content = ByteStreams.copy(processIS, servletOS); + + waitForFinish(process, servletOS, content); } finally { IOUtil.close(processIS); IOUtil.close(processES); + IOUtil.close(servletOS); } } @@ -519,31 +527,6 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor }).start(); } - /** - * Method description - * - * - * @param is - * - * @throws IOException - */ - private void processProcessInputStream(InputStream is) throws IOException - { - parseHeaders(is); - - ServletOutputStream servletOS = null; - - try - { - servletOS = response.getOutputStream(); - IOUtil.copy(is, servletOS, bufferSize); - } - finally - { - IOUtil.close(servletOS); - } - } - /** * Method description * @@ -579,9 +562,15 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor * * * @param process + * @param output + * @param content * + * + * @throws IOException */ - private void waitForFinish(Process process) + private void waitForFinish(Process process, ServletOutputStream output, + long content) + throws IOException { try { @@ -591,11 +580,20 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor { if (logger.isTraceEnabled()) { - logger.trace("handle status code {} with statusCodeHandler", - exitCode); + logger.trace( + "handle status code {} with statusCodeHandler, there are {} bytes written to outputstream", + exitCode, content); } - getStatusCodeHandler().handleStatusCode(request, response, exitCode); + if (content == 0) + { + getStatusCodeHandler().handleStatusCode(request, response, output, + exitCode); + } + else + { + getStatusCodeHandler().handleStatusCode(request, exitCode); + } } else if (logger.isDebugEnabled()) { From f6ff83dd888cd5961e32408f8fb10338f1b68b24 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 8 May 2012 22:12:51 +0200 Subject: [PATCH 05/10] fix missing errorstream output in some situations --- .../sonia/scm/web/cgi/DefaultCGIExecutor.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java index c96d981b29..03c20574cd 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java @@ -390,13 +390,11 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor private void execute(Process process) throws IOException { InputStream processIS = null; - InputStream processES = null; ServletOutputStream servletOS = null; try { - processES = process.getErrorStream(); - processErrorStreamAsync(processES); + processErrorStreamAsync(process); processServletInput(process); processIS = process.getInputStream(); servletOS = response.getOutputStream(); @@ -409,7 +407,6 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor finally { IOUtil.close(processIS); - IOUtil.close(processES); IOUtil.close(servletOS); } } @@ -506,23 +503,31 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor * Method description * * - * @param errorStream + * + * @param process */ - private void processErrorStreamAsync(final InputStream errorStream) + private void processErrorStreamAsync(final Process process) { new Thread(new Runnable() { @Override public void run() { + InputStream errorStream = null; + try { + errorStream = process.getErrorStream(); processErrorStream(errorStream); } catch (IOException ex) { logger.error("could not read errorstream", ex); } + finally + { + IOUtil.close(errorStream); + } } }).start(); } From 33abc06a716833445beaa6f5beec635307d3a75c Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 9 May 2012 20:49:22 +0200 Subject: [PATCH 06/10] added bundle class to handle serverside i18n messages --- .../src/main/java/sonia/scm/i18n/Bundle.java | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 scm-core/src/main/java/sonia/scm/i18n/Bundle.java diff --git a/scm-core/src/main/java/sonia/scm/i18n/Bundle.java b/scm-core/src/main/java/sonia/scm/i18n/Bundle.java new file mode 100644 index 0000000000..bdf3487ef2 --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/i18n/Bundle.java @@ -0,0 +1,140 @@ +/** + * Copyright (c) 2010, Sebastian Sdorra All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. 2. 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. 3. Neither the name of SCM-Manager; + * 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 REGENTS 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. + * + * http://bitbucket.org/sdorra/scm-manager + * + */ + + + +package sonia.scm.i18n; + +//~--- non-JDK imports -------------------------------------------------------- + +import sonia.scm.util.Util; + +//~--- JDK imports ------------------------------------------------------------ + +import java.text.MessageFormat; + +import java.util.Locale; +import java.util.ResourceBundle; + +/** + * This class is a wrapper for {@link ResourceBundle}, it applies some missing + * format options missing in {@link ResourceBundle}. + * + * @author Sebastian Sdorra + * @since 1.15 + */ +public class Bundle +{ + + /** Field description */ + private static final String SEPARATOR = System.getProperty("line.separator"); + + //~--- constructors --------------------------------------------------------- + + /** + * Constructs ... + * + * + * @param bundle + */ + private Bundle(ResourceBundle bundle) + { + this.bundle = bundle; + } + + //~--- get methods ---------------------------------------------------------- + + /** + * Creates a new bundle instance + * + * + * @param path path to the properties file + * + * @return new bundle instance + */ + public static Bundle getBundle(String path) + { + return new Bundle(ResourceBundle.getBundle(path)); + } + + /** + * Creates a new bundle instance + * + * + * @param path path to the properties file + * @param locale locale for the properties file + * + * @return new bundle instance + */ + public static Bundle getBundle(String path, Locale locale) + { + return new Bundle(ResourceBundle.getBundle(path, locale)); + } + + /** + * This method returns the same value as + * {@link #getString(java.lang.String, java.lang.Object[])} + * with a line separator at the end. + * + * @param key key in the properties file + * @param args format arguments + * + * @return formated message + */ + public String getLine(String key, Object... args) + { + return getString(key, args).concat(SEPARATOR); + } + + /** + * Returns the value of the key, formatted with {@link MessageFormat} + * + * + * @param key key in the properties file + * @param args format arguments + * + * @return formated message + */ + public String getString(String key, Object... args) + { + String msg = bundle.getString(key); + + if (Util.isNotEmpty(args)) + { + msg = MessageFormat.format(key, args); + } + + return msg; + } + + //~--- fields --------------------------------------------------------------- + + /** Field description */ + private ResourceBundle bundle; +} From 860230f6dcd74b828765d708bf75026277318895 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 9 May 2012 20:54:39 +0200 Subject: [PATCH 07/10] fix wrong message in bundle class --- scm-core/src/main/java/sonia/scm/i18n/Bundle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-core/src/main/java/sonia/scm/i18n/Bundle.java b/scm-core/src/main/java/sonia/scm/i18n/Bundle.java index bdf3487ef2..9523948cd7 100644 --- a/scm-core/src/main/java/sonia/scm/i18n/Bundle.java +++ b/scm-core/src/main/java/sonia/scm/i18n/Bundle.java @@ -127,7 +127,7 @@ public class Bundle if (Util.isNotEmpty(args)) { - msg = MessageFormat.format(key, args); + msg = MessageFormat.format(msg, args); } return msg; From ac6c366a60f6de71e6c1e74775571513e2b502dc Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 9 May 2012 21:06:41 +0200 Subject: [PATCH 08/10] use resource bundle for mercurial cgi error messages --- .../sonia/scm/web/HgCGIExceptionHandler.java | 45 ++++++++++++------- .../main/java/sonia/scm/web/HgCGIServlet.java | 4 +- .../sonia/scm/web/cgimessages.properties | 32 +++++++++++++ 3 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/web/cgimessages.properties diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java index 4ce938d26e..5c67d147e9 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIExceptionHandler.java @@ -38,6 +38,7 @@ import com.google.common.base.Charsets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import sonia.scm.i18n.Bundle; import sonia.scm.util.IOUtil; import sonia.scm.util.Util; import sonia.scm.web.cgi.CGIExceptionHandler; @@ -49,8 +50,6 @@ import java.io.IOException; import java.io.OutputStream; import java.io.PrintWriter; -import java.text.MessageFormat; - import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -62,20 +61,20 @@ public class HgCGIExceptionHandler implements CGIExceptionHandler, CGIStatusCodeHandler { + /** Field description */ + public static final String BUNDLE_PATH = "sonia.scm.web.cgimessages"; + /** Field description */ public static final String CONTENT_TYPE_ERROR = "application/hg-error"; /** TODO create a bundle for error messages */ - public static final String ERROR_NOT_CONFIGURED = - "The mercurial installation on the scm-manager server seems to be not configured correctly. Please check the settings."; + public static final String ERROR_NOT_CONFIGURED = "error.notConfigured"; /** Field description */ - public static final String ERROR_STATUSCODE = - "Mercurial process ends with return code {0}"; + public static final String ERROR_STATUSCODE = "error.statusCode"; /** Field description */ - public static final String ERROR_UNKNOWN = - "There is an unknown error occurred: '{0}'"; + public static final String ERROR_UNEXPECTED = "error.unexpected"; /** * the logger for HgCGIExceptionHandler @@ -83,6 +82,17 @@ public class HgCGIExceptionHandler private static final Logger logger = LoggerFactory.getLogger(HgCGIExceptionHandler.class); + //~--- constructors --------------------------------------------------------- + + /** + * Constructs ... + * + */ + public HgCGIExceptionHandler() + { + this.bundle = Bundle.getBundle(BUNDLE_PATH); + } + //~--- methods -------------------------------------------------------------- /** @@ -102,7 +112,8 @@ public class HgCGIExceptionHandler logger.error("not able to handle mercurial request", ex); } - sendError(response, createErrorMessage(ex)); + sendError(response, + bundle.getString(ERROR_UNEXPECTED, Util.nonNull(ex.getMessage()))); } /** @@ -126,7 +137,7 @@ public class HgCGIExceptionHandler { response.setContentType(CONTENT_TYPE_ERROR); - String msg = MessageFormat.format(ERROR_STATUSCODE, statusCode); + String msg = bundle.getLine(ERROR_STATUSCODE, statusCode); if (logger.isWarnEnabled()) { @@ -197,12 +208,16 @@ public class HgCGIExceptionHandler * Method description * * - * @param ex - * - * @return + * @param response + * @param key */ - private String createErrorMessage(Throwable ex) + public void sendFormattedError(HttpServletResponse response, String key) { - return MessageFormat.format(ERROR_UNKNOWN, Util.nonNull(ex.getMessage())); + sendError(response, bundle.getString(key)); } + + //~--- fields --------------------------------------------------------------- + + /** Field description */ + private Bundle bundle; } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java index a364278f49..ac6884dfaa 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/web/HgCGIServlet.java @@ -180,8 +180,8 @@ public class HgCGIServlet extends HttpServlet } else if (!handler.isConfigured()) { - exceptionHandler.sendError(response, - HgCGIExceptionHandler.ERROR_NOT_CONFIGURED); + exceptionHandler.sendFormattedError(response, + HgCGIExceptionHandler.ERROR_NOT_CONFIGURED); } else { diff --git a/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/web/cgimessages.properties b/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/web/cgimessages.properties new file mode 100644 index 0000000000..6fac1956dd --- /dev/null +++ b/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/web/cgimessages.properties @@ -0,0 +1,32 @@ +# Copyright (c) 2010, Sebastian Sdorra +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# 2. 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. +# 3. Neither the name of SCM-Manager; 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 REGENTS 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. +# +# http://bitbucket.org/sdorra/scm-manager +# + +error.notConfigured = The mercurial installation on the scm-manager server seems to be not configured correctly. Please check the settings. +error.statusCode = Mercurial/Python process ends with return code {0} +error.unexpected = There is an unexpected error occurred: {0} \ No newline at end of file From bd0f968b4c194ea929bcf9941a37bae17f837eea Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 9 May 2012 21:21:09 +0200 Subject: [PATCH 09/10] fix broken build --- .../web/cgi/DefaultCGIStatusCodeHandler.java | 27 ++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java index 005394b109..3d5eee88cd 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java +++ b/scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIStatusCodeHandler.java @@ -38,6 +38,9 @@ import org.slf4j.LoggerFactory; //~--- JDK imports ------------------------------------------------------------ +import java.io.IOException; +import java.io.OutputStream; + import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -61,16 +64,34 @@ public class DefaultCGIStatusCodeHandler implements CGIStatusCodeHandler * * * @param request - * @param response * @param statusCode */ @Override - public void handleStatusCode(HttpServletRequest request, - HttpServletResponse response, int statusCode) + public void handleStatusCode(HttpServletRequest request, int statusCode) { if (statusCode != 0) { logger.warn("process ends with exit code {}", statusCode); } } + + /** + * Method description + * + * + * @param request + * @param response + * @param ouputStream + * @param statusCode + * + * @throws IOException + */ + @Override + public void handleStatusCode(HttpServletRequest request, + HttpServletResponse response, + OutputStream ouputStream, int statusCode) + throws IOException + { + handleStatusCode(request, statusCode); + } } From ecb72d506e43de74dc29bb7fc1c685a7c3e69f0e Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 10 May 2012 18:30:26 +0200 Subject: [PATCH 10/10] close branch issue-138