Merge branch 'stable-3.1' into stable-3.2

* stable-3.1:
  Set version to 3.1.8-SNAPSHOT
  Set version to 3.1.7
  Use Mockito instead of EasyMock for X-Frame-Options header tests
  Set version to 3.0.12-SNAPSHOT
  Set version to 3.0.11
  Set X-Frame-Options header to avoid clickjacking
  PG: Skip unsupported global capabilities
  Revert "Remove documentation of obsolete gerrit.canLoadInIFrame"
  Fix typos in note-db.txt
  Document skipping of reindexing step for offline NoteDB migration
  Report end of NoteDB migration when skipping reindexing
  Clarify that index.batchThreads is relevant for offline reindexing
  Add project to output when reindexing changes in verbose mode
  Auto-flush SiteIndexer's PrintWriters
  Allow to re-index in verbose mode during NoteDB migration
  Avoid closing System.out after All-Users GC in NoteDB migration
  Honor project watches also for changes created via cherry-pick
  Report the index state after re-indexing

Change-Id: I6038b4d47997d44719923376221f4cc75b24d693

10 files changed, 244 insertions, 6 deletions

diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index ccaca6a3ac..4045ad9960 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -2208,6 +2208,32 @@ file containing the class must be placed in the `$site_path/lib` folder.
 +
 If not specified, the default no-op implementation is used.
 
+[[gerrit.canLoadInIFrame]]gerrit.canLoadInIFrame::
++
+For security reasons Gerrit will always jump out of iframe.
+Setting this option to true will prevent this behavior.
++
+By default false.
+
+[[gerrit.xframeOption]]gerrit.xframeOption::
++
+Add link:https://tools.ietf.org/html/rfc7034[`X-Frame-Options`] header to all HTTP
+responses. The `X-Frame-Options` HTTP response header can be used to indicate
+whether or not a browser should be allowed to render a page in a
+`<frame>`, `<iframe>`, `<embed>` or `<object>`.
++
+Available values:
++
+1. ALLOW - The page can be displayed in a frame.
+2. SAMEORIGIN - The page can only be displayed in a frame on the same origin as the page itself.
++
+If link:#gerrit.canLoadInIFrame is set to false this option is ignored and the
+`X-Frame-Options` header is always set to `DENY`.
+Setting this option to `ALLOW` will cause the `X-Frame-Options` header to be omitted
+the the page can be displayed in a frame.
++
+By default SAMEORIGIN.
+
 [[gerrit.cdnPath]]gerrit.cdnPath::
 +
 Path prefix for PolyGerrit's static resources if using a CDN.
@@ -2942,7 +2968,7 @@ by the JVM. If set to a negative value, defaults to a direct executor.
 [[index.batchThreads]]index.batchThreads::
 +
 Number of threads to use for indexing in background operations, such as
-online schema upgrades.
+online schema upgrades, and also for offline reindexing.
 +
 If not set or set to a zero, defaults to the number of logical CPUs as returned
 by the JVM. If set to a negative value, defaults to a direct executor.
diff --git a/Documentation/note-db.txt b/Documentation/note-db.txt
index 0505dd209d..89758a092c 100644
--- a/Documentation/note-db.txt
+++ b/Documentation/note-db.txt
@@ -110,6 +110,13 @@ Migration requires a heap size comparable to running a Gerrit server. If you
 normally run `gerrit.war daemon` with an `-Xmx` flag, pass that to the migration
 tool as well.
 
+[NOTE]
+Note that by appending `--reindex false` to the above command, you can skip the
+lengthy, implicit reindexing step of the migration. This is useful if you plan
+to perform further Gerrit upgrades while the server is offline and have to
+reindex later anyway (E.g.: a follow-up upgrade to Gerrit 3.2 or newer, which
+requires to reindex changes anyway).
+
 *Advantages*
 
 * Much faster than online; can use all available CPUs, since no live traffic
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0badced11b..da545d0eee 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6186,7 +6186,7 @@ Number of the parent relative to which the cherry-pick should be considered.
 Notify handling that defines to whom email notifications should be sent
 after the cherry-pick. +
 Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
