Improve bad request handling in branch graph, zip, & syndication servlets

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");