From 6876537c856dda897f081430bb11b394f8b291bb Mon Sep 17 00:00:00 2001 From: Florian Zschocke Date: Mon, 9 Nov 2020 23:08:22 +0100 Subject: [PATCH] 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. --- .../java/com/gitblit/servlet/RawServlet.java | 17 ++++++---- .../com/gitblit/servlet/RawServletTest.java | 32 +++++++++---------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/gitblit/servlet/RawServlet.java b/src/main/java/com/gitblit/servlet/RawServlet.java index b0cba787..9161eb6a 100644 --- 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 dac9896e..50587178 100644 --- 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"); -- 2.39.5