From 560f2e3ab0f78429fe22dc23ae25de6a7500cf02 Mon Sep 17 00:00:00 2001
From: Tom <tw201207@gmail.com>
Date: Wed, 26 Oct 2016 22:49:56 +0200
Subject: Issue #1011: do not serialize JGit commit objects

JGit commit objects are a recursive data structure; they have links to
their parent commits. Serializing a JGit commit will try to recursively
serialize all reachable ancestors as faras they have been loaded. If
that ancestor chain is too long, a StackOverflowError is thrown during
Wicket's page serialization if a page has a reference to sucha JGit
commit.

Fixed by making sure that pages o not contain references to JGit
commits. Use the (existing) wrapper object RepositoryCommit instead.

* RepositoryCommit has a transient reference to the JGit commit and
  reads the commit from the repository upon de-serialization.
* RefModel is a similar case (JGit tags/branches may also have links
  to the commits they point to). Solved a bit differently by making it
  a pure data object by transferring the interesting data from the JGit
  object in the constructor.
* Change DataViews instantiated with RevCommit to use RepositoryCommit
  instead.
* Change inner anonymous DataViews to ensure they do not have a
  synthesized field referencing the "allRefs" map. Such a synthesized
  field would also get serialized, and then serialize JGit commits
  again.

Finally, remove non-transient logger instances in Wicket classes. Those
might lead to NotSerializableException.

These StackOverflowErrors have been reported in several places since
2014:

* https://groups.google.com/forum/#!topic/gitblit/GH1d8WSlR6Q
* https://bugs.chromium.org/p/gerrit/issues/detail?id=3316
* https://groups.google.com/d/msg/repo-discuss/Kcl0JIGNiGk/0DjH4mO8hA8J
* https://groups.google.com/d/msg/repo-discuss/0_P6A3fjTec/2kcpVPIUAQAJ
* https://github.com/gitblit/gitblit/issues/1011
* https://github.com/tomaswolf/gerrit-gitblit-plugin/issues/21
---
 src/main/java/com/gitblit/models/RefModel.java     | 93 ++++++++++++++++++----
 .../java/com/gitblit/models/RepositoryCommit.java  | 49 +++++++++---
 .../java/com/gitblit/wicket/SessionlessForm.java   | 14 +++-
 .../java/com/gitblit/wicket/pages/BlamePage.java   |  2 +-
 .../com/gitblit/wicket/pages/EditFilePage.java     |  2 +-
 .../java/com/gitblit/wicket/pages/MetricsPage.java |  2 +-
 .../java/com/gitblit/wicket/pages/RawPage.java     | 31 +++++---
 .../com/gitblit/wicket/pages/RepositoryPage.java   | 10 +--
 .../java/com/gitblit/wicket/pages/SummaryPage.java |  2 +-
 .../java/com/gitblit/wicket/pages/TicketPage.java  | 21 +++--
 .../com/gitblit/wicket/panels/HistoryPanel.java    | 39 +++++----
 .../java/com/gitblit/wicket/panels/LogPanel.java   | 33 +++++---
 .../com/gitblit/wicket/panels/SearchPanel.java     | 29 ++++---
 13 files changed, 230 insertions(+), 97 deletions(-)

