Add property to configure path separator in URLs for a gitweb service

In URLs to a gitweb service, Gerrit by default will escape slashes in
project and branch names replacing them with the corresponding hexa-
decimal encoding. Some web servers, such as Tomcat, reject hexadecimal
encoded slashes in the URL.

The gitweb service GitBlit, allows to configure an alternative path
separator character (e.g. '*'). This change introduces the property
"gitweb.pathSeparator", which allows to configure a path separator in
Gerrit matching the path separator used by the gitweb service.

Change-Id: I0c67e6264abfa156e4c513e1fad17dac141fa7d3
Signed-off-by: Adrian Goerler <adrian.goerler@sap.com>

Documentation/config-gerrit.txt

@@ -1118,6 +1118,23 @@ in the Gerrit web-UI.
 +
 Default linkname for custom type is "gitweb".
 
+[[gitweb.pathSeparator]]gitweb.pathSeparator::
++
+Optional character to substitute the standard path separator (slash) in
+project names and branch names.
++
+By default, Gerrit will use hexadecimal encoding for slashes in project and
+branch names. Some web servers, such as Tomcat, reject this hexadecimal
+encoding in the URL.
++
+Some alternative gitweb services, such as link:http://gitblit.com[Gitblit],
+allow using an alternative path separator character. In Gitblit, this can be
+configured through the property link:http://gitblit.com/properties.html[web.forwardSlashCharacter].
+In Gerrit, the alternative path separator can be configured correspondingly
+using the property 'gitweb.pathSeparator'.
++
+Valid values are the characters '*', '(' and ')'.
+
 [[hooks]]Section hooks
 ~~~~~~~~~~~~~~~~~~~~~~~~
 

gerrit-common/src/main/java/com/google/gerrit/common/data/GitWebType.java

@@ -71,6 +71,10 @@ public class GitWebType {
   /** ParamertizedString for file history view url. */
   private String fileHistory;
 
+  /** Character to substitute the standard path separator '/' in branch and
+    * project names */
+  private char pathSeparator = '/';
+
   /** Private default constructor for gson. */
   protected GitWebType() {
   }
@@ -174,4 +178,27 @@ public class GitWebType {
       fileHistory = pattern;
     }
   }
+
+  /**
+   * Replace the standard path separator ('/') in a branch name or project
+   * name with a custom path separator configured by the property
+   * gitweb.pathSeparator.
+   * @param urlSegment The branch or project to replace the path separator in
+   * @return the urlSegment with the standard path separator replaced by the
+   * custom path separator
+   */
+  public String replacePathSeparator(String urlSegment) {
+    if ('/' != pathSeparator) {
+      return urlSegment.replace('/', pathSeparator);
+    }
+    return urlSegment;
+  }
+
+  /**
+   * Set the custom path separator
+   * @param separator The custom path separator
+   */
+  public void setPathSeparator(char separator) {
+    this.pathSeparator = separator;
+  }
 }

gerrit-common/src/main/java/com/google/gerrit/common/data/GitwebLink.java

@@ -44,8 +44,8 @@ public class GitwebLink {
     ParameterizedString pattern = new ParameterizedString(type.getRevision());
 
     final Map<String, String> p = new HashMap<String, String>();
-    p.put("project", URL.encodeQueryString(project.get()));
-    p.put("commit", URL.encodeQueryString(ps.getRevision().get()));
+    p.put("project", encode(project.get()));
+    p.put("commit", encode(ps.getRevision().get()));
     return baseUrl + pattern.replace(p);
   }
 
@@ -53,7 +53,7 @@ public class GitwebLink {
     ParameterizedString pattern = new ParameterizedString(type.getProject());
 
     final Map<String, String> p = new HashMap<String, String>();
-    p.put("project", URL.encodeQueryString(project.get()));
+    p.put("project", encode(project.get()));
     return baseUrl + pattern.replace(p);
   }
 
@@ -61,8 +61,8 @@ public class GitwebLink {
     ParameterizedString pattern = new ParameterizedString(type.getBranch());
 
     final Map<String, String> p = new HashMap<String, String>();
-    p.put("project", URL.encodeQueryString(branch.getParentKey().get()));
-    p.put("branch", URL.encodeQueryString(branch.get()));
+    p.put("project", encode(branch.getParentKey().get()));
+    p.put("branch", encode(branch.get()));
     return baseUrl + pattern.replace(p);
   }
 
