raw: Fix raw links to branches with a slash in their name

When a branch has a slash in the name, the raw servlet was not able
to find the path under that branch. This is due to the replacement of
the forward slash character for URLs. It was not taken into account
when comparing the branch name later.

This fixes #1290 and its duplicates #1234 and #813.

diff --git a/src/main/java/com/gitblit/servlet/RawServlet.java b/src/main/java/com/gitblit/servlet/RawServlet.java
index b0cba7879cda201bd6ea5eb5e7ae66ddfae97b4d..9161eb6a9c08416deb122a1bb90dcda223eb198a 100644 (file)
--- a/src/main/java/com/gitblit/servlet/RawServlet.java
+++ b/src/main/java/com/gitblit/servlet/RawServlet.java
@@ -69,6 +69,9 @@ import com.google.inject.Singleton;
 @Singleton
 public class RawServlet extends HttpServlet {
 
+       // Forward slash character
+       static final char FSC = '!';
+
        private static final long serialVersionUID = 1L;
 
        private transient Logger logger = LoggerFactory.getLogger(RawServlet.class);
@@ -106,10 +109,9 @@ public class RawServlet extends HttpServlet {
                        repository = repository.substring(1);
                }
 
-               char fsc = '!';
-               char c = GitblitContext.getManager(IRuntimeManager.class).getSettings().getChar(Keys.web.forwardSlashCharacter, '/');
-               if (c != '/') {
-                       fsc = c;
+               char fsc = GitblitContext.getManager(IRuntimeManager.class).getSettings().getChar(Keys.web.forwardSlashCharacter, '/');
+               if (fsc == '/') {
+                       fsc = FSC;
                }
                if (branch != null) {
                        branch = Repository.shortenRefName(branch).replace('/', fsc);
@@ -164,7 +166,11 @@ public class RawServlet extends HttpServlet {
        String getPath(String repository, String branch, String pathInfo)
        {
                if (pathInfo == null || pathInfo.isEmpty() || pathInfo.equals("/")) return "";
-               String base = repository + "/" + branch;
+
+               // Make the branch look like in the URL, or else it won't match later in the `indexOf`.
+               char c = runtimeManager.getSettings().getChar(Keys.web.forwardSlashCharacter, '/');
+               char fsc = (c == '/') ? FSC : c;
+               String base = repository + "/" + Repository.shortenRefName(branch).replace('/', fsc);
 
                // 'repository/' or 'repository/branch' or 'repository/branch/'
                if (pathInfo.equals(base)) {
@@ -182,7 +188,6 @@ public class RawServlet extends HttpServlet {
                // 'leadin/repository/branch/path'
                String path = pathInfo.substring(pathStart);
 
-               char c = runtimeManager.getSettings().getChar(Keys.web.forwardSlashCharacter, '/');
                path =  path.replace('!', '/').replace(c, '/');
 
                // 'repository/branch/path/'

diff --git a/src/test/java/com/gitblit/servlet/RawServletTest.java b/src/test/java/com/gitblit/servlet/RawServletTest.java
index dac9896eadfdede92e0f3bd27d2de7824dc4cbd0..505871787603179f48b4bd0efcf0b410fb8ba465 100644 (file)
--- a/src/test/java/com/gitblit/servlet/RawServletTest.java
+++ b/src/test/java/com/gitblit/servlet/RawServletTest.java
@@ -17,7 +17,7 @@ import static org.mockito.Mockito.mock;
 
 public class RawServletTest
 {
-    private static final String FSC = "!";
+    private static final char FSC = RawServlet.FSC;
 
     private static MockRuntimeManager mockRuntimeManager = new MockRuntimeManager();
     private static IStoredSettings settings;
@@ -170,7 +170,7 @@ public class RawServletTest
         String link = RawServlet.asLink(baseUrl, repository, branch, path);
 
         assertNotNull(link);
-        assertEquals(baseUrl + Constants.RAW_PATH + repository + "/" + branch.replaceAll("/", FSC) + "/", link);
+        assertEquals(baseUrl + Constants.RAW_PATH + repository + "/" + branch.replace('/', FSC) + "/", link);
     }
 
     @Test
@@ -185,7 +185,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository + "/"
-                + branch.replaceAll("/", FSC) + "/", link);
+                + branch.replace('/', FSC) + "/", link);
     }
 
     @Test
@@ -200,7 +200,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch.replaceAll("/", FSC) + "/", link);
+                + branch.replace('/', FSC) + "/", link);
     }
 
     @Test
@@ -215,7 +215,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch.replaceAll("/", FSC) + "/", link);
+                + branch.replace('/', FSC) + "/", link);
     }
 
 
