Merge branch 'stable-2.15' into stable-2.16

* stable-2.15:
  ReceiveCommits: Validate ref operation before commits for non-ff case
  Add test coverage for ref operation validation extension point
  Upgrade JGit to 4.11.8.201904181247-r
  CreateProject: Expose createProject method
  ElasticContainer: Switch to Alpine-based images from blacktop
  Bazel: Fix lint warning flagged by buildifier

Change-Id: I43f6b82744788ffe6635abc3c06af57cf0629a5e

java/com/google/gerrit/server/git/receive/ReceiveCommits.java

@@ -1249,11 +1249,8 @@ class ReceiveCommits {
   }
 
   private void parseRewind(ReceiveCommand cmd) throws PermissionBackendException {
-    RevCommit newObject;
     try {
-      newObject = receivePack.getRevWalk().parseCommit(cmd.getNewId());
-    } catch (IncorrectObjectTypeException notCommit) {
-      newObject = null;
+      receivePack.getRevWalk().parseCommit(cmd.getNewId());
     } catch (IOException err) {
       logger.atSevere().withCause(err).log(
           "Invalid object %s for %s forced update", cmd.getNewId().name(), cmd.getRefName());
@@ -1262,11 +1259,12 @@ class ReceiveCommits {
     }
     logger.atFine().log("Rewinding %s", cmd);
 
-    if (newObject != null) {
-      validateRegularPushCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd);
-      if (cmd.getResult() != NOT_ATTEMPTED) {
-        return;
-      }
+    if (!validRefOperation(cmd)) {
+      return;
+    }
+    validateRegularPushCommits(new Branch.NameKey(project.getNameKey(), cmd.getRefName()), cmd);
+    if (cmd.getResult() != NOT_ATTEMPTED) {
+      return;
     }
 
     Optional<AuthException> err = checkRefPermission(cmd, RefPermission.FORCE_UPDATE);

java/com/google/gerrit/server/restapi/project/CreateProject.java

@@ -240,7 +240,8 @@ public class CreateProject
     }
   }
 
