Merge branch 'stable-2.12'

* stable-2.12:
  Update 2.12.4 release notes
  Fix RebaseIfNecessary strategy introducing duplicate commit
  Handle an NPE in ToolsCatalog.java
  Confirm email: Return proper error when email is used by other account
  RestApiServlet: Allow HEAD requests

Change-Id: I15414c40f1136236708cca17128bf60fa020c160

ReleaseNotes/ReleaseNotes-2.12.4.txt

@@ -65,6 +65,26 @@ changes.
 Fix internal error when a query does not contain any token.
 
 * link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4241[Issue 4241]:
-Fix 'Cannot format velocity template' error.
+Fix 'Cannot format velocity template' error when sending notification emails.
 
 * Fix `sshd.idleTimeout` setting being ignored.
++
+Ths `sshd.idleTimeout` setting was not being correctly set on the SSHD
+backend, causing idle sessions to not time out.
+
+* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4324[Issue 4324]:
+Set the correct uploader on new patch sets created via the inline editor.
+
+* Log a warning instead of failing when invalid commentlinks are configured.
+
+* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4136[Issue 4136]:
+Fix support for `HEAD` requests in the REST API.
++
+Sending a `HEAD` request failed with '404 Not Found'.
+
+* Return proper error response when trying to confirm an email that is already
+used by another user.
+
+* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4318[Issue 4318]
+Fix 'Rebase if Necessary' merge strategy to prevent introducing a duplicate
+commit when submitting a merge commit.

gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/HttpResponse.java

@@ -51,6 +51,16 @@ public class HttpResponse {
     return response.getStatusLine().getStatusCode();
   }
 
