From d3fb91977ce804047bac9fce578b2da0a7ed2f01 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 13:26:46 +0200 Subject: [PATCH] Revive the embedded ssh server tests and run them in CI The -Dssh-tests suite has been dead for years. Three separate faults kept the tests that use the embedded Apache MINA sshd from passing: * plexus-interactivity-api stopped shipping META-INF/plexus/components.xml in 1.3, so plexus-container-default - which is what PlexusTestCase runs - could no longer see a Prompter, and every lookup of a jsch Wagon failed because ConsoleInteractiveUserInfo and PrompterUIKeyboardInteractive both require one. Supply a Prompter for the tests. * ShellCommand flushed the channel's output stream after calling ExitCallback.onExit, which closes it. The resulting SshChannelClosedException tore down the session, so the command after every executeCommand failed with "session is down". * The expected modification time on a get was compared at millisecond precision, but the scp "T" header carries whole seconds. Also register the SFTP subsystem on the embedded server: OpenSSH 9 and later drive scp over SFTP, so wagon-ssh-external's tests, which shell out to the host's scp, could not connect at all. ScpWagon now maps a missing file to ResourceDoesNotExistException when the server reports it with the scp fatal-error code (2) and says so in the message, not only with the warning code (1) that OpenSSH uses. CI passes -Dssh-tests -Dssh-embedded=true, which runs the 42 embedded jsch tests. The rest of the suite needs an sshd on localhost:22 and the developer's own account there, and stays excluded. --- .github/workflows/maven-verify.yml | 4 ++ .../ssh/AbstractEmbeddedScpWagonTest.java | 5 +- .../AbstractEmbeddedScpWagonWithKeyTest.java | 5 +- .../wagon/providers/ssh/ShellCommand.java | 6 -- .../providers/ssh/SshServerEmbedded.java | 6 ++ wagon-providers/wagon-ssh-external/pom.xml | 6 ++ wagon-providers/wagon-ssh/pom.xml | 8 ++- .../wagon/providers/ssh/jsch/ScpWagon.java | 17 ++++- .../providers/ssh/jsch/TestPrompter.java | 68 +++++++++++++++++++ .../resources/META-INF/plexus/components.xml | 35 ++++++++++ 10 files changed, 148 insertions(+), 12 deletions(-) create mode 100644 wagon-providers/wagon-ssh/src/test/java/org/apache/maven/wagon/providers/ssh/jsch/TestPrompter.java create mode 100644 wagon-providers/wagon-ssh/src/test/resources/META-INF/plexus/components.xml diff --git a/.github/workflows/maven-verify.yml b/.github/workflows/maven-verify.yml index 73743fe94..1b67b7f65 100644 --- a/.github/workflows/maven-verify.yml +++ b/.github/workflows/maven-verify.yml @@ -28,3 +28,7 @@ jobs: with: install-subversion: true matrix-exclude: '[ {"jdk": "25"} ]' + # -Dssh-tests turns off the no-ssh-tests profiles, -Dssh-embedded=true then narrows the ssh providers down + # to the tests that run against the embedded Apache MINA sshd; the rest need a real sshd on localhost:22. + # Repeats the workflow's own default for maven-args, which this input replaces rather than extends. + maven-args: '-D"invoker.streamLogsOnFailures" -Dssh-tests -Dssh-embedded=true' diff --git a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonTest.java b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonTest.java index b81288508..4fa398d8a 100644 --- a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonTest.java +++ b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonTest.java @@ -78,7 +78,10 @@ protected AuthenticationInfo getAuthInfo() { } protected long getExpectedLastModifiedOnGet(Repository repository, Resource resource) { - return new File(repository.getBasedir(), resource.getName()).lastModified(); + // the scp protocol carries the modification time in whole seconds (the "T" header), so that is the + // precision the wagon reports back - truncate to match, the "remote" file here is a local file whose + // timestamp still has millisecond precision + return new File(repository.getBasedir(), resource.getName()).lastModified() / 1000L * 1000L; } @Override diff --git a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonWithKeyTest.java b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonWithKeyTest.java index 6bd0fdee4..3f5ede7a9 100644 --- a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonWithKeyTest.java +++ b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/AbstractEmbeddedScpWagonWithKeyTest.java @@ -76,7 +76,10 @@ protected AuthenticationInfo getAuthInfo() { } protected long getExpectedLastModifiedOnGet(Repository repository, Resource resource) { - return new File(repository.getBasedir(), resource.getName()).lastModified(); + // the scp protocol carries the modification time in whole seconds (the "T" header), so that is the + // precision the wagon reports back - truncate to match, the "remote" file here is a local file whose + // timestamp still has millisecond precision + return new File(repository.getBasedir(), resource.getName()).lastModified() / 1000L * 1000L; } public void testConnect() throws Exception { diff --git a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/ShellCommand.java b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/ShellCommand.java index ddb8fd773..862f82a8d 100644 --- a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/ShellCommand.java +++ b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/ShellCommand.java @@ -126,12 +126,6 @@ public void start(ChannelSession channel, Environment env) throws IOException { callback.onExit(exitValue, stdout.getOutput()); } } - /* - out.write( exitValue ); - out.write( '\n' ); - - */ - out.flush(); } public void destroy(ChannelSession channel) throws Exception {} diff --git a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/SshServerEmbedded.java b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/SshServerEmbedded.java index a314869b3..c7fe3dd2f 100644 --- a/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/SshServerEmbedded.java +++ b/wagon-providers/wagon-ssh-common-test/src/main/java/org/apache/maven/wagon/providers/ssh/SshServerEmbedded.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.apache.sshd.common.file.nativefs.NativeFileSystemFactory; @@ -29,6 +30,7 @@ import org.apache.sshd.server.auth.password.PasswordAuthenticator; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; import org.apache.sshd.server.shell.ProcessShellFactory; +import org.apache.sshd.sftp.server.SftpSubsystemFactory; /** * @author Olivier Lamy @@ -97,6 +99,10 @@ public int start() throws IOException { .build(); sshd.setCommandFactory(commandFactory); + // OpenSSH 9 and later drive "scp" over the SFTP protocol rather than the legacy scp protocol, so the + // wagon-ssh-external tests, which shell out to the system scp, need the subsystem to be available + sshd.setSubsystemFactories(Collections.singletonList(new SftpSubsystemFactory())); + sshd.setFileSystemFactory(new NativeFileSystemFactory()); sshd.start(); this.port = sshd.getPort(); diff --git a/wagon-providers/wagon-ssh-external/pom.xml b/wagon-providers/wagon-ssh-external/pom.xml index 7164504b4..67217a1de 100644 --- a/wagon-providers/wagon-ssh-external/pom.xml +++ b/wagon-providers/wagon-ssh-external/pom.xml @@ -115,6 +115,12 @@ under the License. **/SshCommandExecutorTest.* **/Scp*Test.* + + **/EmbeddedScp*WagonWithKeyTest.* diff --git a/wagon-providers/wagon-ssh/pom.xml b/wagon-providers/wagon-ssh/pom.xml index 717d29340..d6f80667d 100644 --- a/wagon-providers/wagon-ssh/pom.xml +++ b/wagon-providers/wagon-ssh/pom.xml @@ -165,6 +165,12 @@ under the License. ssh-embedded + + + !windows + ssh-embedded true @@ -175,7 +181,7 @@ under the License. maven-surefire-plugin - + **/SftpWagonTest.* **/SshCommandExecutorTest.* diff --git a/wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/ScpWagon.java b/wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/ScpWagon.java index 978f225e7..54e3b0886 100644 --- a/wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/ScpWagon.java +++ b/wagon-providers/wagon-ssh/src/main/java/org/apache/maven/wagon/providers/ssh/jsch/ScpWagon.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Locale; import com.jcraft.jsch.ChannelExec; import com.jcraft.jsch.JSchException; @@ -54,6 +55,12 @@ public class ScpWagon extends AbstractJschWagon { private static final char COPY_START_CHAR = 'C'; + /** scp acknowledgement byte for a recoverable error, what OpenSSH sends for a missing file */ + private static final int SCP_WARNING = 1; + + /** scp acknowledgement byte for a fatal error */ + private static final int SCP_ERROR = 2; + private static final char ACK_SEPARATOR = ' '; private static final String END_OF_FILES_MSG = "E\n"; @@ -222,9 +229,13 @@ public void fillInputData(InputData inputData) throws TransferFailedException, R String line = readLine(in); if (exitCode != COPY_START_CHAR) { - if (exitCode == 1 - && (line.contains("No such file or directory") - || line.indexOf("no such file or directory") != 1)) { + // OpenSSH reports a missing file with the scp "warning" code (1); other servers - Apache MINA + // sshd, which the tests run against - report it with the "fatal error" code (2), so fall back on + // the message for those + if (exitCode == SCP_WARNING + || (exitCode == SCP_ERROR + && line != null + && line.toLowerCase(Locale.ROOT).contains("no such file or directory"))) { throw new ResourceDoesNotExistException(line); } else { throw new IOException("Exit code: " + exitCode + " - " + line); diff --git a/wagon-providers/wagon-ssh/src/test/java/org/apache/maven/wagon/providers/ssh/jsch/TestPrompter.java b/wagon-providers/wagon-ssh/src/test/java/org/apache/maven/wagon/providers/ssh/jsch/TestPrompter.java new file mode 100644 index 000000000..8bc9577b3 --- /dev/null +++ b/wagon-providers/wagon-ssh/src/test/java/org/apache/maven/wagon/providers/ssh/jsch/TestPrompter.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 org.apache.maven.wagon.providers.ssh.jsch; + +import java.util.List; + +import org.codehaus.plexus.components.interactivity.Prompter; +import org.codehaus.plexus.components.interactivity.PrompterException; + +/** + * A {@link Prompter} for the tests, wired in through + * {@code src/test/resources/META-INF/plexus/components.xml}. + *