@@ -70,9 +70,13 @@ public class GitwebLink {
     ParameterizedString pattern = new ParameterizedString(type.getFileHistory());
 
     final Map<String, String> p = new HashMap<String, String>();
-    p.put("project", URL.encodeQueryString(branch.getParentKey().get()));
-    p.put("branch", URL.encodeQueryString(branch.get()));
-    p.put("file", URL.encodeQueryString(file));
+    p.put("project", encode(branch.getParentKey().get()));
+    p.put("branch", encode(branch.get()));
+    p.put("file", encode(file));
     return baseUrl + pattern.replace(p);
   }
+
+  private String encode(String segment) {
+    return URL.encodeQueryString(type.replacePathSeparator(segment));
+  }
 }

gerrit-common/src/test/java/com/google/gerrit/common/data/EncodePathSeparatorTest.java

@@ -0,0 +1,36 @@
+// Copyright (C) 2011 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.common.data;
+
+import junit.framework.TestCase;
+
+public class EncodePathSeparatorTest extends TestCase {
+
+  public void testDefaultBehaviour() {
+
+    GitWebType gitWebType = GitWebType.fromName(null);
+
+    assertEquals("a/b", gitWebType.replacePathSeparator("a/b"));
+  }
+
+  public void testExclamationMark() {
+
+    GitWebType gitWebType = GitWebType.fromName(null);
+    gitWebType.setPathSeparator('!');
+
+    assertEquals("a!b", gitWebType.replacePathSeparator("a/b"));
+  }
+
+}

gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitWebConfig.java

@@ -54,6 +54,19 @@ public class GitWebConfig {
     type.setBranch(cfg.getString("gitweb", null, "branch"));
     type.setProject(cfg.getString("gitweb", null, "project"));
     type.setRevision(cfg.getString("gitweb", null, "revision"));
+    String pathSeparator = cfg.getString("gitweb", null, "pathSeparator");
+    if (pathSeparator != null) {
+      if (pathSeparator.length() == 1) {
+        char c = pathSeparator.charAt(0);
+        if (isValidPathSeparator(c)) {
+          type.setPathSeparator(c);
+        } else {
+          log.warn("Invalid value specified for gitweb.pathSeparator: " + c);
+        }
+      } else {
+        log.warn("Value specified for gitweb.pathSeparator is not a single character:" + pathSeparator);
+      }
+    }
 
     if (type.getBranch() == null) {
       log.warn("No Pattern specified for gitweb.branch, disabling.");
@@ -182,4 +195,31 @@ public class GitWebConfig {
   public File getGitLogoPNG() {
     return git_logo_png;
   }
+
+  /**
+   * Determines if a given character can be used unencoded in an URL as a
+   * replacement for the path separator '/'.
+   *
+   * Reasoning: http://www.ietf.org/rfc/rfc1738.txt § 2.2:
+   *
+   * ... only alphanumerics, the special characters "$-_.+!*'(),", and
+   *  reserved characters used for their reserved purposes may be used
+   * unencoded within a URL.
+   *
+   * The following characters might occur in file names, however:
+   *
+   * alphanumeric characters,
+   *
+   * "$-_.+!',"
+   */
+  static boolean isValidPathSeparator(char c) {
+    switch (c) {
+      case '*':
+      case '(':
+      case ')':
+        return true;
+      default:
+        return false;
+    }
+  }
 }

gerrit-httpd/src/test/java/com/google/gerrit/httpd/GitWebConfigTest.java

@@ -0,0 +1,35 @@
+// Copyright (C) 2011 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import junit.framework.TestCase;
+
+public class GitWebConfigTest extends TestCase {
+
+  private static final String VALID_CHARACTERS = "*()";
+  private static final String SOME_INVALID_CHARACTERS = "09AZaz$-_.+!',";
+
+  public void testValidPathSeparator() {
+    for(char c : VALID_CHARACTERS.toCharArray()) {
+      assertTrue("valid character rejected: " + c, GitWebConfig.isValidPathSeparator(c));
+    }
+  }
+
+  public void testInalidPathSeparator() {
+    for(char c : SOME_INVALID_CHARACTERS.toCharArray()) {
+      assertFalse("invalid character accepted: " + c, GitWebConfig.isValidPathSeparator(c));
+    }
+  }
+}