merge with branch 1.x

scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java

@@ -35,6 +35,8 @@ package sonia.scm.api.rest.resources;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.net.UrlEscapers;
 import org.apache.commons.beanutils.BeanComparator;
 import org.apache.shiro.authz.AuthorizationException;
 import org.slf4j.Logger;
@@ -45,7 +47,6 @@ import sonia.scm.ModelObject;
 import sonia.scm.PageResult;
 import sonia.scm.api.rest.RestExceptionResult;
 import sonia.scm.util.AssertUtil;
-import sonia.scm.util.HttpUtil;
 import sonia.scm.util.Util;
 
 import javax.ws.rs.core.CacheControl;
@@ -63,6 +64,7 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.Date;
+import java.net.URI;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -139,11 +141,7 @@ public abstract class AbstractManagerResource<T extends ModelObject> {
       manager.create(item);
 
       String id = getId(item);
-
-      id = HttpUtil.encode(id);
-      response = Response.created(
-        uriInfo.getAbsolutePath().resolve(
-          getPathPart().concat("/").concat(id))).build();
+      response = Response.created(location(uriInfo, id)).build();
     }
     catch (AuthorizationException ex)
     {
@@ -159,6 +157,12 @@ public abstract class AbstractManagerResource<T extends ModelObject> {
     return response;
   }
 
+  @VisibleForTesting
+  URI location(UriInfo uriInfo, String id) {
+    String escaped = UrlEscapers.urlPathSegmentEscaper().escape(id);
+    return uriInfo.getAbsolutePath().resolve(getPathPart().concat("/").concat(escaped));
+  }
+
   /**
    * Method description
    *
@@ -247,7 +251,7 @@ public abstract class AbstractManagerResource<T extends ModelObject> {
    */
   public Response get(Request request, String id)
   {
-    Response response = null;
+    Response response;
     T item = manager.get(id);
 
     if (item != null)

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());
-
-    /**
-     * 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));
-    }
+    env.set(ENV_CONTENT_LENGTH, createCGIContentLength(request, contentLengthWorkaround));
 
     /**
      * 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
    *