-If not set, the default is `NONE`.
+If not set, the default is `ALL`.
 |`notify_details`   |optional|
 Additional information about whom to notify about the update as a map
 of recipient type to link:#notify-info[NotifyInfo] entity.
diff --git a/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java b/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
index 69c1790979..fb03bc5cc3 100644
--- a/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/CherryPickInput.java
@@ -24,7 +24,7 @@ public class CherryPickInput {
   public String base;
   public Integer parent;
 
-  public NotifyHandling notify = NotifyHandling.NONE;
+  public NotifyHandling notify = NotifyHandling.ALL;
   public Map<RecipientType, NotifyInfo> notifyDetails;
 
   public boolean keepReviewers;
diff --git a/java/com/google/gerrit/httpd/AllRequestFilter.java b/java/com/google/gerrit/httpd/AllRequestFilter.java
index 9d171d5a54..1c3cafea04 100644
--- a/java/com/google/gerrit/httpd/AllRequestFilter.java
+++ b/java/com/google/gerrit/httpd/AllRequestFilter.java
@@ -18,6 +18,8 @@ import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.server.plugins.Plugin;
 import com.google.gerrit.server.plugins.StopPluginListener;
 import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Scopes;
 import com.google.inject.Singleton;
 import com.google.inject.internal.UniqueAnnotations;
 import com.google.inject.servlet.ServletModule;
@@ -32,11 +34,15 @@ import javax.servlet.ServletResponse;
 
 /** Filters all HTTP requests passing through the server. */
 public abstract class AllRequestFilter implements Filter {
-  public static ServletModule module() {
+  public static Module module() {
     return new ServletModule() {
       @Override
       protected void configureServlets() {
         DynamicSet.setOf(binder(), AllRequestFilter.class);
+        DynamicSet.bind(binder(), AllRequestFilter.class)
+            .to(AllowRenderInFrameFilter.class)
+            .in(Scopes.SINGLETON);
+
         filter("/*").through(FilterProxy.class);
 
         bind(StopPluginListener.class)
diff --git a/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java b/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java
new file mode 100644
index 0000000000..b414aad7c1
--- /dev/null
+++ b/java/com/google/gerrit/httpd/AllowRenderInFrameFilter.java
@@ -0,0 +1,59 @@
+// Copyright (C) 2020 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.httpd;
+
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.inject.Inject;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
+
+public class AllowRenderInFrameFilter extends AllRequestFilter {
+  static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options";
+
+  public static enum XFrameOption {
+    ALLOW,
+    SAMEORIGIN;
+  }
+
+  private final String xframeOptionString;
+  private final boolean skipXFrameOption;
+
+  @Inject
+  public AllowRenderInFrameFilter(@GerritServerConfig Config cfg) {
+    XFrameOption xframeOption =
+        cfg.getEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+    boolean canLoadInIFrame = cfg.getBoolean("gerrit", "canLoadInIFrame", false);
+    xframeOptionString = canLoadInIFrame ? xframeOption.name() : "DENY";
+
+    skipXFrameOption = xframeOption.equals(XFrameOption.ALLOW) && canLoadInIFrame;
+  }
+
+  @Override
+  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+      throws IOException, ServletException {
+    if (skipXFrameOption) {
+      chain.doFilter(request, response);
+    } else {
+      HttpServletResponse httpResponse = (HttpServletResponse) response;
+      httpResponse.addHeader(X_FRAME_OPTIONS_HEADER_NAME, xframeOptionString);
+      chain.doFilter(request, httpResponse);
+    }
+  }
+}
diff --git a/java/com/google/gerrit/index/SiteIndexer.java b/java/com/google/gerrit/index/SiteIndexer.java
index ecfc7bdf9f..32b4b21b99 100644
--- a/java/com/google/gerrit/index/SiteIndexer.java
+++ b/java/com/google/gerrit/index/SiteIndexer.java
@@ -83,7 +83,7 @@ public abstract class SiteIndexer<K, V, I extends Index<K, V>> {
   }
 
   protected PrintWriter newPrintWriter(OutputStream out) {
-    return new PrintWriter(new OutputStreamWriter(out, UTF_8));
+    return new PrintWriter(new OutputStreamWriter(out, UTF_8), true);
   }
 
   private static class ErrorListener implements Runnable {
diff --git a/java/com/google/gerrit/pgm/Reindex.java b/java/com/google/gerrit/pgm/Reindex.java
index 2e526bb454..966801f382 100644
--- a/java/com/google/gerrit/pgm/Reindex.java
+++ b/java/com/google/gerrit/pgm/Reindex.java
@@ -202,6 +202,9 @@ public class Reindex extends SiteProgram {
     if (result.success()) {
       index.markReady(true);
     }
+    System.out.format(
+        "Index %s in version %d is %sready\n",
+        def.getName(), index.getSchema().getVersion(), result.success() ? "" : "NOT ");
     return result.success();
   }
 }
diff --git a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
index 2a8a5baff2..3c40c4fd45 100644
--- a/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
+++ b/java/com/google/gerrit/server/index/change/AllChangesIndexer.java
@@ -244,7 +244,8 @@ public class AllChangesIndexer extends SiteIndexer<Change.Id, ChangeData, Change
       try {
         indexer.index(changeDataFactory.create(r.notes()));
         done.update(1);
-        verboseWriter.println("Reindexed change " + r.id());
+        verboseWriter.format(
+            "Reindexed change %d (project: %s)\n", r.id().get(), r.notes().getProjectName().get());
       } catch (RejectedExecutionException e) {
         // Server shutdown, don't spam the logs.
         failSilently();
diff --git a/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java b/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java
new file mode 100644
index 0000000000..26798290c0
--- /dev/null
+++ b/javatests/com/google/gerrit/httpd/AllowRenderInFrameFilterTest.java
@@ -0,0 +1,136 @@
+// Copyright (C) 2020 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.httpd;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.AllowRenderInFrameFilter.X_FRAME_OPTIONS_HEADER_NAME;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+import com.google.gerrit.httpd.AllowRenderInFrameFilter.XFrameOption;
+import java.io.IOException;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class AllowRenderInFrameFilterTest {
+
+  private Config cfg = new Config();
+  @Mock ServletRequest request;
+  @Mock HttpServletResponse response;
+  @Mock FilterChain filterChain;
+
+  @Test
+  public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalse()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+  }
+
+  @Test
+  public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalseAndXFormOptionIsSAMEORIGIN()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+    cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+  }
+
+  @Test
+  public void shouldDenyInFrameRenderingWhenCanRenderInFrameIsFalseAndXFormOptionIsALLOW()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", false);
+    cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.ALLOW);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "DENY");
+  }
+
+  @Test
+  public void shouldRestrictAccessToSAMEORIGINWhenCanRenderInFrameIsTrue()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+  }
+
+  @Test
+  public void shouldSkipHeaderWhenCanRenderInFrameIsTrueAndXFormOptionIsALLOW()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+    cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.ALLOW);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, never()).addHeader(eq(X_FRAME_OPTIONS_HEADER_NAME), anyString());
+  }
+
+  @Test
+  public void shouldRestrictAccessToSAMEORIGINWhenCanRenderInFrameIsTrueAndXFormOptionIsSAMEORIGIN()
+      throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+    cfg.setEnum("gerrit", null, "xframeOption", XFrameOption.SAMEORIGIN);
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+  }
+
+  @Test
+  public void shouldIgnoreXFrameOriginCaseSensitivity() throws IOException, ServletException {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+    cfg.setString("gerrit", null, "xframeOption", "sameOrigin");
+
+    AllowRenderInFrameFilter objectUnderTest = new AllowRenderInFrameFilter(cfg);
+    objectUnderTest.doFilter(request, response, filterChain);
+
+    verify(response, times(1)).addHeader(X_FRAME_OPTIONS_HEADER_NAME, "SAMEORIGIN");
+  }
+
+  @Test
+  public void shouldThrowExceptionWhenUnknownXFormOptionValue() {
+    cfg.setBoolean("gerrit", null, "canLoadInIFrame", true);
+    cfg.setString("gerrit", null, "xframeOption", "unsupported value");
+
+    IllegalArgumentException e =
+        assertThrows(IllegalArgumentException.class, () -> new AllowRenderInFrameFilter(cfg));
+    assertThat(e).hasMessageThat().contains("gerrit.xframeOption=unsupported value");
+  }
+}