+  public String getContentType() {
+    return response.getFirstHeader("X-FYI-Content-Type").getValue();
+  }
+
+  public boolean hasContent() {
+    Preconditions.checkNotNull(response,
+        "Response is not initialized.");
+    return response.getEntity() != null;
+  }
+
   public String getEntityContent() throws IOException {
     Preconditions.checkNotNull(response,
         "Response is not initialized.");

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java

@@ -27,8 +27,8 @@ import static org.junit.Assert.fail;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.acceptance.TestAccount;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.changes.ChangeApi;
@@ -77,7 +77,6 @@ import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 
-@NoHttpd
 public class RevisionIT extends AbstractDaemonTest {
 
   @Inject
@@ -622,6 +621,20 @@ public class RevisionIT extends AbstractDaemonTest {
     assertThat(res).isEqualTo(FILE_CONTENT);
   }
 
+  @Test
+  public void contentType() throws Exception {
+    PushOneCommit.Result r = createChange();
+
+    String endPoint = "/changes/" + r.getChangeId()
+      + "/revisions/" + r.getCommit().name()
+      + "/files/" + FILE_NAME
+      + "/content";
+    RestResponse response = adminRestSession.head(endPoint);
+    response.assertOK();
+    assertThat(response.getContentType()).startsWith("text/plain");
+    assertThat(response.hasContent()).isFalse();
+  }
+
   private void assertMergeable(String id, boolean expected) throws Exception {
     MergeableInfo m = gApi.changes().id(id).current().mergeable();
     assertThat(m.mergeable).isEqualTo(expected);

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByRebaseIfNecessaryIT.java

@@ -19,6 +19,7 @@ import static com.google.gerrit.acceptance.GitUtil.getChangeId;
 import static com.google.gerrit.acceptance.GitUtil.pushHead;
 
 import com.google.common.collect.Iterables;
+import com.google.common.collect.ImmutableList;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.TestProjectInput;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
@@ -150,6 +151,57 @@ public class SubmitByRebaseIfNecessaryIT extends AbstractSubmit {
         change4.getChangeId(), headAfterSecondSubmit.name());
   }
 
+  @Test
+  public void submitWithRebaseMergeCommit() throws Exception {
+    /*
+        *  (HEAD, origin/master, origin/HEAD) Merge changes X,Y
+        |\
+        | *   Merge branch 'master' into origin/master
+        | |\
+        | | * SHA Added a
+        | |/
+        * | Before
+        |/
+        * Initial empty repository
+     */
+    RevCommit initialHead = getRemoteHead();
+    PushOneCommit.Result change1 = createChange("Added a", "a.txt", "");
+
+    PushOneCommit change2Push = pushFactory.create(db, admin.getIdent(), testRepo,
+        "Merge to master", "m.txt", "");
+    change2Push.setParents(ImmutableList.of(initialHead, change1.getCommit()));
+    PushOneCommit.Result change2 = change2Push.to("refs/for/master");
+
+    testRepo.reset(initialHead);
+    PushOneCommit.Result change3 = createChange("Before", "b.txt", "");
+
+    approve(change3.getChangeId());
+    submit(change3.getChangeId());
+
+    approve(change1.getChangeId());
+    approve(change2.getChangeId());
+    submit(change2.getChangeId());
+
+    RevCommit newHead = getRemoteHead();
+    assertThat(newHead.getParentCount()).isEqualTo(2);
+
+    RevCommit headParent1 = parse(newHead.getParent(0).getId());
+    RevCommit headParent2 = parse(newHead.getParent(1).getId());
+
+    assertThat(headParent1.getId()).isEqualTo(change3.getCommit().getId());
+    assertThat(headParent1.getParentCount()).isEqualTo(1);
+    assertThat(headParent1.getParent(0)).isEqualTo(initialHead);
+
+    assertThat(headParent2.getId()).isEqualTo(change2.getCommit().getId());
+    assertThat(headParent2.getParentCount()).isEqualTo(2);
+
+    RevCommit headGrandparent1 = parse(headParent2.getParent(0).getId());
+    RevCommit headGrandparent2 = parse(headParent2.getParent(1).getId());
+
+    assertThat(headGrandparent1.getId()).isEqualTo(initialHead.getId());
+    assertThat(headGrandparent2.getId()).isEqualTo(change1.getCommit().getId());
+  }
+
   @Test
   @TestProjectInput(useContentMerge = InheritableBoolean.TRUE)
   public void submitWithContentMerge() throws Exception {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -326,8 +326,7 @@ public class RestApiServlet extends HttpServlet {
         return;
       }
 
-      if (viewData.view instanceof RestReadView<?>
+      if (viewData.view instanceof RestReadView<?> && isGetOrHead(req)) {
-          && "GET".equals(req.getMethod())) {
         result = ((RestReadView<RestResource>) viewData.view).apply(rsrc);
       } else if (viewData.view instanceof RestModifyView<?, ?>) {
         @SuppressWarnings("unchecked")

gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java

@@ -49,6 +49,17 @@ public class RebaseIfNecessary extends SubmitStrategy {
     List<SubmitStrategyOp> ops = new ArrayList<>(sorted.size());
     boolean first = true;
 
+    for (CodeReviewCommit c : sorted) {
+      if (c.getParentCount() > 1) {
+        // Since there is a merge commit, sort and prune again using
+        // MERGE_IF_NECESSARY semantics to avoid creating duplicate
+        // commits.
+        //
+        sorted = args.mergeUtil.reduceToMinimalMerge(args.mergeSorter, sorted);
+        break;
+      }
+    }
+
     while (!sorted.isEmpty()) {
       CodeReviewCommit n = sorted.remove(0);
       if (first && args.mergeTip.getInitialTip() == null) {

gerrit-server/src/main/java/com/google/gerrit/server/tools/ToolsCatalog.java

@@ -16,6 +16,8 @@ package com.google.gerrit.server.tools;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.base.Strings;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.Version;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -60,7 +62,11 @@ public class ToolsCatalog {
    * @param name path of the item, relative to the root of the catalog.
    * @return the entry; null if the item is not part of the catalog.
    */
-  public Entry get(String name) {
+  @Nullable
+  public Entry get(@Nullable String name) {
+    if (Strings.isNullOrEmpty(name)) {
+      return null;
+    }
     if (name.startsWith("/")) {
       name = name.substring(1);
     }
@@ -109,6 +115,7 @@ public class ToolsCatalog {
     return Collections.unmodifiableSortedMap(toc);
   }
 
+  @Nullable
   private static byte[] read(String path) {
     String name = "root/" + path;
     try (InputStream in = ToolsCatalog.class.getResourceAsStream(name)) {
@@ -128,6 +135,7 @@ public class ToolsCatalog {
     }
   }
 
+  @Nullable
   private static String dirOf(String path) {
     final int s = path.lastIndexOf('/');
     return s < 0 ? null : path.substring(0, s);