Fix rejection of banned commits
Commit 936470ed2e9967e9e509b8472a3e1872b5e5fe77 broke the rejection of
banned commits. During the refactoring the check for banned commits
was removed. Add a new commit validator that rejects pushes that
contain banned commits.
Change-Id: I1645dd437129336fcbfb0f18ee1c801db973d482
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
new file mode 100644
index 0000000..bfce523
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/BanCommitIT.java
@@ -0,0 +1,52 @@
+// Copyright (C) 2014 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.ssh;
+
+import static com.google.gerrit.acceptance.GitUtil.add;
+import static com.google.gerrit.acceptance.GitUtil.createCommit;
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GitUtil.Commit;
+
+import com.jcraft.jsch.JSchException;
+
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.transport.PushResult;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Locale;
+
+public class BanCommitIT extends AbstractDaemonTest {
+
+ @Test
+ public void banCommit() throws IOException, GitAPIException, JSchException {
+ add(git, "a.txt", "some content");
+ Commit c = createCommit(git, admin.getIdent(), "subject");
+
+ String response =
+ sshSession.exec("gerrit ban-commit " + project.get() + " "
+ + c.getCommit().getName());
+ assertFalse(sshSession.hasError());
+ assertFalse(response, response.toLowerCase(Locale.US).contains("error"));
+
+ PushResult pushResult = pushHead(git, "refs/heads/master", false);
+ assertTrue(pushResult.getRemoteUpdate("refs/heads/master").getMessage()
+ .startsWith("contains banned commit"));
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
index 7a64155..329e7e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java
@@ -18,6 +18,7 @@
import com.google.gerrit.common.errors.PermissionDeniedException;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ProjectControl;
@@ -31,9 +32,11 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.Note;
import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
@@ -47,6 +50,34 @@
BanCommit create();
}
+ /**
+ * Loads a list of commits to reject from {@code refs/meta/reject-commits}.
+ *
+ * @param repo repository from which the rejected commits should be loaded
+ * @return NoteMap of commits to be rejected, null if there are none.
+ * @throws IOException the map cannot be loaded.
+ */
+ public static NoteMap loadRejectCommitsMap(Repository repo)
+ throws IOException {
+ try {
+ Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
+ if (ref == null) {
+ return NoteMap.newEmptyMap();
+ }
+
+ RevWalk rw = new RevWalk(repo);
+ try {
+ RevCommit map = rw.parseCommit(ref.getObjectId());
+ return NoteMap.read(rw.getObjectReader(), map);
+ } finally {
+ rw.release();
+ }
+ } catch (IOException badMap) {
+ throw new IOException("Cannot load "
+ + RefNames.REFS_REJECT_COMMITS, badMap);
+ }
+ }
+
private final Provider<IdentifiedUser> currentUser;
private final GitRepositoryManager repoManager;
private final PersonIdent gerritIdent;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 788e1a6..625c757 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -403,7 +403,7 @@
this.project = projectControl.getProject();
this.repo = repo;
this.rp = new ReceivePack(repo);
- this.rejectCommits = loadRejectCommitsMap();
+ this.rejectCommits = BanCommit.loadRejectCommitsMap(repo);
this.subOpFactory = subOpFactory;
this.submitProvider = submitProvider;
@@ -1298,28 +1298,6 @@
}
}
- /**
- * Loads a list of commits to reject from {@code refs/meta/reject-commits}.
- *
- * @return NoteMap of commits to be rejected, null if there are none.
- * @throws IOException the map cannot be loaded.
- */
- private NoteMap loadRejectCommitsMap() throws IOException {
- try {
- Ref ref = repo.getRef(RefNames.REFS_REJECT_COMMITS);
- if (ref == null) {
- return NoteMap.newEmptyMap();
- }
-
- RevWalk rw = rp.getRevWalk();
- RevCommit map = rw.parseCommit(ref.getObjectId());
- return NoteMap.read(rw.getObjectReader(), map);
- } catch (IOException badMap) {
- throw new IOException("Cannot load "
- + RefNames.REFS_REJECT_COMMITS, badMap);
- }
- }
-
private void parseReplaceCommand(final ReceiveCommand cmd,
final Change.Id changeId) {
if (cmd.getType() != ReceiveCommand.Type.CREATE) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 1bf754a..32eabbc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -24,6 +24,7 @@
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.events.CommitReceivedEvent;
+import com.google.gerrit.server.git.BanCommit;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ReceiveCommits;
import com.google.gerrit.server.git.ValidationError;
@@ -40,6 +41,7 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterKey;
import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -47,6 +49,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Collections;
@@ -108,6 +111,7 @@
installCommitMsgHookCommand, sshInfo));
}
validators.add(new ConfigValidator(refControl, repo));
+ validators.add(new BannedCommitsValidator(repo));
validators.add(new PluginCommitValidationListener(commitValidationListeners));
List<CommitValidationMessage> messages =
@@ -524,6 +528,31 @@
}
}
+ /** Reject banned commits. */
+ public static class BannedCommitsValidator implements
+ CommitValidationListener {
+ private final Repository repo;
+
+ public BannedCommitsValidator(Repository repo) {
+ this.repo = repo;
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(
+ CommitReceivedEvent receiveEvent) throws CommitValidationException {
+ try {
+ NoteMap rejectCommits = BanCommit.loadRejectCommitsMap(repo);
+ if (rejectCommits.contains(receiveEvent.commit)) {
+ throw new CommitValidationException("contains banned commit "
+ + receiveEvent.commit.getName());
+ }
+ return Collections.emptyList();
+ } catch (IOException e) {
+ throw new CommitValidationException(e.getMessage(), e);
+ }
+ }
+ }
+
private static CommitValidationMessage getInvalidEmailError(RevCommit c, String type,
PersonIdent who, IdentifiedUser currentUser, String canonicalWebUrl) {
StringBuilder sb = new StringBuilder();
