From 2916cfd79848ef555226b5d2a5179f540ffc428d Mon Sep 17 00:00:00 2001 From: James Moger Date: Mon, 8 Sep 2014 14:37:46 -0400 Subject: [PATCH] Improve bad request handling in branch graph, zip, & syndication servlets --- .../servlet/AccessRestrictionFilter.java | 4 +++ .../gitblit/servlet/BranchGraphServlet.java | 33 ++++++++++++++++++- .../gitblit/servlet/DownloadZipFilter.java | 11 ++++--- .../gitblit/servlet/SyndicationServlet.java | 6 ++-- .../com/gitblit/utils/SyndicationUtils.java | 6 +++- 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java b/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java index 0e6d323d..7f691196 100644 --- a/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java +++ b/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java @@ -141,6 +141,10 @@ public abstract class AccessRestrictionFilter extends AuthenticationFilter { String fullUrl = getFullUrl(httpRequest); String repository = extractRepositoryName(fullUrl); + if (StringUtils.isEmpty(repository)) { + httpResponse.setStatus(HttpServletResponse.SC_BAD_REQUEST); + return; + } if (repositoryManager.isCollectingGarbage(repository)) { logger.info(MessageFormat.format("ARF: Rejecting request for {0}, busy collecting garbage!", repository)); diff --git a/src/main/java/com/gitblit/servlet/BranchGraphServlet.java b/src/main/java/com/gitblit/servlet/BranchGraphServlet.java index 0abe347f..fa2152c6 100644 --- a/src/main/java/com/gitblit/servlet/BranchGraphServlet.java +++ b/src/main/java/com/gitblit/servlet/BranchGraphServlet.java @@ -40,6 +40,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revplot.AbstractPlotRenderer; @@ -48,6 +49,8 @@ import org.eclipse.jgit.revplot.PlotCommitList; import org.eclipse.jgit.revplot.PlotLane; import org.eclipse.jgit.revplot.PlotWalk; import org.eclipse.jgit.revwalk.RevCommit; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.gitblit.Constants; import com.gitblit.IStoredSettings; @@ -76,6 +79,8 @@ public class BranchGraphServlet extends DaggerServlet { private static final int RIGHT_PAD = 2; + private final Logger log = LoggerFactory.getLogger(getClass()); + private final Stroke[] strokeCache; private IStoredSettings settings; @@ -117,6 +122,9 @@ public class BranchGraphServlet extends DaggerServlet { @Override protected long getLastModified(HttpServletRequest req) { String repository = req.getParameter("r"); + if (StringUtils.isEmpty(repository)) { + return 0; + } String objectId = req.getParameter("h"); Repository r = null; try { @@ -124,8 +132,15 @@ public class BranchGraphServlet extends DaggerServlet { if (StringUtils.isEmpty(objectId)) { objectId = JGitUtils.getHEADRef(r); } + ObjectId id = r.resolve(objectId); + if (id == null) { + return 0; + } RevCommit commit = JGitUtils.getCommit(r, objectId); return JGitUtils.getCommitDate(commit).getTime(); + } catch (Exception e) { + log.error("Failed to determine last modified", e); + return 0; } finally { if (r != null) { r.close(); @@ -141,17 +156,33 @@ public class BranchGraphServlet extends DaggerServlet { PlotWalk rw = null; try { String repository = request.getParameter("r"); + if (StringUtils.isEmpty(repository)) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + response.getWriter().append("Bad request"); + return; + } String objectId = request.getParameter("h"); String length = request.getParameter("l"); r = repositoryManager.getRepository(repository); + if (r == null) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + response.getWriter().append("Bad request"); + return; + } rw = new PlotWalk(r); if (StringUtils.isEmpty(objectId)) { objectId = JGitUtils.getHEADRef(r); } - rw.markStart(rw.lookupCommit(r.resolve(objectId))); + ObjectId id = r.resolve(objectId); + if (id == null) { + response.setStatus(HttpServletResponse.SC_BAD_REQUEST); + response.getWriter().append("Bad request"); + return; + } + rw.markStart(rw.lookupCommit(id)); // default to the items-per-page setting, unless specified int maxCommits = settings.getInteger(Keys.web.itemsPerPage, 50); diff --git a/src/main/java/com/gitblit/servlet/DownloadZipFilter.java b/src/main/java/com/gitblit/servlet/DownloadZipFilter.java index 0c7b3e56..42257a23 100644 --- a/src/main/java/com/gitblit/servlet/DownloadZipFilter.java +++ b/src/main/java/com/gitblit/servlet/DownloadZipFilter.java @@ -38,11 +38,14 @@ public class DownloadZipFilter extends AccessRestrictionFilter { @Override protected String extractRepositoryName(String url) { int a = url.indexOf("r="); - String repository = url.substring(a + 2); - if (repository.indexOf('&') > -1) { - repository = repository.substring(0, repository.indexOf('&')); + if (a > -1) { + String repository = url.substring(a + 2); + if (repository.indexOf('&') > -1) { + repository = repository.substring(0, repository.indexOf('&')); + } + return repository; } - return repository; + return null; } /** diff --git a/src/main/java/com/gitblit/servlet/SyndicationServlet.java b/src/main/java/com/gitblit/servlet/SyndicationServlet.java index 631df781..e3c25967 100644 --- a/src/main/java/com/gitblit/servlet/SyndicationServlet.java +++ b/src/main/java/com/gitblit/servlet/SyndicationServlet.java @@ -148,7 +148,7 @@ public class SyndicationServlet extends DaggerServlet { String servletUrl = request.getContextPath() + request.getServletPath(); String url = request.getRequestURI().substring(servletUrl.length()); - if (url.charAt(0) == '/' && url.length() > 1) { + if (url.length() > 1 && url.charAt(0) == '/') { url = url.substring(1); } String repositoryName = url; @@ -193,7 +193,7 @@ public class SyndicationServlet extends DaggerServlet { response.setContentType("application/rss+xml; charset=UTF-8"); boolean isProjectFeed = false; - String feedName = null; + String feedName = "Gitblit"; String feedTitle = null; String feedDescription = null; @@ -237,7 +237,7 @@ public class SyndicationServlet extends DaggerServlet { RepositoryModel model = repositoryManager.getRepositoryModel(name); if (repository == null) { - if (model.isCollectingGarbage) { + if (model != null && model.isCollectingGarbage) { logger.warn(MessageFormat.format("Temporarily excluding {0} from feed, busy collecting garbage", name)); } continue; diff --git a/src/main/java/com/gitblit/utils/SyndicationUtils.java b/src/main/java/com/gitblit/utils/SyndicationUtils.java index 93e9321a..7afd0383 100644 --- a/src/main/java/com/gitblit/utils/SyndicationUtils.java +++ b/src/main/java/com/gitblit/utils/SyndicationUtils.java @@ -71,7 +71,11 @@ public class SyndicationUtils { feed.setEncoding("UTF-8"); feed.setTitle(title); feed.setLink(feedLink); - feed.setDescription(description); + if (StringUtils.isEmpty(description)) { + feed.setDescription(title); + } else { + feed.setDescription(description); + } SyndImageImpl image = new SyndImageImpl(); image.setTitle(Constants.NAME); image.setUrl(hostUrl + "/gitblt_25.png"); -- 2.39.5