Merge "Fix rejection of banned commits" into stable-2.9

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

gerrit-server/src/main/java/com/google/gerrit/server/git/BanCommit.java

@@ -18,6 +18,7 @@ import static com.google.gerrit.reviewdb.client.RefNames.REFS_REJECT_COMMITS;
 
 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.Constants;
 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 @@ public class BanCommit {
     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;

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -403,7 +403,7 @@ public class ReceiveCommits {
     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 @@ public class ReceiveCommits {
     }
   }
 
-  /**
-   * 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) {

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -24,6 +24,7 @@ import com.google.gerrit.server.IdentifiedUser;
 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.errors.ConfigInvalidException;
 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.eclipse.jgit.util.SystemReader;
 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 @@ public class CommitValidators {
           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 @@ public class CommitValidators {
     }
   }
 
+  /** 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();