Merge branch 'stable-3.1'

* stable-3.1:
  Set version to 3.1.3-SNAPSHOT
  Set version to 3.1.2
  Set version to 3.0.7-SNAPSHOT
  Set version to 3.0.6
  Set version to 2.16.16-SNAPSHOT
  Set version to 2.16.15
  Revert "Revert "Fix handling of interactive/batch users in the QoS filter""
  Separate request context filter from cleanup
  Assert RequestCleanup ran only once
  Test Git/HTTP Servlet when SSH is enabled
  Only add the gr-app.js script at the end of the dom
  Remove preload on gr-app.js
  Fix linter errors under eslint 6.x
  Add example command for how to display note of an external ID
  Add example command for how to find SHA1 of an external ID
  Link global eslint packages to local project
  Revert "Fix handling of interactive/batch users in the QoS filter"
  Log also thread name in the sshd_log
  Log also thread name in the httpd_log

Change-Id: I8fe42761906571234fc401629a16071233212318

java/com/google/gerrit/httpd/RequestCleanupFilter.java

@@ -0,0 +1,65 @@
+// 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.httpd;
+
+import com.google.gerrit.server.RequestCleanup;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.servlet.ServletModule;
+import java.io.IOException;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+
+/** Executes any pending {@link RequestCleanup} at the end of a request. */
+@Singleton
+public class RequestCleanupFilter implements Filter {
+  public static Module module() {
+    return new ServletModule() {
+      @Override
+      protected void configureServlets() {
+        filter("/*").through(RequestCleanupFilter.class);
+      }
+    };
+  }
+
+  private final Provider<RequestCleanup> cleanup;
+
+  @Inject
+  RequestCleanupFilter(Provider<RequestCleanup> r) {
+    cleanup = r;
+  }
+
+  @Override
+  public void init(FilterConfig filterConfig) {}
+
+  @Override
+  public void destroy() {}
+
+  @Override
+  public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+      throws IOException, ServletException {
+    try {
+      chain.doFilter(request, response);
+    } finally {
+      cleanup.get().run();
+    }
+  }
+}

java/com/google/gerrit/httpd/RequestContextFilter.java

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.httpd;
 