diff --git a/src/main/java/com/gitblit/models/RefModel.java b/src/main/java/com/gitblit/models/RefModel.java
index d20c2dc6..86d98474 100644
--- a/src/main/java/com/gitblit/models/RefModel.java
+++ b/src/main/java/com/gitblit/models/RefModel.java
@@ -36,18 +36,42 @@ import com.gitblit.utils.JGitUtils;
  */
 public class RefModel implements Serializable, Comparable<RefModel> {
 
-	private static final long serialVersionUID = 1L;
+	private static final long serialVersionUID = 876822269940583606L;
+
 	public final String displayName;
-	public final RevObject referencedObject;
-	public transient Ref reference;
+
+	private final Date date;
+	private final String name;
+	private final int type;
+	private final String id;
+	private final String referencedId;
+	private final boolean annotated;
+	private final PersonIdent person;
+	private final String shortMessage;
+	private final String fullMessage;
+
+	private transient ObjectId objectId;
+	private transient ObjectId referencedObjectId;
+
+	public transient Ref reference; // Used in too many places.
 
 	public RefModel(String displayName, Ref ref, RevObject refObject) {
-		this.displayName = displayName;
 		this.reference = ref;
-		this.referencedObject = refObject;
+		this.displayName = displayName;
+		this.date = internalGetDate(refObject);
+		this.name = (ref != null) ? ref.getName() : displayName;
+		this.type = internalGetReferencedObjectType(refObject);
+		this.objectId = internalGetObjectId(reference);
+		this.id = this.objectId.getName();
+		this.referencedObjectId = internalGetReferencedObjectId(refObject);
+		this.referencedId = this.referencedObjectId.getName();
+		this.annotated = internalIsAnnotatedTag(ref, refObject);
+		this.person = internalGetAuthorIdent(refObject);
+		this.shortMessage = internalGetShortMessage(refObject);
+		this.fullMessage = internalGetFullMessage(refObject);
 	}
 
-	public Date getDate() {
+	private Date internalGetDate(RevObject referencedObject) {
 		Date date = new Date(0);
 		if (referencedObject != null) {
 			if (referencedObject instanceof RevTag) {
@@ -64,14 +88,15 @@ public class RefModel implements Serializable, Comparable<RefModel> {
 		return date;
 	}
 
+	public Date getDate() {
+		return date;
+	}
+
 	public String getName() {
-		if (reference == null) {
-			return displayName;
-		}
-		return reference.getName();
+		return name;
 	}
 
-	public int getReferencedObjectType() {
+	private int internalGetReferencedObjectType(RevObject referencedObject) {
 		int type = referencedObject.getType();
 		if (referencedObject instanceof RevTag) {
 			type = ((RevTag) referencedObject).getObject().getType();
@@ -79,14 +104,25 @@ public class RefModel implements Serializable, Comparable<RefModel> {
 		return type;
 	}
 
-	public ObjectId getReferencedObjectId() {
+	public int getReferencedObjectType() {
+		return type;
+	}
+
+	private ObjectId internalGetReferencedObjectId(RevObject referencedObject) {
 		if (referencedObject instanceof RevTag) {
 			return ((RevTag) referencedObject).getObject().getId();
 		}
 		return referencedObject.getId();
 	}
 
-	public String getShortMessage() {
+	public ObjectId getReferencedObjectId() {
+		if (referencedObjectId == null) {
+			referencedObjectId = ObjectId.fromString(referencedId);
+		}
+		return referencedObjectId;
+	}
+
+	private String internalGetShortMessage(RevObject referencedObject) {
 		String message = "";
 		if (referencedObject instanceof RevTag) {
 			message = ((RevTag) referencedObject).getShortMessage();
@@ -96,7 +132,11 @@ public class RefModel implements Serializable, Comparable<RefModel> {
 		return message;
 	}
 
-	public String getFullMessage() {
+	public String getShortMessage() {
+		return shortMessage;
+	}
+
+	private String internalGetFullMessage(RevObject referencedObject) {
 		String message = "";
 		if (referencedObject instanceof RevTag) {
 			message = ((RevTag) referencedObject).getFullMessage();
@@ -106,7 +146,11 @@ public class RefModel implements Serializable, Comparable<RefModel> {
 		return message;
 	}
 
-	public PersonIdent getAuthorIdent() {
+	public String getFullMessage() {
+		return fullMessage;
+	}
+
+	private PersonIdent internalGetAuthorIdent(RevObject referencedObject) {
 		if (referencedObject instanceof RevTag) {
 			return ((RevTag) referencedObject).getTaggerIdent();
 		} else if (referencedObject instanceof RevCommit) {
@@ -115,17 +159,32 @@ public class RefModel implements Serializable, Comparable<RefModel> {
 		return null;
 	}
 
-	public ObjectId getObjectId() {
+	public PersonIdent getAuthorIdent() {
+		return person;
+	}
+
+	private ObjectId internalGetObjectId(Ref reference) {
 		return reference.getObjectId();
 	}
 
-	public boolean isAnnotatedTag() {
+	public ObjectId getObjectId() {
+		if (objectId == null) {
+			objectId = ObjectId.fromString(id);
+		}
+		return objectId;
+	}
+
+	private boolean internalIsAnnotatedTag(Ref reference, RevObject referencedObject) {
 		if (referencedObject instanceof RevTag) {
 			return !getReferencedObjectId().equals(getObjectId());
 		}
 		return reference.getPeeledObjectId() != null;
 	}
 
+	public boolean isAnnotatedTag() {
+		return annotated;
+	}
+
 	@Override
 	public int hashCode() {
 		return getReferencedObjectId().hashCode() + getName().hashCode();
diff --git a/src/main/java/com/gitblit/models/RepositoryCommit.java b/src/main/java/com/gitblit/models/RepositoryCommit.java
index 765b4898..43f314a3 100644
--- a/src/main/java/com/gitblit/models/RepositoryCommit.java
+++ b/src/main/java/com/gitblit/models/RepositoryCommit.java
@@ -15,6 +15,8 @@
  */
 package com.gitblit.models;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
 import java.io.Serializable;
 import java.text.MessageFormat;
 import java.util.Date;
@@ -22,30 +24,36 @@ import java.util.List;
 
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+import com.gitblit.wicket.GitBlitWebApp;
 
 /**
- * Model class to represent a RevCommit, it's source repository, and the branch.
- * This class is used by the activity page.
+ * Model class to represent a RevCommit, it's source repository, and the branch. This class is used by the activity page.
  *
  * @author James Moger
  */
 public class RepositoryCommit implements Serializable, Comparable<RepositoryCommit> {
 
-	private static final long serialVersionUID = 1L;
+	private static final long serialVersionUID = -2214911650485772022L;
 
-	public final String repository;
+	public String repository;
 
-	public final String branch;
+	public String branch;
 
-	private final RevCommit commit;
+	private final String commitId;
 
 	private List<RefModel> refs;
 
+	private transient RevCommit commit;
+
 	public RepositoryCommit(String repository, String branch, RevCommit commit) {
 		this.repository = repository;
 		this.branch = branch;
 		this.commit = commit;
+		this.commitId = commit.getName();
 	}
 
 	public void setRefs(List<RefModel> refs) {
@@ -80,7 +88,7 @@ public class RepositoryCommit implements Serializable, Comparable<RepositoryComm
 		return commit.getParentCount();
 	}
 
-	public RevCommit [] getParents() {
+	public RevCommit[] getParents() {
 		return commit.getParents();
 	}
 
@@ -92,10 +100,14 @@ public class RepositoryCommit implements Serializable, Comparable<RepositoryComm
 		return commit.getCommitterIdent();
 	}
 
+	public RevCommit getCommit() {
+		return commit;
+	}
+
 	@Override
 	public boolean equals(Object o) {
 		if (o instanceof RepositoryCommit) {
-			RepositoryCommit commit = (RepositoryCommit) o;
+			final RepositoryCommit commit = (RepositoryCommit) o;
 			return repository.equals(commit.repository) && getName().equals(commit.getName());
 		}
 		return false;
@@ -123,8 +135,23 @@ public class RepositoryCommit implements Serializable, Comparable<RepositoryComm
 
 	@Override
 	public String toString() {
-		return MessageFormat.format("{0} {1} {2,date,yyyy-MM-dd HH:mm} {3} {4}",
-				getShortName(), branch, getCommitterIdent().getWhen(), getAuthorIdent().getName(),
-				getShortMessage());
+		return MessageFormat.format("{0} {1} {2,date,yyyy-MM-dd HH:mm} {3} {4}", getShortName(), branch, getCommitterIdent().getWhen(),
+				getAuthorIdent().getName(), getShortMessage());
+	}
+
+	// Serialization: restore the JGit RevCommit on reading
+
+	private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
+		// Read in fields and any hidden stuff
+		input.defaultReadObject();
+		// Go find the commit again.
+		final Repository repo = GitBlitWebApp.get().repositories().getRepository(repository);
+		if (repo == null) {
+			throw new IOException("Cannot find repositoy " + repository);
+		}
+		try (RevWalk walk = new RevWalk(repo)) {
+			commit = walk.parseCommit(repo.resolve(commitId));
+		}
 	}
+
 }
\ No newline at end of file
diff --git a/src/main/java/com/gitblit/wicket/SessionlessForm.java b/src/main/java/com/gitblit/wicket/SessionlessForm.java
index 6f790717..c53f095e 100644
--- a/src/main/java/com/gitblit/wicket/SessionlessForm.java
+++ b/src/main/java/com/gitblit/wicket/SessionlessForm.java
@@ -58,7 +58,7 @@ public class SessionlessForm<T> extends StatelessForm<T> {
 
 	protected final PageParameters pageParameters;
 
-	private final Logger log = LoggerFactory.getLogger(SessionlessForm.class);
+	private transient Logger logger;
 
 	/**
 	 * Sessionless forms must have a bookmarkable page class.  A bookmarkable
@@ -118,7 +118,10 @@ public class SessionlessForm<T> extends StatelessForm<T> {
 				if (c != null) {
 					// this form has a field id which matches the
 					// parameter name, skip embedding a hidden value
-					log.warn(MessageFormat.format("Skipping page parameter \"{0}\" from sessionless form hidden fields because it collides with a form child wicket:id", key));
+					logger().warn(
+							MessageFormat
+									.format("Skipping page parameter \"{0}\" from sessionless form hidden fields because it collides with a form child wicket:id",
+											key));
 					continue;
 				}
 				String value = pageParameters.getString(key);
@@ -156,4 +159,11 @@ public class SessionlessForm<T> extends StatelessForm<T> {
 		String absoluteUrl = RequestUtils.toAbsolutePath(relativeUrl);
 		return absoluteUrl;
 	}
+
+	private Logger logger() {
+		if (logger == null) {
+			logger = LoggerFactory.getLogger(SessionlessForm.class);
+		}
+		return logger;
+	}
 }
diff --git a/src/main/java/com/gitblit/wicket/pages/BlamePage.java b/src/main/java/com/gitblit/wicket/pages/BlamePage.java
index 2fcca0ae..8465058f 100644
--- a/src/main/java/com/gitblit/wicket/pages/BlamePage.java
+++ b/src/main/java/com/gitblit/wicket/pages/BlamePage.java
@@ -108,7 +108,7 @@ public class BlamePage extends RepositoryPage {
 		if (pathModel == null) {
 			final String notFound = MessageFormat.format("Blame page failed to find {0} in {1} @ {2}",
 					blobPath, repositoryName, objectId);
-			logger.error(notFound);
+			logger().error(notFound);
 			add(new Label("annotation").setVisible(false));
 			add(new Label("missingBlob", missingBlob(blobPath, commit)).setEscapeModelStrings(false));
 			return;
diff --git a/src/main/java/com/gitblit/wicket/pages/EditFilePage.java b/src/main/java/com/gitblit/wicket/pages/EditFilePage.java
index dbf8a79e..5ec9b6ee 100644
--- a/src/main/java/com/gitblit/wicket/pages/EditFilePage.java
+++ b/src/main/java/com/gitblit/wicket/pages/EditFilePage.java
@@ -123,7 +123,7 @@ public class EditFilePage extends RepositoryPage {
 					try {			
 						ObjectId docAtLoad = getRepository().resolve(commitIdAtLoad.getObject());
 						
-						logger.trace("Commiting Edit File page: " + commitIdAtLoad.getObject());
+						logger().trace("Commiting Edit File page: " + commitIdAtLoad.getObject());
 						
 						DirCache index = DirCache.newInCore();
 						DirCacheBuilder builder = index.builder();
diff --git a/src/main/java/com/gitblit/wicket/pages/MetricsPage.java b/src/main/java/com/gitblit/wicket/pages/MetricsPage.java
index 96113b0f..77811687 100644
--- a/src/main/java/com/gitblit/wicket/pages/MetricsPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/MetricsPage.java
@@ -94,7 +94,7 @@ public class MetricsPage extends RepositoryPage {
 				try {
 					date = df.parse(metric.name);
 				} catch (ParseException e) {
-					logger.error("Unable to parse date: " + metric.name);
+					logger().error("Unable to parse date: " + metric.name);
 					return;
 				}
 				chart.addValue(date, (int)metric.count);
diff --git a/src/main/java/com/gitblit/wicket/pages/RawPage.java b/src/main/java/com/gitblit/wicket/pages/RawPage.java
index c4357478..bd901f24 100644
--- a/src/main/java/com/gitblit/wicket/pages/RawPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/RawPage.java
@@ -45,7 +45,7 @@ import com.gitblit.wicket.WicketUtils;
 
 public class RawPage extends SessionPage {
 
-	private final Logger logger = LoggerFactory.getLogger(getClass().getSimpleName());
+	private transient Logger logger;
 
 	String contentType;
 
@@ -95,7 +95,7 @@ public class RawPage extends SessionPage {
 					if (binary == null) {
 						final String objectNotFound = MessageFormat.format("Raw page failed to find object {0} in {1}",
 								objectId, repositoryName);
-						logger.error(objectNotFound);
+						logger().error(objectNotFound);
 						throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, objectNotFound);
 					}
 					contentType = "application/octet-stream";
@@ -104,7 +104,7 @@ public class RawPage extends SessionPage {
 					try {
 						response.getOutputStream().write(binary);
 					} catch (Exception e) {
-						logger.error("Failed to write binary response", e);
+						logger().error("Failed to write binary response", e);
 					}
 				} else {
 					// standard raw blob view
@@ -112,7 +112,7 @@ public class RawPage extends SessionPage {
 					if (commit == null) {
 						final String commitNotFound = MessageFormat.format("Raw page failed to find commit {0} in {1}",
 								objectId, repositoryName);
-						logger.error(commitNotFound);
+						logger().error(commitNotFound);
 						throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, commitNotFound);
 					}
 
@@ -148,7 +148,7 @@ public class RawPage extends SessionPage {
 							// image blobs
 							byte[] image = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true);
 							if (image == null) {
-								logger.error(blobNotFound);
+								logger().error(blobNotFound);
 								throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
 							}
 							contentType = "image/" + extension.toLowerCase();
@@ -157,14 +157,14 @@ public class RawPage extends SessionPage {
 							try {
 								response.getOutputStream().write(image);
 							} catch (IOException e) {
-								logger.error("Failed to write image response", e);
+								logger().error("Failed to write image response", e);
 							}
 							break;
 						case 3:
 							// binary blobs (download)
 							byte[] binary = JGitUtils.getByteContent(r, commit.getTree(), blobPath, true);
 							if (binary == null) {
-								logger.error(blobNotFound);
+								logger().error(blobNotFound);
 								throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
 							}
 							contentType = "application/octet-stream";
@@ -193,7 +193,7 @@ public class RawPage extends SessionPage {
 							try {
 								response.getOutputStream().write(binary);
 							} catch (IOException e) {
-								logger.error("Failed to write binary response", e);
+								logger().error("Failed to write binary response", e);
 							}
 							break;
 						default:
@@ -201,7 +201,7 @@ public class RawPage extends SessionPage {
 							String content = JGitUtils.getStringContent(r, commit.getTree(),
 									blobPath, encodings);
 							if (content == null) {
-								logger.error(blobNotFound);
+								logger().error(blobNotFound);
 								throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
 							}
 							contentType = "text/plain; charset=UTF-8";
@@ -209,7 +209,7 @@ public class RawPage extends SessionPage {
 							try {
 								response.getOutputStream().write(content.getBytes("UTF-8"));
 							} catch (Exception e) {
-								logger.error("Failed to write text response", e);
+								logger().error("Failed to write text response", e);
 							}
 						}
 
@@ -218,7 +218,7 @@ public class RawPage extends SessionPage {
 						String content = JGitUtils.getStringContent(r, commit.getTree(), blobPath,
 								encodings);
 						if (content == null) {
-							logger.error(blobNotFound);
+							logger().error(blobNotFound);
 							throw new AbortWithWebErrorCodeException(HttpServletResponse.SC_NOT_FOUND, blobNotFound);
 						}
 						contentType = "text/plain; charset=UTF-8";
@@ -226,7 +226,7 @@ public class RawPage extends SessionPage {
 						try {
 							response.getOutputStream().write(content.getBytes("UTF-8"));
 						} catch (Exception e) {
-							logger.error("Failed to write text response", e);
+							logger().error("Failed to write text response", e);
 						}
 					}
 				}
@@ -235,6 +235,13 @@ public class RawPage extends SessionPage {
 		});
 	}
 
+	protected Logger logger() {
+		if (logger == null) {
+			logger = LoggerFactory.getLogger(getClass());
+		}
+		return logger;
+	}
+
 	@Override
 	protected void setHeaders(WebResponse response) {
 		super.setHeaders(response);
diff --git a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java
index 36c5ae16..4d30e049 100644
--- a/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/RepositoryPage.java
@@ -42,8 +42,6 @@ import org.eclipse.jgit.diff.DiffEntry.ChangeType;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import com.gitblit.Constants;
 import com.gitblit.GitBlitException;
@@ -78,8 +76,6 @@ import com.google.common.base.Optional;
 
 public abstract class RepositoryPage extends RootPage {
 
-	protected final Logger logger = LoggerFactory.getLogger(getClass());
-
 	private final String PARAM_STAR = "star";
 
 	protected final String projectName;
@@ -93,7 +89,7 @@ public abstract class RepositoryPage extends RootPage {
 	private Map<String, SubmoduleModel> submodules;
 
 	private boolean showAdmin;
-	private boolean isOwner;
+	private final boolean isOwner;
 
 	public RepositoryPage(PageParameters params) {
 		super(params);
@@ -144,7 +140,7 @@ public abstract class RepositoryPage extends RootPage {
 				try {
 					app().gitblit().reviseUser(user.username, user);
 				} catch (GitBlitException e) {
-					logger.error("Failed to update user " + user.username, e);
+					logger().error("Failed to update user " + user.username, e);
 					error(getString("gb.failedToUpdateUser"), false);
 				}
 			}
@@ -579,7 +575,7 @@ public abstract class RepositoryPage extends RootPage {
 	}
 
 	protected void addRefs(Repository r, RevCommit c) {
-		add(new RefsPanel("refsPanel", repositoryName, c, JGitUtils.getAllRefs(r, getRepositoryModel().showRemoteBranches)));
+		add(new RefsPanel("refsPanel", repositoryName, JGitUtils.getAllRefs(r, getRepositoryModel().showRemoteBranches).get(c.getId())));
 	}
 
 	protected void addFullText(String wicketId, String text) {
diff --git a/src/main/java/com/gitblit/wicket/pages/SummaryPage.java b/src/main/java/com/gitblit/wicket/pages/SummaryPage.java
index 3cfa152e..1a5f0518 100644
--- a/src/main/java/com/gitblit/wicket/pages/SummaryPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/SummaryPage.java
@@ -190,7 +190,7 @@ public class SummaryPage extends RepositoryPage {
 			try {
 				date = df.parse(metric.name);
 			} catch (ParseException e) {
-				logger.error("Unable to parse date: " + metric.name);
+				logger().error("Unable to parse date: " + metric.name);
 				return charts;
 			}
 			chart.addValue(date, (int)metric.count);
diff --git a/src/main/java/com/gitblit/wicket/pages/TicketPage.java b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
index e2133966..1750b859 100644
--- a/src/main/java/com/gitblit/wicket/pages/TicketPage.java
+++ b/src/main/java/com/gitblit/wicket/pages/TicketPage.java
@@ -66,6 +66,7 @@ import com.gitblit.git.PatchsetCommand;
 import com.gitblit.git.PatchsetReceivePack;
 import com.gitblit.models.PathModel.PathChangeModel;
 import com.gitblit.models.RegistrantAccessPermission;
+import com.gitblit.models.RepositoryCommit;
 import com.gitblit.models.RepositoryModel;
 import com.gitblit.models.RepositoryUrl;
 import com.gitblit.models.SubmoduleModel;
@@ -815,13 +816,17 @@ public class TicketPage extends RepositoryPage {
 
 			// commits
 			List<RevCommit> commits = JGitUtils.getRevLog(getRepository(), currentPatchset.base, currentPatchset.tip);
-			ListDataProvider<RevCommit> commitsDp = new ListDataProvider<RevCommit>(commits);
-			DataView<RevCommit> commitsView = new DataView<RevCommit>("commit", commitsDp) {
+			List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
+			for (RevCommit c : commits) {
+				repoCommits.add(new RepositoryCommit(repositoryName, "", c));
+			}
+			ListDataProvider<RepositoryCommit> commitsDp = new ListDataProvider<RepositoryCommit>(repoCommits);
+			DataView<RepositoryCommit> commitsView = new DataView<RepositoryCommit>("commit", commitsDp) {
 				private static final long serialVersionUID = 1L;
 
 				@Override
-				public void populateItem(final Item<RevCommit> item) {
-					RevCommit commit = item.getModelObject();
+				public void populateItem(final Item<RepositoryCommit> item) {
+					RepositoryCommit commit = item.getModelObject();
 					PersonIdent author = commit.getAuthorIdent();
 					item.add(new AvatarImage("authorAvatar", author.getName(), author.getEmailAddress(), null, 16, false));
 					item.add(new Label("author", commit.getAuthorIdent().getName()));
@@ -830,7 +835,7 @@ public class TicketPage extends RepositoryPage {
 					item.add(new LinkPanel("diff", "link", getString("gb.diff"), CommitDiffPage.class,
 							WicketUtils.newObjectParameter(repositoryName, commit.getName()), true));
 					item.add(new Label("title", StringUtils.trimString(commit.getShortMessage(), Constants.LEN_SHORTLOG_REFS)));
-					item.add(WicketUtils.createDateLabel("commitDate", JGitUtils.getAuthorDate(commit), GitBlitWebSession
+					item.add(WicketUtils.createDateLabel("commitDate", author.getWhen(), GitBlitWebSession
 							.get().getTimezone(), getTimeUtils(), false));
 					item.add(new DiffStatPanel("commitDiffStat", 0, 0, true));
 				}
@@ -1451,7 +1456,7 @@ public class TicketPage extends RepositoryPage {
 											} else {
 												// merge failure
 												String msg = MessageFormat.format("Failed to merge ticket {0,number,0}: {1}", ticket.number, result.name());
-												logger.error(msg);
+												logger().error(msg);
 												GitBlitWebSession.get().cacheErrorMessage(msg);
 											}
 										}
@@ -1461,13 +1466,13 @@ public class TicketPage extends RepositoryPage {
 									String msg = MessageFormat.format("Can not merge ticket {0,number,0}, patchset {1,number,0} has been vetoed!",
 											ticket.number, patchset.number);
 									GitBlitWebSession.get().cacheErrorMessage(msg);
-									logger.error(msg);
+									logger().error(msg);
 								}
 							} else {
 								// not current patchset
 								String msg = MessageFormat.format("Can not merge ticket {0,number,0}, the patchset has been updated!", ticket.number);
 								GitBlitWebSession.get().cacheErrorMessage(msg);
-								logger.error(msg);
+								logger().error(msg);
 							}
 							
 							redirectTo(TicketsPage.class, getPageParameters());
diff --git a/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java b/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java
index 75fd70e7..737fe5aa 100644
--- a/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/HistoryPanel.java
@@ -44,7 +44,9 @@ import com.gitblit.Keys;
 import com.gitblit.models.PathModel;
 import com.gitblit.models.PathModel.PathChangeModel;
 import com.gitblit.models.RefModel;
+import com.gitblit.models.RepositoryCommit;
 import com.gitblit.models.SubmoduleModel;
+import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.JGitUtils;
 import com.gitblit.utils.MarkdownUtils;
 import com.gitblit.utils.StringUtils;
@@ -61,7 +63,7 @@ public class HistoryPanel extends BasePanel {
 
 	private static final long serialVersionUID = 1L;
 
-	private boolean hasMore;
+	private final boolean hasMore;
 
 	public HistoryPanel(String wicketId, final String repositoryName, final String objectId,
 			final String path, Repository r, int limit, int pageOffset, boolean showRemoteRefs) {
@@ -101,10 +103,9 @@ public class HistoryPanel extends BasePanel {
 			if (matchingPath == null) {
 				// path not in commit
 				// manually locate path in tree
-				TreeWalk tw = new TreeWalk(r);
-				tw.reset();
-				tw.setRecursive(true);
-				try {
+				try (TreeWalk tw = new TreeWalk(r)) {
+					tw.reset();
+					tw.setRecursive(true);
 					tw.addTree(commit.getTree());
 					tw.setFilter(PathFilterGroup.createFromStrings(Collections.singleton(path)));
 					while (tw.next()) {
@@ -115,8 +116,6 @@ public class HistoryPanel extends BasePanel {
 						}
 					}
 				} catch (Exception e) {
-				} finally {
-					tw.close();
 				}
 			}
 		}
@@ -136,7 +135,7 @@ public class HistoryPanel extends BasePanel {
 			hasSubmodule = false;
 		}
 
-		final Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
+		Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
 		List<RevCommit> commits;
 		if (pageResults) {
 			// Paging result set
@@ -152,15 +151,23 @@ public class HistoryPanel extends BasePanel {
 		hasMore = commits.size() >= itemsPerPage;
 
 		final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6);
-		ListDataProvider<RevCommit> dp = new ListDataProvider<RevCommit>(commits);
-		DataView<RevCommit> logView = new DataView<RevCommit>("commit", dp) {
+		List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
+		for (RevCommit c : commits) {
+			RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c);
+			if (allRefs.containsKey(c)) {
+				repoCommit.setRefs(allRefs.get(c));
+			}
+			repoCommits.add(repoCommit);
+		}
+		ListDataProvider<RepositoryCommit> dp = new ListDataProvider<RepositoryCommit>(repoCommits);
+		DataView<RepositoryCommit> logView = new DataView<RepositoryCommit>("commit", dp) {
 			private static final long serialVersionUID = 1L;
 			int counter;
 
 			@Override
-			public void populateItem(final Item<RevCommit> item) {
-				final RevCommit entry = item.getModelObject();
-				final Date date = JGitUtils.getAuthorDate(entry);
+			public void populateItem(final Item<RepositoryCommit> item) {
+				final RepositoryCommit entry = item.getModelObject();
+				Date date = entry.getAuthorIdent().getWhen();
 
 				item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils()));
 
@@ -182,7 +189,7 @@ public class HistoryPanel extends BasePanel {
 
 				String shortMessage = entry.getShortMessage();
 				String trimmedMessage = shortMessage;
-				if (allRefs.containsKey(entry.getId())) {
+				if (!ArrayUtils.isEmpty(entry.getRefs())) {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS);
 				} else {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG);
@@ -195,7 +202,7 @@ public class HistoryPanel extends BasePanel {
 				}
 				item.add(shortlog);
 
-				item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs));
+				item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));
 
 				if (isTree) {
 					// tree
@@ -214,7 +221,7 @@ public class HistoryPanel extends BasePanel {
 				} else if (isSubmodule) {
 					// submodule
 					Repository repository = app().repositories().getRepository(repositoryName);
-					String submoduleId = JGitUtils.getSubmoduleCommitId(repository, path, entry);
+					String submoduleId = JGitUtils.getSubmoduleCommitId(repository, path, entry.getCommit());
 					repository.close();
 					if (StringUtils.isEmpty(submoduleId)) {
 						// not a submodule at this commit, just a matching path
diff --git a/src/main/java/com/gitblit/wicket/panels/LogPanel.java b/src/main/java/com/gitblit/wicket/panels/LogPanel.java
index e9d240d0..4521d430 100644
--- a/src/main/java/com/gitblit/wicket/panels/LogPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/LogPanel.java
@@ -15,6 +15,7 @@
  */
 package com.gitblit.wicket.panels;
 
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -35,7 +36,9 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import com.gitblit.Constants;
 import com.gitblit.Keys;
 import com.gitblit.models.RefModel;
+import com.gitblit.models.RepositoryCommit;
 import com.gitblit.servlet.BranchGraphServlet;
+import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.JGitUtils;
 import com.gitblit.utils.StringUtils;
 import com.gitblit.wicket.ExternalImage;
@@ -50,7 +53,7 @@ public class LogPanel extends BasePanel {
 
 	private static final long serialVersionUID = 1L;
 
-	private boolean hasMore;
+	private final boolean hasMore;
 
 	public LogPanel(String wicketId, final String repositoryName, final String objectId,
 			Repository r, int limit, int pageOffset, boolean showRemoteRefs) {
@@ -61,7 +64,7 @@ public class LogPanel extends BasePanel {
 			itemsPerPage = 50;
 		}
 
-		final Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
+		Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
 		List<RevCommit> commits;
 		if (pageResults) {
 			// Paging result set
@@ -75,8 +78,8 @@ public class LogPanel extends BasePanel {
 		// works unless commits.size() represents the exact end.
 		hasMore = commits.size() >= itemsPerPage;
 
-		final String baseUrl = WicketUtils.getGitblitURL(getRequest());
-		final boolean showGraph = app().settings().getBoolean(Keys.web.showBranchGraph, true);
+		String baseUrl = WicketUtils.getGitblitURL(getRequest());
+		boolean showGraph = app().settings().getBoolean(Keys.web.showBranchGraph, true);
 
 		MarkupContainer graph = new WebMarkupContainer("graph");
 		add(graph);
@@ -101,15 +104,23 @@ public class LogPanel extends BasePanel {
 		}
 
 		final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6);
-		ListDataProvider<RevCommit> dp = new ListDataProvider<RevCommit>(commits);
-		DataView<RevCommit> logView = new DataView<RevCommit>("commit", dp) {
+		List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
+		for (RevCommit c : commits) {
+			RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c);
+			if (allRefs.containsKey(c)) {
+				repoCommit.setRefs(allRefs.get(c));
+			}
+			repoCommits.add(repoCommit);
+		}
+		ListDataProvider<RepositoryCommit> dp = new ListDataProvider<RepositoryCommit>(repoCommits);
+		DataView<RepositoryCommit> logView = new DataView<RepositoryCommit>("commit", dp) {
 			private static final long serialVersionUID = 1L;
 			int counter;
 
 			@Override
-			public void populateItem(final Item<RevCommit> item) {
-				final RevCommit entry = item.getModelObject();
-				final Date date = JGitUtils.getAuthorDate(entry);
+			public void populateItem(final Item<RepositoryCommit> item) {
+				final RepositoryCommit entry = item.getModelObject();
+				final Date date = entry.getAuthorIdent().getWhen();
 				final boolean isMerge = entry.getParentCount() > 1;
 
 				item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils()));
@@ -132,7 +143,7 @@ public class LogPanel extends BasePanel {
 				// short message
 				String shortMessage = entry.getShortMessage();
 				String trimmedMessage = shortMessage;
-				if (allRefs.containsKey(entry.getId())) {
+				if (!ArrayUtils.isEmpty(entry.getRefs())) {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS);
 				} else {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG);
@@ -145,7 +156,7 @@ public class LogPanel extends BasePanel {
 				}
 				item.add(shortlog);
 
-				item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs));
+				item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));
 
 				// commit hash link
 				LinkPanel commitHash = new LinkPanel("hashLink", null, entry.getName().substring(0, hashLen),
diff --git a/src/main/java/com/gitblit/wicket/panels/SearchPanel.java b/src/main/java/com/gitblit/wicket/panels/SearchPanel.java
index 09322bc7..b29adcb5 100644
--- a/src/main/java/com/gitblit/wicket/panels/SearchPanel.java
+++ b/src/main/java/com/gitblit/wicket/panels/SearchPanel.java
@@ -15,6 +15,7 @@
  */
 package com.gitblit.wicket.panels;
 
+import java.util.ArrayList;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
@@ -31,6 +32,8 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import com.gitblit.Constants;
 import com.gitblit.Keys;
 import com.gitblit.models.RefModel;
+import com.gitblit.models.RepositoryCommit;
+import com.gitblit.utils.ArrayUtils;
 import com.gitblit.utils.JGitUtils;
 import com.gitblit.utils.StringUtils;
 import com.gitblit.wicket.WicketUtils;
@@ -43,7 +46,7 @@ public class SearchPanel extends BasePanel {
 
 	private static final long serialVersionUID = 1L;
 
-	private boolean hasMore;
+	private final boolean hasMore;
 
 	public SearchPanel(String wicketId, final String repositoryName, final String objectId,
 			final String value, Constants.SearchType searchType, Repository r, int limit, int pageOffset,
@@ -57,7 +60,7 @@ public class SearchPanel extends BasePanel {
 
 		RevCommit commit = JGitUtils.getCommit(r, objectId);
 
-		final Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
+		Map<ObjectId, List<RefModel>> allRefs = JGitUtils.getAllRefs(r, showRemoteRefs);
 		List<RevCommit> commits;
 		if (pageResults) {
 			// Paging result set
@@ -78,15 +81,23 @@ public class SearchPanel extends BasePanel {
 		add(new Label("searchString", value));
 		add(new Label("searchType", searchType.toString()));
 
-		ListDataProvider<RevCommit> dp = new ListDataProvider<RevCommit>(commits);
-		DataView<RevCommit> searchView = new DataView<RevCommit>("commit", dp) {
+		List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
+		for (RevCommit c : commits) {
+			RepositoryCommit repoCommit = new RepositoryCommit(repositoryName, "", c);
+			if (allRefs.containsKey(c)) {
+				repoCommit.setRefs(allRefs.get(c));
+			}
+			repoCommits.add(repoCommit);
+		}
+		ListDataProvider<RepositoryCommit> dp = new ListDataProvider<RepositoryCommit>(repoCommits);
+		DataView<RepositoryCommit> searchView = new DataView<RepositoryCommit>("commit", dp) {
 			private static final long serialVersionUID = 1L;
 			int counter;
 
 			@Override
-			public void populateItem(final Item<RevCommit> item) {
-				final RevCommit entry = item.getModelObject();
-				final Date date = JGitUtils.getAuthorDate(entry);
+			public void populateItem(final Item<RepositoryCommit> item) {
+				final RepositoryCommit entry = item.getModelObject();
+				final Date date = entry.getAuthorIdent().getWhen();
 
 				item.add(WicketUtils.createDateLabel("commitDate", date, getTimeZone(), getTimeUtils()));
 
@@ -107,7 +118,7 @@ public class SearchPanel extends BasePanel {
 
 				String shortMessage = entry.getShortMessage();
 				String trimmedMessage = shortMessage;
-				if (allRefs.containsKey(entry.getId())) {
+				if (!ArrayUtils.isEmpty(entry.getRefs())) {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG_REFS);
 				} else {
 					trimmedMessage = StringUtils.trimString(shortMessage, Constants.LEN_SHORTLOG);
@@ -120,7 +131,7 @@ public class SearchPanel extends BasePanel {
 				}
 				item.add(shortlog);
 
-				item.add(new RefsPanel("commitRefs", repositoryName, entry, allRefs));
+				item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));
 
 				item.add(new BookmarkablePageLink<Void>("commit", CommitPage.class, WicketUtils
 						.newObjectParameter(repositoryName, entry.getName())));
-- 
cgit v1.2.3