Skip to content

Commit

Permalink
Issue #21: ensure that inner classes don't reference JGit objects
Browse files Browse the repository at this point in the history 
Gitblit uses a few inner anonymous classes in some pages. Ensure that
those do not hold references to JGit RevCommits to avoid that they get
serialized when the page is.

The usage of serialized inner anonymous classes is an anti-pattern in
Wicket programming and is discouraged in the Java serialization spec,
but cleaning this up properly would amount to a larger refactoring than
can be done in this plugin (and also larger than I'm willing to do).
  • Loading branch information
@tomaswolf
tomaswolf committed Oct 26, 2016
1 parent a3f6b69 commit fc809d3
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 17 deletions.
15 changes: 10 additions & 5 deletions src/main/java/com/gitblit/wicket/panels/HistoryPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
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;
Expand Down Expand Up @@ -132,7 +133,7 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o
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
Expand All @@ -147,9 +148,13 @@ public HistoryPanel(String wicketId, final String repositoryName, final String o
hasMore = commits.size() >= itemsPerPage;

final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6);
final List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
for (RevCommit c : commits) {
repoCommits.add(new RepositoryCommit(repositoryName, "", c));
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) {
Expand Down Expand Up @@ -179,7 +184,7 @@ public void populateItem(final Item<RepositoryCommit> item) {

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);
Expand All @@ -191,7 +196,7 @@ public void populateItem(final Item<RepositoryCommit> item) {
}
item.add(shortlog);

item.add(new RefsPanel("commitRefs", repositoryName, allRefs.get(entry.getId())));
item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));

if (isTree) {
// tree
Expand Down
19 changes: 12 additions & 7 deletions src/main/java/com/gitblit/wicket/panels/LogPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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;
Expand All @@ -63,7 +64,7 @@ public LogPanel(String wicketId, final String repositoryName, final String objec
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
Expand All @@ -77,8 +78,8 @@ public LogPanel(String wicketId, final String repositoryName, final String objec
// 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);
Expand All @@ -103,9 +104,13 @@ public LogPanel(String wicketId, final String repositoryName, final String objec
}

final int hashLen = app().settings().getInteger(Keys.web.shortCommitIdLength, 6);
final List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
for (RevCommit c : commits) {
repoCommits.add(new RepositoryCommit(repositoryName, "", c));
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) {
Expand Down Expand Up @@ -138,7 +143,7 @@ public void populateItem(final Item<RepositoryCommit> item) {
// 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);
Expand All @@ -151,7 +156,7 @@ public void populateItem(final Item<RepositoryCommit> item) {
}
item.add(shortlog);

item.add(new RefsPanel("commitRefs", repositoryName, allRefs.get(entry.getId())));
item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));

// commit hash link
LinkPanel commitHash = new LinkPanel("hashLink", null, entry.getName().substring(0, hashLen),
Expand Down
15 changes: 10 additions & 5 deletions src/main/java/com/gitblit/wicket/panels/SearchPanel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
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;
Expand All @@ -59,7 +60,7 @@ public SearchPanel(String wicketId, final String repositoryName, final String ob

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
Expand All @@ -80,9 +81,13 @@ public SearchPanel(String wicketId, final String repositoryName, final String ob
add(new Label("searchString", value));
add(new Label("searchType", searchType.toString()));

final List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
List<RepositoryCommit> repoCommits = new ArrayList<>(commits.size());
for (RevCommit c : commits) {
repoCommits.add(new RepositoryCommit(repositoryName, "", c));
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) {
Expand Down Expand Up @@ -113,7 +118,7 @@ public void populateItem(final Item<RepositoryCommit> item) {

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);
Expand All @@ -126,7 +131,7 @@ public void populateItem(final Item<RepositoryCommit> item) {
}
item.add(shortlog);

item.add(new RefsPanel("commitRefs", repositoryName, allRefs.get(entry.getId())));
item.add(new RefsPanel("commitRefs", repositoryName, entry.getRefs()));

item.add(new BookmarkablePageLink<Void>("commit", CommitPage.class, WicketUtils
.newObjectParameter(repositoryName, entry.getName())));
Expand Down

0 comments on commit fc809d3

Please sign in to comment.