-import com.google.gerrit.server.RequestCleanup;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.inject.Inject;
@@ -30,7 +29,7 @@ import javax.servlet.ServletException;
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 
-/** Executes any pending {@link RequestCleanup} at the end of a request. */
+/** Set the request context for the downstream filters and invocation. */
 @Singleton
 public class RequestContextFilter implements Filter {
   public static Module module() {
@@ -42,14 +41,11 @@ public class RequestContextFilter implements Filter {
     };
   }
 
-  private final Provider<RequestCleanup> cleanup;
   private final Provider<HttpRequestContext> requestContext;
   private final ThreadLocalRequestContext local;
 
   @Inject
-  RequestContextFilter(
-      Provider<RequestCleanup> r, Provider<HttpRequestContext> c, ThreadLocalRequestContext l) {
-    cleanup = r;
+  RequestContextFilter(Provider<HttpRequestContext> c, ThreadLocalRequestContext l) {
     requestContext = c;
     local = l;
   }
@@ -65,11 +61,7 @@ public class RequestContextFilter implements Filter {
       throws IOException, ServletException {
     RequestContext old = local.setContext(requestContext.get());
     try {
-      try {
-        chain.doFilter(request, response);
-      } finally {
-        cleanup.get().run();
-      }
+      chain.doFilter(request, response);
     } finally {
       local.setContext(old);
     }

java/com/google/gerrit/httpd/init/WebAppInitializer.java

@@ -27,6 +27,7 @@ import com.google.gerrit.httpd.GetUserFilter;
 import com.google.gerrit.httpd.GitOverHttpModule;
 import com.google.gerrit.httpd.H2CacheBasedWebSession;
 import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.RequestCleanupFilter;
 import com.google.gerrit.httpd.RequestContextFilter;
 import com.google.gerrit.httpd.RequestMetricsFilter;
 import com.google.gerrit.httpd.RequireSslFilter;
@@ -383,6 +384,7 @@ public class WebAppInitializer extends GuiceServletContextListener implements Fi
     modules.add(RequestMetricsFilter.module());
     modules.add(sysInjector.getInstance(GerritAuthModule.class));
     modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+    modules.add(RequestCleanupFilter.module());
     modules.add(AllRequestFilter.module());
     modules.add(sysInjector.getInstance(WebModule.class));
     modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));

java/com/google/gerrit/pgm/Daemon.java

@@ -31,6 +31,7 @@ import com.google.gerrit.httpd.GetUserFilter;
 import com.google.gerrit.httpd.GitOverHttpModule;
 import com.google.gerrit.httpd.H2CacheBasedWebSession;
 import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.RequestCleanupFilter;
 import com.google.gerrit.httpd.RequestContextFilter;
 import com.google.gerrit.httpd.RequestMetricsFilter;
 import com.google.gerrit.httpd.RequireSslFilter;
@@ -546,6 +547,7 @@ public class Daemon extends SiteProgram {
     if (sshd) {
       modules.add(new ProjectQoSFilter.Module());
     }
+    modules.add(RequestCleanupFilter.module());
     modules.add(AllRequestFilter.module());
     modules.add(sysInjector.getInstance(WebModule.class));
     modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));

java/com/google/gerrit/pgm/http/jetty/HttpLog.java

@@ -73,7 +73,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
             TimeUtil.nowMs(), // when
             Level.INFO, // level
             "", // message text
-            "HTTPD", // thread name
+            Thread.currentThread().getName(), // thread name
             null, // exception information
             null, // current NDC string
             null, // caller location

java/com/google/gerrit/pgm/http/jetty/HttpLogLayout.java

@@ -40,6 +40,11 @@ public final class HttpLogLayout extends Layout {
 
     opt(buf, event, HttpLog.P_HOST);
 
+    buf.append(' ');
+    buf.append('[');
+    buf.append(event.getThreadName());
+    buf.append(']');
+
     buf.append(' ');
     buf.append('-'); // identd on client system (never requested)
 

java/com/google/gerrit/server/RequestCleanup.java

@@ -31,9 +31,7 @@ public class RequestCleanup implements Runnable {
   /** Register a task to be completed after the request ends. */
   public void add(Runnable task) {
     synchronized (cleanup) {
-      if (ran) {
-        throw new IllegalStateException("Request has already been cleaned up");
-      }
+      assertNotRan();
       cleanup.add(task);
     }
   }
@@ -41,6 +39,7 @@ public class RequestCleanup implements Runnable {
   @Override
   public void run() {
     synchronized (cleanup) {
+      assertNotRan();
       ran = true;
       for (Iterator<Runnable> i = cleanup.iterator(); i.hasNext(); ) {
         try {
@@ -52,4 +51,10 @@ public class RequestCleanup implements Runnable {
       }
     }
   }
+
+  private void assertNotRan() {
+    if (ran) {
+      throw new IllegalStateException("Request has already been cleaned up");
+    }
+  }
 }

java/com/google/gerrit/sshd/SshLog.java

@@ -250,7 +250,7 @@ class SshLog implements LifecycleListener, GerritConfigListener {
             TimeUtil.nowMs(), // when
             Level.INFO, // level
             msg, // message text
-            "SSHD", // thread name
+            Thread.currentThread().getName(), // thread name
             null, // exception information
             null, // current NDC string
             null, // caller location

java/com/google/gerrit/sshd/SshLogLayout.java

@@ -54,6 +54,12 @@ public final class SshLogLayout extends Layout {
     buf.append(']');
 
     req(P_SESSION, buf, event);
+
+    buf.append(' ');
+    buf.append('[');
+    buf.append(event.getThreadName());
+    buf.append(']');
+
     req(P_USER_NAME, buf, event);
     req(P_ACCOUNT_ID, buf, event);
 

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

@@ -0,0 +1,99 @@
+// 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 com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.FakeGroupAuditService;
+import com.google.gerrit.acceptance.Sandboxed;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.audit.HttpAuditEvent;
+import com.google.inject.Inject;
+import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.junit.Before;
+import org.junit.Test;
+
+public class AbstractGitOverHttpServlet extends AbstractPushForReview {
+  @Inject protected FakeGroupAuditService auditService;
+
+  @Before
+  public void beforeEach() throws Exception {
+    CredentialsProvider.setDefault(
+        new UsernamePasswordCredentialsProvider(admin.username(), admin.httpPassword()));
+    selectProtocol(AbstractPushForReview.Protocol.HTTP);
+    // Don't clear audit events here, since we can't guarantee all test setup has run yet.
+  }
+
+  @Test
+  @Sandboxed
+  public void receivePackAuditEventLog() throws Exception {
+    auditService.drainHttpAuditEvents();
+    testRepo
+        .git()
+        .push()
+        .setRemote("origin")
+        .setRefSpecs(new RefSpec("HEAD:refs/for/master"))
+        .call();
+
+    ImmutableList<HttpAuditEvent> auditEvents = auditService.drainHttpAuditEvents();
+    assertThat(auditEvents).hasSize(2);
+
+    HttpAuditEvent lsRemote = auditEvents.get(0);
+    assertThat(lsRemote.who.getAccountId()).isEqualTo(admin.id());
+    assertThat(lsRemote.what).endsWith("/info/refs?service=git-receive-pack");
+    assertThat(lsRemote.params).containsExactly("service", "git-receive-pack");
+    assertThat(lsRemote.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+
+    HttpAuditEvent receivePack = auditEvents.get(1);
+    assertThat(receivePack.who.getAccountId()).isEqualTo(admin.id());
+    assertThat(receivePack.what).endsWith("/git-receive-pack");
+    assertThat(receivePack.params).isEmpty();
+    assertThat(receivePack.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+  }
+
+  @Test
+  @Sandboxed
+  public void uploadPackAuditEventLog() throws Exception {
+    auditService.drainHttpAuditEvents();
+    // testRepo is already a clone. Make a server-side change so we have something to fetch.
+    try (Repository repo = repoManager.openRepository(project);
+        TestRepository<?> testRepo = new TestRepository<>(repo)) {
+      testRepo.branch("master").commit().create();
+    }
+    testRepo.git().fetch().call();
+
+    ImmutableList<HttpAuditEvent> auditEvents = auditService.drainHttpAuditEvents();
+    assertThat(auditEvents).hasSize(2);
+
+    HttpAuditEvent lsRemote = auditEvents.get(0);
+    // Repo URL doesn't include /a, so fetching doesn't cause authentication.
+    assertThat(lsRemote.who).isInstanceOf(AnonymousUser.class);
+    assertThat(lsRemote.what).endsWith("/info/refs?service=git-upload-pack");
+    assertThat(lsRemote.params).containsExactly("service", "git-upload-pack");
+    assertThat(lsRemote.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+
+    HttpAuditEvent uploadPack = auditEvents.get(1);
+    assertThat(lsRemote.who).isInstanceOf(AnonymousUser.class);
+    assertThat(uploadPack.what).endsWith("/git-upload-pack");
+    assertThat(uploadPack.params).isEmpty();
+    assertThat(uploadPack.httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+  }
+}

javatests/com/google/gerrit/acceptance/git/BUILD

@@ -17,7 +17,7 @@ load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
 java_library(
     name = "push_for_review",
     testonly = True,
-    srcs = ["AbstractPushForReview.java"],
+    srcs = glob(["Abstract*.java"]),
     deps = [
         "//java/com/google/gerrit/acceptance:lib",
         "//java/com/google/gerrit/git",

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

@@ -0,0 +1,17 @@
+// Copyright (C) 2018 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;
+
+public class GitOverHttpServletIT extends AbstractGitOverHttpServlet {}

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

@@ -0,0 +1,20 @@
+// 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 com.google.gerrit.acceptance.UseSsh;
+
+@UseSsh
+public class GitOverHttpServletWithSshdEnabledIT extends AbstractGitOverHttpServlet {}

polygerrit-ui/app/.eslintrc.json

@@ -69,6 +69,9 @@
     "no-undef": "off",
     "no-useless-escape": "off",
     "no-var": "error",
+    "no-prototype-builtins": "off",
+    "no-redeclare": "off",
+    "operator-linebreak": "off",
     "object-shorthand": ["error", "always"],
     "padding-line-between-statements": [
       "error",

polygerrit-ui/app/lint_test.sh

@@ -2,15 +2,15 @@
 
 set -ex
 
-eslint_bin=$(which npm)
-if [ -z "$eslint_bin" ]; then
+npm_bin=$(which npm) && true
+if [ -z "$npm_bin" ]; then
     echo "NPM must be on the path."
     exit 1
 fi
 
-eslint_bin=$(which eslint)
-eslint_config=$(npm list -g | grep -c eslint-config-google)
-eslint_plugin=$(npm list -g | grep -c eslint-plugin-html)
+eslint_bin=$(which eslint) && true
+eslint_config=$(npm list -g | grep -c eslint-config-google) && true
+eslint_plugin=$(npm list -g | grep -c eslint-plugin-html) && true
 if [ -z "$eslint_bin" ] || [ "$eslint_config" -eq "0" ] || [ "$eslint_plugin" -eq "0" ]; then
     echo "You must install ESLint and its dependencies from NPM."
     echo "> npm install -g eslint eslint-config-google eslint-plugin-html"
@@ -19,4 +19,15 @@ if [ -z "$eslint_bin" ] || [ "$eslint_config" -eq "0" ] || [ "$eslint_plugin" -e
     exit 1
 fi
 
-${eslint_bin} --ext .html,.js .
+# get absolute path to lint_test.sh path
+SCRIPT=$(readlink -f "$0")
+UI_PATH=$(dirname "$SCRIPT")
+
+# To make sure npm link happens in the right place
+cd ${UI_PATH}
+
+# Linking global eslint packages to the local project. Required to use eslint plugins with a global
+# eslint installation.
+npm link eslint eslint-config-google eslint-plugin-html
+
+${eslint_bin} -c ${UI_PATH}/.eslintrc.json --ignore-pattern 'node_modules/' --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js ${UI_PATH}

polygerrit-ui/app/template_test.sh

@@ -8,13 +8,13 @@ exit 0
 
 set -ex
 
-node_bin=$(which node)
+node_bin=$(which node) && true
 if [ -z "$node_bin" ]; then
     echo "node must be on the path."
     exit 1
 fi
 
-npm_bin=$(which npm)
+npm_bin=$(which npm) && true
 if [[ -z "$npm_bin" ]]; then
     echo "NPM must be on the path. (https://www.npmjs.com/)"
     exit 1

tools/bzl/js.bzl

@@ -371,6 +371,7 @@ def _bundle_impl(ctx):
             "python",
             ctx.file._run_npm.path,
             ctx.file._crisper_archive.path,
+            "--script-in-head=false",
             "--always-write-script",
             "--source",
             bundled.path,