From 2916cfd79848ef555226b5d2a5179f540ffc428d Mon Sep 17 00:00:00 2001
From: James Moger <james.moger@gitblit.com>
Date: Mon, 08 Sep 2014 14:37:46 -0400
Subject: [PATCH] Improve bad request handling in branch graph, zip, & syndication servlets

---
 src/main/java/com/gitblit/utils/SyndicationUtils.java          |    6 +++++-
 src/main/java/com/gitblit/servlet/SyndicationServlet.java      |    6 +++---
 src/main/java/com/gitblit/servlet/BranchGraphServlet.java      |   33 ++++++++++++++++++++++++++++++++-
 src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java |    4 ++++
 src/main/java/com/gitblit/servlet/DownloadZipFilter.java       |   11 +++++++----
 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 0e6d323..7f69119 100644
--- a/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java
+++ b/src/main/java/com/gitblit/servlet/AccessRestrictionFilter.java
@@ -141,6 +141,10 @@
 
 		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 0abe347..fa2152c 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.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.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;
@@ -75,6 +78,8 @@
 	private static final int ROW_HEIGHT = 24;
 
 	private static final int RIGHT_PAD = 2;
+
+	private final Logger log = LoggerFactory.getLogger(getClass());
 
 	private final Stroke[] strokeCache;
 
@@ -117,6 +122,9 @@
 	@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 @@
 			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 @@
 		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 0c7b3e5..42257a2 100644
--- a/src/main/java/com/gitblit/servlet/DownloadZipFilter.java
+++ b/src/main/java/com/gitblit/servlet/DownloadZipFilter.java
@@ -38,11 +38,14 @@
 	@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 631df78..e3c2596 100644
--- a/src/main/java/com/gitblit/servlet/SyndicationServlet.java
+++ b/src/main/java/com/gitblit/servlet/SyndicationServlet.java
@@ -148,7 +148,7 @@
 
 		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 @@
 		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 @@
 			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 93e9321..7afd038 100644
--- a/src/main/java/com/gitblit/utils/SyndicationUtils.java
+++ b/src/main/java/com/gitblit/utils/SyndicationUtils.java
@@ -71,7 +71,11 @@
 		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");

--
Gitblit v1.9.1