@@ -245,7 +245,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository + "/"
-                + branch + "/" + path.replaceAll("/", FSC), link);
+                + branch + "/" + path.replace('/', FSC), link);
     }
 
     @Test
@@ -274,7 +274,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch + "/" + path.replaceAll("/", FSC), link);
+                + branch + "/" + path.replace('/', FSC), link);
     }
 
 
@@ -290,7 +290,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository + "/"
-                + branch.replaceAll("/", FSC) + "/" + path, link);
+                + branch.replace('/', FSC) + "/" + path, link);
     }
 
     @Test
@@ -305,7 +305,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository + "/"
-                + branch.replaceAll("/", FSC) + "/" + path.replaceAll("/", FSC), link);
+                + branch.replace('/', FSC) + "/" + path.replace('/', FSC), link);
     }
 
     @Test
@@ -320,7 +320,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch.replaceAll("/", FSC) + "/" + path, link);
+                + branch.replace('/', FSC) + "/" + path, link);
     }
 
     @Test
@@ -335,7 +335,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl.substring(0, baseUrl.length()-1) + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch.replaceAll("/", FSC) + "/" + path.replaceAll("/", FSC), link);
+                + branch.replace('/', FSC) + "/" + path.replace('/', FSC), link);
     }
 
     @Test
@@ -393,7 +393,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository + "/"
-                 + branch.replaceAll("/", FSC) + "/", link);
+                 + branch.replace('/', FSC) + "/", link);
     }
 
     @Test
@@ -423,7 +423,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository.substring(1) + "/"
-                + branch.replaceAll("/", FSC) + "/" + path.replaceAll("/", FSC), link);
+                + branch.replace('/', FSC) + "/" + path.replace('/', FSC), link);
     }
 
     @Test
@@ -541,7 +541,7 @@ public class RawServletTest
 
         assertNotNull(link);
         assertEquals(baseUrl + Constants.RAW_PATH + repository + "/"
-                + branch + "/" + path.replaceAll("/", FSC), link);
+                + branch + "/" + path.replace('/', FSC), link);
     }
 
     @Test
@@ -1289,7 +1289,7 @@ public class RawServletTest
     @Test
     public void getPath_RepoBranchWithFsc_explicitFscSameAsDefault()
     {
-        settings.overrideSetting(Keys.web.forwardSlashCharacter, FSC);
+        settings.overrideSetting(Keys.web.forwardSlashCharacter, String.valueOf(FSC));
 
         String path = rawServlet.getPath("git.git", "some/feature", "git.git/some" + FSC + "feature");
 
@@ -1309,7 +1309,7 @@ public class RawServletTest
     @Test
     public void getPath_LeadindRepoBranchWithFscFolderFile_explicitFscSameAsDefault()
     {
-        settings.overrideSetting(Keys.web.forwardSlashCharacter, FSC);
+        settings.overrideSetting(Keys.web.forwardSlashCharacter, String.valueOf(FSC));
 
         String path = rawServlet.getPath("git.git", "some/feature", "IBM/git.git/some" + FSC + "feature/some" + FSC + "folder" + FSC + "file.dot");