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

1 files changed, 22 insertions, 11 deletions

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),