merge with branch issue-953

scm-webapp/src/main/java/sonia/scm/web/cgi/DefaultCGIExecutor.java

@@ -35,6 +35,7 @@ package sonia.scm.web.cgi;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Strings;
 import com.google.common.io.ByteStreams;
 
@@ -139,12 +140,6 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor
       apendOsEnvironment(env);
     }
 
-    // workaround for mercurial 2.1
-    if (isContentLengthWorkaround())
-    {
-      env.set(ENV_CONTENT_LENGTH, Integer.toString(request.getContentLength()));
-    }
-
     if (workDirectory == null)
     {
       workDirectory = command.getParentFile();
@@ -304,26 +299,10 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor
     String uri = HttpUtil.removeMatrixParameter(request.getRequestURI());
     String scriptName = uri.substring(0, uri.length() - pathInfo.length());
     String scriptPath = context.getRealPath(scriptName);
-    int len = request.getContentLength();
     EnvList env = new EnvList();
 
     env.set(ENV_AUTH_TYPE, request.getAuthType());
-
+    env.set(ENV_CONTENT_LENGTH, createCGIContentLength(request, contentLengthWorkaround));
-    /**
-     * Note CGI spec says CONTENT_LENGTH must be NULL ("") or undefined
-     * if there is no content, so we cannot put 0 or -1 in as per the
-     * Servlet API spec.
-     *
-     * see org.apache.catalina.servlets.CGIServlet
-     */
-    if (len <= 0)
-    {
-      env.set(ENV_CONTENT_LENGTH, "");
-    }
-    else
-    {
-      env.set(ENV_CONTENT_LENGTH, Integer.toString(len));
-    }
 
     /**
      * Decode PATH_INFO
@@ -383,6 +362,39 @@ public class DefaultCGIExecutor extends AbstractCGIExecutor
     return env;
   }
 
+  /**
+   * Returns the content length as string in the cgi specific format.
+   *
+   * CGI spec says CONTENT_LENGTH must be NULL ("") or undefined
+   * if there is no content, so we cannot put 0 or -1 in as per the
+   * Servlet API spec. Some CGI applications require a content
+   * length environment variable, which is not null or empty
+   * (e.g. mercurial). For those application the disallowEmptyResults
+   * parameter should be used.
+   *
+   * @param disallowEmptyResults {@code true} to return -1 instead of an empty string
+   *
+   * @return content length as cgi specific string
+   */
+  @VisibleForTesting
+  static String createCGIContentLength(HttpServletRequest request, boolean disallowEmptyResults) {
+    String cgiContentLength = disallowEmptyResults ? "-1" : "";
+
+    String contentLength = request.getHeader("Content-Length");
+    if (!Strings.isNullOrEmpty(contentLength)) {
+      try {
+        long len = Long.parseLong(contentLength);
+        if (len > 0) {
+          cgiContentLength = String.valueOf(len);
+        }
+      } catch (NumberFormatException ex) {
+        logger.warn("received request with invalid content-length header value: {}", contentLength);
+      }
+    }
+
+    return cgiContentLength;
+  }
+
   /**
    * Method description
    *

scm-webapp/src/test/java/sonia/scm/web/cgi/DefaultCGIExecutorTest.java

@@ -0,0 +1,54 @@
+package sonia.scm.web.cgi;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import javax.servlet.http.HttpServletRequest;
+
+import static org.junit.Assert.*;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link DefaultCGIExecutor}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class DefaultCGIExecutorTest {
+
+  @Mock
+  private HttpServletRequest request;
+
+  @Test
+  public void testCreateCGIContentLength() {
+    when(request.getHeader("Content-Length")).thenReturn("42");
+    assertEquals("42", DefaultCGIExecutor.createCGIContentLength(request, false));
+    assertEquals("42", DefaultCGIExecutor.createCGIContentLength(request, true));
+  }
+
+  @Test
+  public void testCreateCGIContentLengthWithZeroLength() {
+    when(request.getHeader("Content-Length")).thenReturn("0");
+    assertEquals("", DefaultCGIExecutor.createCGIContentLength(request, false));
+    assertEquals("-1", DefaultCGIExecutor.createCGIContentLength(request, true));
+  }
+
+  @Test
+  public void testCreateCGIContentLengthWithoutContentLengthHeader() {
+    assertEquals("", DefaultCGIExecutor.createCGIContentLength(request, false));
+    assertEquals("-1", DefaultCGIExecutor.createCGIContentLength(request, true));
+  }
+
+  @Test
+  public void testCreateCGIContentLengthWithLengthThatExeedsInteger() {
+    when(request.getHeader("Content-Length")).thenReturn("6314297259");
+    assertEquals("6314297259", DefaultCGIExecutor.createCGIContentLength(request, false));
+  }
+
+  @Test
+  public void testCreateCGIContentLengthWithNonNumberHeader() {
+    when(request.getHeader("Content-Length")).thenReturn("abc");
+    assertEquals("", DefaultCGIExecutor.createCGIContentLength(request, false));
+  }
+
+}