+ * {@code plexus-interactivity-api} stopped shipping a {@code META-INF/plexus/components.xml} in 1.3, so its + * {@code DefaultPrompter} is invisible to the {@code plexus-container-default} that {@code PlexusTestCase} runs; + * without a replacement every {@code lookup( Wagon.ROLE, "scp" )} fails because {@code ConsoleInteractiveUserInfo} + * and {@code PrompterUIKeyboardInteractive} both require one. + *

+ * The tests are non-interactive, so nothing should ever prompt: every method throws rather than blocking on + * {@code System.in}. + */ +public class TestPrompter implements Prompter { + + public String prompt(String message) throws PrompterException { + throw unexpected(message); + } + + public String prompt(String message, String defaultReply) throws PrompterException { + throw unexpected(message); + } + + public String prompt(String message, List possibleValues) throws PrompterException { + throw unexpected(message); + } + + public String prompt(String message, List possibleValues, String defaultReply) throws PrompterException { + throw unexpected(message); + } + + public String promptForPassword(String message) throws PrompterException { + throw unexpected(message); + } + + public void showMessage(String message) throws PrompterException { + // tests run non-interactively, but a message costs nothing and helps when a test does go interactive + System.out.println(message); + } + + private PrompterException unexpected(String message) { + return new PrompterException("The tests are non-interactive, unexpected prompt: " + message); + } +} diff --git a/wagon-providers/wagon-ssh/src/test/resources/META-INF/plexus/components.xml b/wagon-providers/wagon-ssh/src/test/resources/META-INF/plexus/components.xml new file mode 100644 index 000000000..5aa9211fe --- /dev/null +++ b/wagon-providers/wagon-ssh/src/test/resources/META-INF/plexus/components.xml @@ -0,0 +1,35 @@ + + + + + + + + org.codehaus.plexus.components.interactivity.Prompter + default + org.apache.maven.wagon.providers.ssh.jsch.TestPrompter + + +