From 44c005e7c1efb0564d07394ca2e98308b9b48581 Mon Sep 17 00:00:00 2001 From: James Moger Date: Fri, 27 Jun 2014 23:06:13 -0400 Subject: [PATCH] Use consistent keys for repository cache lookups --- .../gitblit/manager/RepositoryManager.java | 96 ++++++++++++++----- .../java/com/gitblit/servlet/RawServlet.java | 3 + .../gitblit/wicket/pages/RepositoryPage.java | 16 ++-- 3 files changed, 86 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/gitblit/manager/RepositoryManager.java b/src/main/java/com/gitblit/manager/RepositoryManager.java index 6601336e..0160363e 100644 --- a/src/main/java/com/gitblit/manager/RepositoryManager.java +++ b/src/main/java/com/gitblit/manager/RepositoryManager.java @@ -422,11 +422,12 @@ public class RepositoryManager implements IRepositoryManager { @Override public void addToCachedRepositoryList(RepositoryModel model) { if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { - repositoryListCache.put(model.name.toLowerCase(), model); + String key = getRepositoryKey(model.name); + repositoryListCache.put(key, model); // update the fork origin repository with this repository clone if (!StringUtils.isEmpty(model.originRepository)) { - String originKey = model.originRepository.toLowerCase(); + String originKey = getRepositoryKey(model.originRepository); if (repositoryListCache.containsKey(originKey)) { RepositoryModel origin = repositoryListCache.get(originKey); origin.addFork(model.name); @@ -445,7 +446,8 @@ public class RepositoryManager implements IRepositoryManager { if (StringUtils.isEmpty(name)) { return null; } - return repositoryListCache.remove(name.toLowerCase()); + String key = getRepositoryKey(name); + return repositoryListCache.remove(key); } /** @@ -554,7 +556,7 @@ public class RepositoryManager implements IRepositoryManager { // rebuild fork networks for (RepositoryModel model : repositoryListCache.values()) { if (!StringUtils.isEmpty(model.originRepository)) { - String originKey = model.originRepository.toLowerCase(); + String originKey = getRepositoryKey(model.originRepository); if (repositoryListCache.containsKey(originKey)) { RepositoryModel origin = repositoryListCache.get(originKey); origin.addFork(model.name); @@ -590,15 +592,13 @@ public class RepositoryManager implements IRepositoryManager { /** * Returns the JGit repository for the specified name. * - * @param repositoryName + * @param name * @param logError * @return repository or null */ @Override - public Repository getRepository(String repositoryName, boolean logError) { - // Decode url-encoded repository name (issue-278) - // http://stackoverflow.com/questions/17183110 - repositoryName = repositoryName.replace("%7E", "~").replace("%7e", "~"); + public Repository getRepository(String name, boolean logError) { + String repositoryName = fixRepositoryName(name); if (isCollectingGarbage(repositoryName)) { logger.warn(MessageFormat.format("Rejecting request for {0}, busy collecting garbage!", repositoryName)); @@ -680,16 +680,14 @@ public class RepositoryManager implements IRepositoryManager { * Returns the repository model for the specified repository. This method * does not consider user access permissions. * - * @param repositoryName + * @param name * @return repository model or null */ @Override - public RepositoryModel getRepositoryModel(String repositoryName) { - // Decode url-encoded repository name (issue-278) - // http://stackoverflow.com/questions/17183110 - repositoryName = repositoryName.replace("%7E", "~").replace("%7e", "~"); + public RepositoryModel getRepositoryModel(String name) { + String repositoryName = fixRepositoryName(name); - String repositoryKey = repositoryName.toLowerCase(); + String repositoryKey = getRepositoryKey(repositoryName); if (!repositoryListCache.containsKey(repositoryKey)) { RepositoryModel model = loadRepositoryModel(repositoryName); if (model == null) { @@ -757,6 +755,52 @@ public class RepositoryManager implements IRepositoryManager { return count; } + /** + * Replaces illegal character patterns in a repository name. + * + * @param repositoryName + * @return a corrected name + */ + private String fixRepositoryName(String repositoryName) { + if (StringUtils.isEmpty(repositoryName)) { + return repositoryName; + } + + // Decode url-encoded repository name (issue-278) + // http://stackoverflow.com/questions/17183110 + String name = repositoryName.replace("%7E", "~").replace("%7e", "~"); + name = name.replace("%2F", "/").replace("%2f", "/"); + + if (name.charAt(name.length() - 1) == '/') { + name = name.substring(0, name.length() - 1); + } + + // strip duplicate-slashes from requests for repositoryName (ticket-117, issue-454) + // specify first char as slash so we strip leading slashes + char lastChar = '/'; + StringBuilder sb = new StringBuilder(); + for (char c : name.toCharArray()) { + if (c == '/' && lastChar == c) { + continue; + } + sb.append(c); + lastChar = c; + } + + return sb.toString(); + } + + /** + * Returns the cache key for the repository name. + * + * @param repositoryName + * @return the cache key for the repository + */ + private String getRepositoryKey(String repositoryName) { + String name = fixRepositoryName(repositoryName); + return StringUtils.stripDotGit(name).toLowerCase(); + } + /** * Workaround JGit. I need to access the raw config object directly in order * to see if the config is dirty so that I can reload a repository model. @@ -932,7 +976,8 @@ public class RepositoryManager implements IRepositoryManager { if (!caseSensitiveCheck && settings.getBoolean(Keys.git.cacheRepositoryList, true)) { // if we are caching use the cache to determine availability // otherwise we end up adding a phantom repository to the cache - return repositoryListCache.containsKey(repositoryName.toLowerCase()); + String key = getRepositoryKey(repositoryName); + return repositoryListCache.containsKey(key); } Repository r = getRepository(repositoryName, false); if (r == null) { @@ -970,7 +1015,7 @@ public class RepositoryManager implements IRepositoryManager { } String userProject = ModelUtils.getPersonalPath(username); if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { - String originKey = origin.toLowerCase(); + String originKey = getRepositoryKey(origin); String userPath = userProject + "/"; // collect all origin nodes in fork network @@ -987,7 +1032,7 @@ public class RepositoryManager implements IRepositoryManager { } if (originModel.originRepository != null) { - String ooKey = originModel.originRepository.toLowerCase(); + String ooKey = getRepositoryKey(originModel.originRepository); roots.add(ooKey); originModel = repositoryListCache.get(ooKey); } else { @@ -1000,7 +1045,8 @@ public class RepositoryManager implements IRepositoryManager { if (repository.startsWith(userPath)) { RepositoryModel model = repositoryListCache.get(repository); if (!StringUtils.isEmpty(model.originRepository)) { - if (roots.contains(model.originRepository.toLowerCase())) { + String ooKey = getRepositoryKey(model.originRepository); + if (roots.contains(ooKey)) { // user has a fork in this graph return model.name; } @@ -1038,9 +1084,11 @@ public class RepositoryManager implements IRepositoryManager { public ForkModel getForkNetwork(String repository) { if (settings.getBoolean(Keys.git.cacheRepositoryList, true)) { // find the root, cached - RepositoryModel model = repositoryListCache.get(repository.toLowerCase()); + String key = getRepositoryKey(repository); + RepositoryModel model = repositoryListCache.get(key); while (model.originRepository != null) { - model = repositoryListCache.get(model.originRepository.toLowerCase()); + String originKey = getRepositoryKey(model.originRepository); + model = repositoryListCache.get(originKey); } ForkModel root = getForkModelFromCache(model.name); return root; @@ -1056,7 +1104,8 @@ public class RepositoryManager implements IRepositoryManager { } private ForkModel getForkModelFromCache(String repository) { - RepositoryModel model = repositoryListCache.get(repository.toLowerCase()); + String key = getRepositoryKey(repository); + RepositoryModel model = repositoryListCache.get(key); if (model == null) { return null; } @@ -1371,7 +1420,8 @@ public class RepositoryManager implements IRepositoryManager { // update this repository's origin's fork list if (!StringUtils.isEmpty(repository.originRepository)) { - RepositoryModel origin = repositoryListCache.get(repository.originRepository.toLowerCase()); + String originKey = getRepositoryKey(repository.originRepository); + RepositoryModel origin = repositoryListCache.get(originKey); if (origin != null && !ArrayUtils.isEmpty(origin.forks)) { origin.forks.remove(repositoryName); origin.forks.add(repository.name); diff --git a/src/main/java/com/gitblit/servlet/RawServlet.java b/src/main/java/com/gitblit/servlet/RawServlet.java index 15e036ea..0def062a 100644 --- a/src/main/java/com/gitblit/servlet/RawServlet.java +++ b/src/main/java/com/gitblit/servlet/RawServlet.java @@ -175,6 +175,9 @@ public class RawServlet extends DaggerServlet { repository = path.substring(0, slash); } offset += slash; + if (offset == 0) { + offset++; + } r = repositoryManager.getRepository(repository, false); if (repository.equals(path)) { // either only repository in url or no repository found diff --git a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java index fcf659af..253c4fe4 100644 --- a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java +++ b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java @@ -166,10 +166,10 @@ public abstract class RepositoryPage extends RootPage { add(navigationPanel); add(new ExternalLink("syndication", SyndicationServlet.asLink(getRequest() - .getRelativePathPrefixToContextRoot(), repositoryName, null, 0))); + .getRelativePathPrefixToContextRoot(), getRepositoryName(), null, 0))); // add floating search form - SearchForm searchForm = new SearchForm("searchForm", repositoryName); + SearchForm searchForm = new SearchForm("searchForm", getRepositoryName()); add(searchForm); searchForm.setTranslatedAttributes(); @@ -193,7 +193,7 @@ public abstract class RepositoryPage extends RootPage { private List registerNavLinks() { PageParameters params = null; if (!StringUtils.isEmpty(repositoryName)) { - params = WicketUtils.newRepositoryParameter(repositoryName); + params = WicketUtils.newRepositoryParameter(getRepositoryName()); } List navLinks = new ArrayList(); @@ -216,7 +216,7 @@ public abstract class RepositoryPage extends RootPage { navLinks.add(new PageNavLink("gb.commits", LogPage.class, params)); navLinks.add(new PageNavLink("gb.tree", TreePage.class, params)); if (app().tickets().isReady() && (app().tickets().isAcceptingNewTickets(model) || app().tickets().hasTickets(model))) { - PageParameters tParams = WicketUtils.newOpenTicketsParameter(repositoryName); + PageParameters tParams = WicketUtils.newOpenTicketsParameter(getRepositoryName()); navLinks.add(new PageNavLink("gb.tickets", TicketsPage.class, tParams)); } navLinks.add(new PageNavLink("gb.docs", DocsPage.class, params, true)); @@ -229,7 +229,7 @@ public abstract class RepositoryPage extends RootPage { // per-repository extra navlinks if (JGitUtils.getPagesBranch(r) != null) { ExternalNavLink pagesLink = new ExternalNavLink("gb.pages", PagesServlet.asLink( - getRequest().getRelativePathPrefixToContextRoot(), repositoryName, null), true); + getRequest().getRelativePathPrefixToContextRoot(), getRepositoryName(), null), true); navLinks.add(pagesLink); } @@ -422,6 +422,10 @@ public abstract class RepositoryPage extends RootPage { return m; } + protected String getRepositoryName() { + return getRepositoryModel().name; + } + protected RevCommit getCommit() { RevCommit commit = JGitUtils.getCommit(r, objectId); if (commit == null) { @@ -630,7 +634,7 @@ public abstract class RepositoryPage extends RootPage { r = null; } // setup page header and footer - setupPage(repositoryName, "/ " + getPageName()); + setupPage(getRepositoryName(), "/ " + getPageName()); super.onBeforeRender(); } -- 2.39.5