-  private ProjectState createProject(CreateProjectArgs args)
+  // TODO(dpursehouse): Add @UsedAt annotation
+  public ProjectState createProject(CreateProjectArgs args)
       throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
     final Project.NameKey nameKey = args.getProject();
     try {

javatests/com/google/gerrit/acceptance/git/RefOperationValidationIT.java

@@ -0,0 +1,198 @@
+// Copyright (C) 2019 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.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.acceptance.GitUtil.deleteRef;
+import static org.eclipse.jgit.lib.Constants.HEAD;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.CREATE;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.DELETE;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE;
+import static org.eclipse.jgit.transport.ReceiveCommand.Type.UPDATE_NONFASTFORWARD;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.projects.BranchInput;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.RegistrationHandle;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.events.RefReceivedEvent;
+import com.google.gerrit.server.git.validators.RefOperationValidationListener;
+import com.google.gerrit.server.git.validators.ValidationMessage;
+import com.google.gerrit.server.validators.ValidationException;
+import com.google.inject.Inject;
+import java.util.Collections;
+import java.util.List;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.PushResult;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.transport.RemoteRefUpdate;
+import org.junit.Test;
+
+public class RefOperationValidationIT extends AbstractDaemonTest {
+  private static final String TEST_REF = "refs/heads/protected";
+
+  @Inject DynamicSet<RefOperationValidationListener> validators;
+
+  private class TestRefValidator implements RefOperationValidationListener, AutoCloseable {
+    private final ReceiveCommand.Type rejectType;
+    private final String rejectRef;
+    private final RegistrationHandle handle;
+
+    public TestRefValidator(ReceiveCommand.Type rejectType) {
+      this.rejectType = rejectType;
+      this.rejectRef = TEST_REF;
+      this.handle = validators.add("test-" + rejectType.name(), this);
+    }
+
+    @Override
+    public List<ValidationMessage> onRefOperation(RefReceivedEvent refEvent)
+        throws ValidationException {
+      if (refEvent.getRefName().equals(rejectRef)
+          && refEvent.command.getType().equals(rejectType)) {
+        throw new ValidationException(rejectType.name());
+      }
+      return Collections.emptyList();
+    }
+
+    @Override
+    public void close() throws Exception {
+      handle.remove();
+    }
+  }
+
+  @Test
+  public void rejectRefCreation() throws Exception {
+    try (TestRefValidator validator = new TestRefValidator(CREATE)) {
+      gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+      assert_().fail("expected exception");
+    } catch (RestApiException expected) {
+      assertThat(expected).hasMessageThat().contains(CREATE.name());
+    }
+  }
+
+  @Test
+  public void rejectRefCreationByPush() throws Exception {
+    try (TestRefValidator validator = new TestRefValidator(CREATE)) {
+      grant(project, "refs/*", Permission.PUSH, true);
+      PushOneCommit push1 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+      PushOneCommit.Result r1 = push1.to("refs/heads/master");
+      r1.assertOkStatus();
+      PushOneCommit.Result r2 = push1.to(TEST_REF);
+      r2.assertErrorStatus(CREATE.name());
+    }
+  }
+
+  @Test
+  public void rejectRefDeletion() throws Exception {
+    gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+    try (TestRefValidator validator = new TestRefValidator(DELETE)) {
+      gApi.projects().name(project.get()).branch(TEST_REF).delete();
+      assert_().fail("expected exception");
+    } catch (RestApiException expected) {
+      assertThat(expected).hasMessageThat().contains(DELETE.name());
+    }
+  }
+
+  @Test
+  public void rejectRefDeletionByPush() throws Exception {
+    gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+    grant(project, "refs/*", Permission.DELETE, true);
+    try (TestRefValidator validator = new TestRefValidator(DELETE)) {
+      PushResult result = deleteRef(testRepo, TEST_REF);
+      RemoteRefUpdate refUpdate = result.getRemoteUpdate(TEST_REF);
+      assertThat(refUpdate.getMessage()).contains(DELETE.name());
+    }
+  }
+
+  @Test
+  public void rejectRefUpdateFastForward() throws Exception {
+    gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+    try (TestRefValidator validator = new TestRefValidator(UPDATE)) {
+      grant(project, "refs/*", Permission.PUSH, true);
+      PushOneCommit push1 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+      PushOneCommit.Result r1 = push1.to(TEST_REF);
+      r1.assertErrorStatus(UPDATE.name());
+    }
+  }
+
+  @Test
+  public void rejectRefUpdateNonFastForward() throws Exception {
+    gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+    try (TestRefValidator validator = new TestRefValidator(UPDATE_NONFASTFORWARD)) {
+      ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
+      grant(project, "refs/*", Permission.PUSH, true);
+      PushOneCommit push1 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+      PushOneCommit.Result r1 = push1.to(TEST_REF);
+      r1.assertOkStatus();
+
+      // Reset HEAD to initial so the new change is a non-fast forward
+      RefUpdate ru = repo().updateRef(HEAD);
+      ru.setNewObjectId(initial);
+      assertThat(ru.forceUpdate()).isEqualTo(RefUpdate.Result.FORCED);
+
+      PushOneCommit push2 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+      push2.setForce(true);
+      PushOneCommit.Result r2 = push2.to(TEST_REF);
+      r2.assertErrorStatus(UPDATE_NONFASTFORWARD.name());
+    }
+  }
+
+  @Test
+  public void rejectRefUpdateNonFastForwardToExistingCommit() throws Exception {
+    gApi.projects().name(project.get()).branch(TEST_REF).create(new BranchInput());
+
+    try (TestRefValidator validator = new TestRefValidator(UPDATE_NONFASTFORWARD)) {
+      grant(project, "refs/*", Permission.PUSH, true);
+      PushOneCommit push1 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change1", "a.txt", "content");
+      PushOneCommit.Result r1 = push1.to("refs/heads/master");
+      r1.assertOkStatus();
+      ObjectId push1Id = r1.getCommit();
+
+      PushOneCommit push2 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change2", "b.txt", "content");
+      PushOneCommit.Result r2 = push2.to("refs/heads/master");
+      r2.assertOkStatus();
+      ObjectId push2Id = r2.getCommit();
+
+      RefUpdate ru = repo().updateRef(HEAD);
+      ru.setNewObjectId(push1Id);
+      assertThat(ru.forceUpdate()).isEqualTo(RefUpdate.Result.FORCED);
+
+      PushOneCommit push3 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change3", "c.txt", "content");
+      PushOneCommit.Result r3 = push3.to(TEST_REF);
+      r3.assertOkStatus();
+
+      ru = repo().updateRef(HEAD);
+      ru.setNewObjectId(push2Id);
+      assertThat(ru.forceUpdate()).isEqualTo(RefUpdate.Result.FORCED);
+
+      PushOneCommit push4 =
+          pushFactory.create(db, admin.getIdent(), testRepo, "change4", "d.txt", "content");
+      push4.setForce(true);
+      PushOneCommit.Result r4 = push4.to(TEST_REF);
+      r4.assertErrorStatus(UPDATE_NONFASTFORWARD.name());
+    }
+  }
+}

javatests/com/google/gerrit/elasticsearch/ElasticContainer.java

@@ -37,23 +37,23 @@ public class ElasticContainer extends ElasticsearchContainer {
   private static String getImageName(ElasticVersion version) {
     switch (version) {
       case V5_6:
-        return "docker.elastic.co/elasticsearch/elasticsearch:5.6.16";
+        return "blacktop/elasticsearch:5.6.16";
       case V6_2:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.2.4";
+        return "blacktop/elasticsearch:6.2.4";
       case V6_3:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.3.2";
+        return "blacktop/elasticsearch:6.3.2";
       case V6_4:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.4.3";
+        return "blacktop/elasticsearch:6.4.3";
       case V6_5:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.5.4";
+        return "blacktop/elasticsearch:6.5.4";
       case V6_6:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.6.2";
+        return "blacktop/elasticsearch:6.6.2";
       case V6_7:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:6.7.2";
+        return "blacktop/elasticsearch:6.7.2";
       case V7_0:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:7.0.1";
+        return "blacktop/elasticsearch:7.0.1";
       case V7_1:
-        return "docker.elastic.co/elasticsearch/elasticsearch-oss:7.1.1";
+        return "blacktop/elasticsearch:7.1.1";
     }
     throw new IllegalStateException("No tests for version: " + version.name());
   }

tools/bzl/javadoc.bzl

@@ -17,8 +17,8 @@
 def _impl(ctx):
     zip_output = ctx.outputs.zip
 
-    transitive_jars = depset(transitive = [l.java.transitive_deps for l in ctx.attr.libs])
-    source_jars = depset(transitive = [l.java.source_jars for l in ctx.attr.libs])
+    transitive_jars = depset(transitive = [j.java.transitive_deps for j in ctx.attr.libs])
+    source_jars = depset(transitive = [j.java.source_jars for j in ctx.attr.libs])
 
     transitive_jar_paths = [j.path for j in transitive_jars.to_list()]
     dir = ctx.outputs.zip.path + ".dir"

tools/bzl/junit.bzl

@@ -28,8 +28,8 @@ public class %s {}
 
 _PREFIXES = ("org", "com", "edu")
 
-def _SafeIndex(l, val):
-    for i, v in enumerate(l):
+def _SafeIndex(j, val):
+    for i, v in enumerate(j):
         if val == v:
             return i
     return -1

tools/bzl/pkg_war.bzl

@@ -77,11 +77,11 @@ def _war_impl(ctx):
 
     # Add lib
     transitive_libs = []
-    for l in ctx.attr.libs:
-        if hasattr(l, "java"):
-            transitive_libs.append(l.java.transitive_runtime_deps)
-        elif hasattr(l, "files"):
-            transitive_libs.append(l.files)
+    for j in ctx.attr.libs:
+        if hasattr(j, "java"):
+            transitive_libs.append(j.java.transitive_runtime_deps)
+        elif hasattr(j, "files"):
+            transitive_libs.append(j.files)
 
     transitive_lib_deps = depset(transitive = transitive_libs)
     for dep in transitive_lib_deps.to_list():
@@ -90,8 +90,8 @@ def _war_impl(ctx):
 
     # Add pgm lib
     transitive_pgmlibs = []
-    for l in ctx.attr.pgmlibs:
-        transitive_pgmlibs.append(l.java.transitive_runtime_deps)
+    for j in ctx.attr.pgmlibs:
+        transitive_pgmlibs.append(j.java.transitive_runtime_deps)
 
     transitive_pgmlib_deps = depset(transitive = transitive_pgmlibs)
     for dep in transitive_pgmlib_deps.to_list():