From ca94ab19fb1b0f7626a83c8ade6bdff5be4dad82 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 12 Oct 2018 09:07:02 +0900 Subject: [PATCH 1/3] Set version to 2.14.16-SNAPSHOT Change-Id: Ie0fb7cde41981e8689cb76ee13f1ed0a8bf7e364 --- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- version.bzl | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index dbc5419c37..e65c3ebcb2 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.14.15 + 2.14.16-SNAPSHOT jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 32d6f38d02..f50dd1d160 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.14.15 + 2.14.16-SNAPSHOT jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index f28c1e4732..1990f90474 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.14.15 + 2.14.16-SNAPSHOT jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index cf8d8c0004..9f46700830 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.14.15 + 2.14.16-SNAPSHOT jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index f945b0c244..3318e9c35c 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.14.15 + 2.14.16-SNAPSHOT war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index 2a4d83edb9..a2ada9b8ab 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "2.14.15" +GERRIT_VERSION = "2.14.16-SNAPSHOT" From bdcd60240fc50329e546a94fe518213d3327a4b5 Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Thu, 11 Oct 2018 23:14:05 +0000 Subject: [PATCH 2/3] Revert "Fix access path propagation on git/ssh protocol" This reverts commit f6e7913d143b4e35c6d17ec9b592d9820a7132d8. Reason for revert: propagating the Context across threads created DB leaks. Bug: Issue 9836 Change-Id: I439ea0c4c6ea24346f0bbd3a721bdd760844b110 --- .../gerrit/sshd/AbstractGitCommand.java | 40 ++++++++------ .../com/google/gerrit/sshd/BaseCommand.java | 54 +++++-------------- 2 files changed, 37 insertions(+), 57 deletions(-) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java index b13a4d2597..4144ed2d8b 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java @@ -31,6 +31,8 @@ public abstract class AbstractGitCommand extends BaseCommand { @Argument(index = 0, metaVar = "PROJECT.git", required = true, usage = "project name") protected ProjectControl projectControl; + @Inject private SshScope sshScope; + @Inject private GitRepositoryManager repoManager; @Inject private SshSession session; @@ -47,25 +49,29 @@ public abstract class AbstractGitCommand extends BaseCommand { @Override public void start(final Environment env) { Context ctx = context.subContext(newSession(), context.getCommandLine()); - startThreadWithContext( - ctx, - new ProjectCommandRunnable() { - @Override - public void executeParseCommand() throws Exception { - parseCommandLine(); - } + final Context old = sshScope.set(ctx); + try { + startThread( + new ProjectCommandRunnable() { + @Override + public void executeParseCommand() throws Exception { + parseCommandLine(); + } - @Override - public void run() throws Exception { - AbstractGitCommand.this.service(); - } + @Override + public void run() throws Exception { + AbstractGitCommand.this.service(); + } - @Override - public Project.NameKey getProjectName() { - Project project = projectControl.getProjectState().getProject(); - return project.getNameKey(); - } - }); + @Override + public Project.NameKey getProjectName() { + Project project = projectControl.getProjectState().getProject(); + return project.getNameKey(); + } + }); + } finally { + sshScope.set(old); + } } private SshSession newSession() { diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java index 1f030de969..c83bcc7683 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java @@ -43,7 +43,6 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.StringWriter; import java.nio.charset.Charset; -import java.util.Optional; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicReference; import org.apache.sshd.common.SshException; @@ -268,38 +267,6 @@ public abstract class BaseCommand implements Command { }); } - /** - * Spawn a function into its own thread with the provided context. - * - *

Typically this should be invoked within {@link Command#start(Environment)}, such as: - * - * 

-   * startThreadWithContext(SshScope.Context context, new CommandRunnable() {
-   *   public void run() throws Exception {
-   *     runImp();
-   *   }
-   * });
-   *
- * - *

If the function throws an exception, it is translated to a simple message for the client, a - * non-zero exit code, and the stack trace is logged. - * - * @param thunk the runnable to execute on the thread, performing the command's logic. - */ - protected void startThreadWithContext(SshScope.Context context, final CommandRunnable thunk) { - final TaskThunk tt = new TaskThunk(thunk, Optional.ofNullable(context)); - - if (isAdminHighPriorityCommand() && user.getCapabilities().canAdministrateServer()) { - // Admin commands should not block the main work threads (there - // might be an interactive shell there), nor should they wait - // for the main work threads. - // - new Thread(tt, tt.toString()).start(); - } else { - task.set(executor.submit(tt)); - } - } - /** * Spawn a function into its own thread. * @@ -319,7 +286,17 @@ public abstract class BaseCommand implements Command { * @param thunk the runnable to execute on the thread, performing the command's logic. */ protected void startThread(final CommandRunnable thunk) { - startThreadWithContext(null, thunk); + final TaskThunk tt = new TaskThunk(thunk); + + if (isAdminHighPriorityCommand() && user.getCapabilities().canAdministrateServer()) { + // Admin commands should not block the main work threads (there + // might be an interactive shell there), nor should they wait + // for the main work threads. + // + new Thread(tt, tt.toString()).start(); + } else { + task.set(executor.submit(tt)); + } } private boolean isAdminHighPriorityCommand() { @@ -436,21 +413,18 @@ public abstract class BaseCommand implements Command { private final class TaskThunk implements CancelableRunnable, ProjectRunnable { private final CommandRunnable thunk; - private final Context taskContext; private final String taskName; - private Project.NameKey projectName; - private TaskThunk(final CommandRunnable thunk, Optional oneOffContext) { + private TaskThunk(final CommandRunnable thunk) { this.thunk = thunk; this.taskName = getTaskName(); - this.taskContext = oneOffContext.orElse(context); } @Override public void cancel() { synchronized (this) { - final Context old = sshScope.set(taskContext); + final Context old = sshScope.set(context); try { onExit(STATUS_CANCEL); } finally { @@ -465,7 +439,7 @@ public abstract class BaseCommand implements Command { final Thread thisThread = Thread.currentThread(); final String thisName = thisThread.getName(); int rc = 0; - final Context old = sshScope.set(taskContext); + final Context old = sshScope.set(context); try { context.started = TimeUtil.nowMs(); thisThread.setName("SSH " + taskName); From 17e14e8476270f9ee900b94b3619503ad2a82cad Mon Sep 17 00:00:00 2001 From: Luca Milanesio Date: Fri, 12 Oct 2018 00:27:50 +0100 Subject: [PATCH 3/3] Propagate access path for SSH commands Pass the right access path for the commands executed in Git because of an incoming SSH connection: - GIT for all Git/SSH commands - SSH_COMMAND for everything else Allow to honour force push ACL (and potentially other operations) because of the correct propagation of the original identity with the right access path. Previously the code was setting the access path on SshScope.Context which is stored in the ThreadLocal context of the receiving SSH command, which is different from the other thread that execute the action. Bug: Issue 9823 Change-Id: I2b9e1369e53a000457d4571ecf5e6d7ddf843827 --- .../google/gerrit/sshd/AbstractGitCommand.java | 4 ++-- .../com/google/gerrit/sshd/BaseCommand.java | 17 ++++++++++++----- .../java/com/google/gerrit/sshd/SshCommand.java | 4 +++- .../google/gerrit/sshd/commands/ScpCommand.java | 4 +++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java index 4144ed2d8b..cf76dcb3df 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java @@ -68,7 +68,8 @@ public abstract class AbstractGitCommand extends BaseCommand { Project project = projectControl.getProjectState().getProject(); return project.getNameKey(); } - }); + }, + AccessPath.GIT); } finally { sshScope.set(old); } @@ -80,7 +81,6 @@ public abstract class AbstractGitCommand extends BaseCommand { session, session.getRemoteAddress(), userFactory.create(session.getRemoteAddress(), user.getAccountId())); - n.setAccessPath(AccessPath.GIT); return n; } diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java index c83bcc7683..7092fb0480 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/BaseCommand.java @@ -22,6 +22,7 @@ import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.AccessPath; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.RequestCleanup; @@ -256,15 +257,17 @@ public abstract class BaseCommand implements Command { * * * @param thunk the runnable to execute on the thread, performing the command's logic. + * @param accessPath the path used by the end user for running the SSH command */ - protected void startThread(final Runnable thunk) { + protected void startThread(final Runnable thunk, AccessPath accessPath) { startThread( new CommandRunnable() { @Override public void run() throws Exception { thunk.run(); } - }); + }, + accessPath); } /** @@ -284,9 +287,10 @@ public abstract class BaseCommand implements Command { * non-zero exit code, and the stack trace is logged. * * @param thunk the runnable to execute on the thread, performing the command's logic. + * @param accessPath the path used by the end user for running the SSH command */ - protected void startThread(final CommandRunnable thunk) { - final TaskThunk tt = new TaskThunk(thunk); + protected void startThread(final CommandRunnable thunk, AccessPath accessPath) { + final TaskThunk tt = new TaskThunk(thunk, accessPath); if (isAdminHighPriorityCommand() && user.getCapabilities().canAdministrateServer()) { // Admin commands should not block the main work threads (there @@ -414,11 +418,13 @@ public abstract class BaseCommand implements Command { private final class TaskThunk implements CancelableRunnable, ProjectRunnable { private final CommandRunnable thunk; private final String taskName; + private final AccessPath accessPath; private Project.NameKey projectName; - private TaskThunk(final CommandRunnable thunk) { + private TaskThunk(final CommandRunnable thunk, AccessPath accessPath) { this.thunk = thunk; this.taskName = getTaskName(); + this.accessPath = accessPath; } @Override @@ -439,6 +445,7 @@ public abstract class BaseCommand implements Command { final Thread thisThread = Thread.currentThread(); final String thisName = thisThread.getName(); int rc = 0; + context.getSession().setAccessPath(accessPath); final Context old = sshScope.set(context); try { context.started = TimeUtil.nowMs(); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCommand.java index 3e42ebee09..b4e44d3564 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/SshCommand.java @@ -14,6 +14,7 @@ package com.google.gerrit.sshd; +import com.google.gerrit.server.AccessPath; import java.io.IOException; import java.io.PrintWriter; import org.apache.sshd.server.Environment; @@ -38,7 +39,8 @@ public abstract class SshCommand extends BaseCommand { stderr.flush(); } } - }); + }, + AccessPath.SSH_COMMAND); } protected abstract void run() throws UnloggedFailure, Failure, Exception; diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ScpCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ScpCommand.java index 1ed0bb0425..ff45d753b1 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ScpCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ScpCommand.java @@ -24,6 +24,7 @@ package com.google.gerrit.sshd.commands; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.gerrit.server.AccessPath; import com.google.gerrit.server.tools.ToolsCatalog; import com.google.gerrit.server.tools.ToolsCatalog.Entry; import com.google.gerrit.sshd.BaseCommand; @@ -88,7 +89,8 @@ final class ScpCommand extends BaseCommand { public void run() { runImp(); } - }); + }, + AccessPath.SSH_COMMAND); } private void runImp() {