Adapt SendMessage of the NoShell command to AsyncCommand type
Before this change, the SendMessage class inside the NoShell command implemented the Command interface. Therefore, the command was using instances of OutputStream and closing them at the end of the command. Given that the channel isn't closed at this stage, this produced a problem as apache-sshd calls the flush method of one of the output streams. Therefore, the command was hanging and didn't release the terminal until the user pressed a key. This change adapts the SendMessage class to an AyncCommand type. Therefore, the Apache library treats the command as AsyncCommand and uses the newly introduced I/O streams instead of normal streams. As a result, the problematic flush call of the output stream doesn't happen, because the normal output stream is not initialized, instead the I/O streams are initialized and used. An integration test is added to verify that the command doesn't hang indefinitely. It does so by setting a timeout to the command. Bug: Issue 11142 Change-Id: Ia6ed0d4ee264d2e901eaa17ea444bf715e3b44db
This commit is contained in:
committed by
David Ostrovsky
parent
df853d3e1e
commit
387a01d699
7
javatests/com/google/gerrit/integration/ssh/BUILD
Normal file
7
javatests/com/google/gerrit/integration/ssh/BUILD
Normal file
@@ -0,0 +1,7 @@
|
||||
load("//javatests/com/google/gerrit/acceptance:tests.bzl", "acceptance_tests")
|
||||
|
||||
acceptance_tests(
|
||||
srcs = ["NoShellIT.java"],
|
||||
group = "no-shell",
|
||||
labels = ["ssh-no-shell"],
|
||||
)
|
||||
96
javatests/com/google/gerrit/integration/ssh/NoShellIT.java
Normal file
96
javatests/com/google/gerrit/integration/ssh/NoShellIT.java
Normal file
@@ -0,0 +1,96 @@
|
||||
// Copyright (C) 2021 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.integration.ssh;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.gerrit.acceptance.GerritServer.TestSshServerAddress;
|
||||
import com.google.gerrit.acceptance.NoHttpd;
|
||||
import com.google.gerrit.acceptance.StandaloneSiteTest;
|
||||
import com.google.gerrit.acceptance.UseSsh;
|
||||
import com.google.gerrit.extensions.api.GerritApi;
|
||||
import com.google.inject.Inject;
|
||||
import java.io.IOException;
|
||||
import java.net.InetSocketAddress;
|
||||
import org.junit.Test;
|
||||
|
||||
@NoHttpd
|
||||
@UseSsh
|
||||
public class NoShellIT extends StandaloneSiteTest {
|
||||
private static final String[] SSH_KEYGEN_CMD =
|
||||
new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
|
||||
|
||||
@Inject private GerritApi gApi;
|
||||
@Inject private @TestSshServerAddress InetSocketAddress sshAddress;
|
||||
|
||||
private String identityPath;
|
||||
|
||||
@Test(timeout = 30000)
|
||||
public void verifyCommandsIsClosed() throws Exception {
|
||||
try (ServerContext ctx = startServer()) {
|
||||
setUpTestHarness(ctx);
|
||||
|
||||
IOException thrown = assertThrows(IOException.class, () -> execute(cmd()));
|
||||
assertThat(thrown)
|
||||
.hasMessageThat()
|
||||
.contains("Hi Administrator, you have successfully connected over SSH.");
|
||||
}
|
||||
}
|
||||
|
||||
private void setUpTestHarness(ServerContext ctx) throws Exception {
|
||||
ctx.getInjector().injectMembers(this);
|
||||
setUpAuthentication();
|
||||
identityPath = sitePaths.data_dir.resolve(String.format("id_rsa_%s", "admin")).toString();
|
||||
}
|
||||
|
||||
private void setUpAuthentication() throws Exception {
|
||||
execute(
|
||||
ImmutableList.<String>builder()
|
||||
.add(SSH_KEYGEN_CMD)
|
||||
.add(String.format("id_rsa_%s", "admin"))
|
||||
.build());
|
||||
gApi.accounts()
|
||||
.id("admin")
|
||||
.addSshKey(
|
||||
new String(
|
||||
java.nio.file.Files.readAllBytes(
|
||||
sitePaths.data_dir.resolve(String.format("id_rsa_%s.pub", "admin"))),
|
||||
UTF_8));
|
||||
}
|
||||
|
||||
private ImmutableList<String> cmd() {
|
||||
return ImmutableList.<String>builder()
|
||||
.add("ssh")
|
||||
.add("-tt")
|
||||
.add("-o")
|
||||
.add("StrictHostKeyChecking=no")
|
||||
.add("-o")
|
||||
.add("UserKnownHostsFile=/dev/null")
|
||||
.add("-p")
|
||||
.add(String.valueOf(sshAddress.getPort()))
|
||||
.add("admin@" + sshAddress.getHostName())
|
||||
.add("-i")
|
||||
.add(identityPath)
|
||||
.build();
|
||||
}
|
||||
|
||||
private String execute(ImmutableList<String> cmd) throws Exception {
|
||||
return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user