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()) {