From 4bb3f5041f0a84cd736399fc1a0da96b7aeab742 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 10 Mar 2017 15:45:02 -0500 Subject: [PATCH] INT-4243: Upgrade to sshd 1.4 JIRA: https://jira.spring.io/browse/INT-4243 Remove newline char from key exchange. Fix CR and Add Windows Delete Diagnostics Fix For Windows More diagnostics. It appears the new server doesn't close the file when the session is closed. In the streaming test, consume the stream before closing the session. Polishing - PR Comments * More polishing according PR comments --- build.gradle | 2 +- .../file/remote/RemoteFileTestSupport.java | 25 +++++++++++++-- ...RedisInboundChannelAdapterParserTests.java | 4 +-- .../integration/sftp/SftpTestSupport.java | 13 ++++---- .../integration/sftp/dsl/SftpTests.java | 26 +++++++++------- .../session/SftpRemoteFileTemplateTests.java | 17 ++++++++-- .../sftp/session/SftpServerTests.java | 31 ++++++++++++------- .../sftp/session/SftpSessionFactoryTests.java | 13 ++++---- 8 files changed, 86 insertions(+), 45 deletions(-) diff --git a/build.gradle b/build.gradle index 8d071c786d..475e867e22 100644 --- a/build.gradle +++ b/build.gradle @@ -89,7 +89,7 @@ subprojects { subproject -> ext { activeMqVersion = '5.14.3' aspectjVersion = '1.8.9' - apacheSshdVersion = '0.14.0' + apacheSshdVersion = '1.4.0' boonVersion = '0.33' chronicleVersion = '3.5.3' commonsDbcpVersion = '1.4' diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java index 889e61f62d..659988292c 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 the original author or authors. + * Copyright 2016-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,10 +19,15 @@ package org.springframework.integration.file.remote; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.util.Arrays; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.junit.Before; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.rules.TemporaryFolder; +import org.junit.rules.TestName; /** * Abstract base class for tests requiring remote file servers, e.g. (S)FTP. @@ -33,6 +38,8 @@ import org.junit.rules.TemporaryFolder; */ public abstract class RemoteFileTestSupport { + protected final Log logger = LogFactory.getLog(getClass()); + protected static int port; @ClassRule @@ -41,6 +48,9 @@ public abstract class RemoteFileTestSupport { @ClassRule public static final TemporaryFolder localTemporaryFolder = new TemporaryFolder(); + @Rule + public final TestName testName = new TestName(); + protected volatile File sourceRemoteDirectory; protected volatile File targetRemoteDirectory; @@ -162,15 +172,24 @@ public abstract class RemoteFileTestSupport { File[] files = file.listFiles(); if (files != null) { for (File fyle : files) { + logger.info("Deleting: " + fyle + " in " + testName.getMethodName()); if (fyle.isDirectory()) { recursiveDelete(fyle); } else { - fyle.delete(); + if (!fyle.delete()) { + logger.error("Couldn't delete: " + fyle + " in " + testName.getMethodName()); + } } } } - file.delete(); + logger.info("Deleting: " + file + " in " + testName.getMethodName()); + if (!file.delete()) { + logger.error("Couldn't delete: " + file + " in " + testName.getMethodName()); + if (file.isDirectory()) { + logger.error("Contents: " + Arrays.toString(file.listFiles())); + } + } } } diff --git a/spring-integration-redis/src/test/java/org/springframework/integration/redis/config/RedisInboundChannelAdapterParserTests.java b/spring-integration-redis/src/test/java/org/springframework/integration/redis/config/RedisInboundChannelAdapterParserTests.java index 0482ae1332..4f4144a9b1 100644 --- a/spring-integration-redis/src/test/java/org/springframework/integration/redis/config/RedisInboundChannelAdapterParserTests.java +++ b/spring-integration-redis/src/test/java/org/springframework/integration/redis/config/RedisInboundChannelAdapterParserTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -95,7 +95,7 @@ public class RedisInboundChannelAdapterParserTests extends RedisAvailableTests { QueueChannel receiveChannel = context.getBean("receiveChannel", QueueChannel.class); for (int i = 0; i < 3; i++) { - Message receive = receiveChannel.receive(2000); + Message receive = receiveChannel.receive(10000); assertNotNull(receive); assertThat(receive.getPayload(), Matchers.isOneOf("Hello Redis from foo", "Hello Redis from bar")); } diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/SftpTestSupport.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/SftpTestSupport.java index e12af0f2e3..f9174296c7 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/SftpTestSupport.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/SftpTestSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2016 the original author or authors. + * Copyright 2016-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,12 +19,12 @@ package org.springframework.integration.sftp; import java.io.File; import java.util.Collections; -import org.apache.sshd.SshServer; import org.apache.sshd.common.NamedFactory; import org.apache.sshd.common.file.virtualfs.VirtualFileSystemFactory; import org.apache.sshd.server.Command; +import org.apache.sshd.server.SshServer; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; -import org.apache.sshd.server.sftp.SftpSubsystem; +import org.apache.sshd.server.subsystem.sftp.SftpSubsystemFactory; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -46,6 +46,7 @@ public class SftpTestSupport extends RemoteFileTestSupport { private static SshServer server; + @Override public String getTargetLocalDirectoryName() { return targetLocalDirectory.getAbsolutePath() + File.separator; } @@ -60,9 +61,9 @@ public class SftpTestSupport extends RemoteFileTestSupport { server = SshServer.setUpDefaultServer(); server.setPasswordAuthenticator((username, password, session) -> true); server.setPort(0); - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser")); - server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystem.Factory())); - server.setFileSystemFactory(new VirtualFileSystemFactory(remoteTemporaryFolder.getRoot().getAbsolutePath())); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser"))); + server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystemFactory())); + server.setFileSystemFactory(new VirtualFileSystemFactory(remoteTemporaryFolder.getRoot().toPath())); server.start(); port = server.getPort(); } diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/dsl/SftpTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/dsl/SftpTests.java index 0de8846e35..276521c054 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/dsl/SftpTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/dsl/SftpTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -75,15 +75,15 @@ public class SftpTests extends SftpTestSupport { public void testSftpInboundFlow() { QueueChannel out = new QueueChannel(); IntegrationFlow flow = IntegrationFlows - .from(Sftp.inboundAdapter(sessionFactory()) - .preserveTimestamp(true) - .remoteDirectory("sftpSource") - .regexFilter(".*\\.txt$") - .localFilenameExpression("#this.toUpperCase() + '.a'") - .localDirectory(getTargetLocalDirectory()), - e -> e.id("sftpInboundAdapter").poller(Pollers.fixedDelay(100))) - .channel(out) - .get(); + .from(Sftp.inboundAdapter(sessionFactory()) + .preserveTimestamp(true) + .remoteDirectory("sftpSource") + .regexFilter(".*\\.txt$") + .localFilenameExpression("#this.toUpperCase() + '.a'") + .localDirectory(getTargetLocalDirectory()), + e -> e.id("sftpInboundAdapter").poller(Pollers.fixedDelay(100))) + .channel(out) + .get(); IntegrationFlowRegistration registration = this.flowContext.registration(flow).register(); Message message = out.receive(10_000); assertNotNull(message); @@ -110,19 +110,21 @@ public class SftpTests extends SftpTestSupport { .remoteDirectory("sftpSource") .regexFilter(".*\\.txt$"), e -> e.id("sftpInboundAdapter").poller(Pollers.fixedDelay(100))) - .channel(out) - .get(); + .channel(out) + .get(); IntegrationFlowRegistration registration = this.flowContext.registration(flow).register(); Message message = out.receive(10_000); assertNotNull(message); assertThat(message.getPayload(), instanceOf(InputStream.class)); assertThat(message.getHeaders().get(FileHeaders.REMOTE_FILE), isOneOf(" sftpSource1.txt", "sftpSource2.txt")); + ((InputStream) message.getPayload()).close(); new IntegrationMessageHeaderAccessor(message).getCloseableResource().close(); message = out.receive(10_000); assertNotNull(message); assertThat(message.getPayload(), instanceOf(InputStream.class)); assertThat(message.getHeaders().get(FileHeaders.REMOTE_FILE), isOneOf("sftpSource1.txt", "sftpSource2.txt")); + ((InputStream) message.getPayload()).close(); new IntegrationMessageHeaderAccessor(message).getCloseableResource().close(); registration.destroy(); diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplateTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplateTests.java index 0e0387fc06..b8e015d0ad 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplateTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplateTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,10 +16,16 @@ package org.springframework.integration.sftp.session; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; + import org.junit.Test; import org.junit.runner.RunWith; @@ -81,10 +87,15 @@ public class SftpRemoteFileTemplateTests extends SftpTestSupport { } }); template.execute((SessionCallbackWithoutResult) session -> { + LsEntry[] files = session.list("foo/"); + assertEquals(4, files.length); assertTrue(session.remove("foo/foobar.txt")); assertTrue(session.rmdir("foo/bar/")); - LsEntry[] files = session.list("foo/"); - assertEquals(0, files.length); + files = session.list("foo/"); + assertEquals(2, files.length); + List list = Arrays.asList(files); + assertThat(list.stream().map(l -> l.getFilename()).collect(Collectors.toList()), + containsInAnyOrder(".", "..")); assertTrue(session.rmdir("foo/")); }); assertFalse(template.exists("foo")); diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpServerTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpServerTests.java index c2720ea192..2b82622b80 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpServerTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpServerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,23 +25,25 @@ import java.io.IOException; import java.io.InputStream; import java.math.BigInteger; import java.nio.ByteBuffer; +import java.nio.file.Paths; import java.security.KeyFactory; import java.security.PublicKey; import java.security.spec.RSAPublicKeySpec; +import java.util.Arrays; import java.util.Collections; -import org.apache.sshd.SshServer; import org.apache.sshd.common.NamedFactory; import org.apache.sshd.common.file.virtualfs.VirtualFileSystemFactory; -import org.apache.sshd.common.util.Base64; import org.apache.sshd.server.Command; +import org.apache.sshd.server.SshServer; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; -import org.apache.sshd.server.sftp.SftpSubsystem; +import org.apache.sshd.server.subsystem.sftp.SftpSubsystemFactory; import org.junit.Test; import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.ClassPathResource; import org.springframework.integration.file.remote.session.Session; +import org.springframework.util.Base64Utils; import org.springframework.util.StreamUtils; import com.jcraft.jsch.ChannelSftp.LsEntry; @@ -62,11 +64,11 @@ public class SftpServerTests { try { server.setPasswordAuthenticator((arg0, arg1, arg2) -> true); server.setPort(0); - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser")); - server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystem.Factory())); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser"))); + server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystemFactory())); final String pathname = System.getProperty("java.io.tmpdir") + File.separator + "sftptest" + File.separator; new File(pathname).mkdirs(); - server.setFileSystemFactory(new VirtualFileSystemFactory(pathname)); + server.setFileSystemFactory(new VirtualFileSystemFactory(Paths.get(pathname))); server.start(); DefaultSftpSessionFactory f = new DefaultSftpSessionFactory(); @@ -100,11 +102,11 @@ public class SftpServerTests { try { server.setPublickeyAuthenticator((username, key, session) -> key.equals(allowedKey)); server.setPort(0); - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser")); - server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystem.Factory())); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser"))); + server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystemFactory())); final String pathname = System.getProperty("java.io.tmpdir") + File.separator + "sftptest" + File.separator; new File(pathname).mkdirs(); - server.setFileSystemFactory(new VirtualFileSystemFactory(pathname)); + server.setFileSystemFactory(new VirtualFileSystemFactory(Paths.get(pathname))); server.start(); DefaultSftpSessionFactory f = new DefaultSftpSessionFactory(); @@ -125,7 +127,12 @@ public class SftpServerTests { private PublicKey decodePublicKey(String key) throws Exception { InputStream stream = new ClassPathResource(key).getInputStream(); - byte[] decodeBuffer = Base64.decodeBase64(StreamUtils.copyToByteArray(stream)); + byte[] keyBytes = StreamUtils.copyToByteArray(stream); + // strip any newline chars + while (keyBytes[keyBytes.length - 1] == 0x0a || keyBytes[keyBytes.length - 1] == 0x0d) { + keyBytes = Arrays.copyOf(keyBytes, keyBytes.length - 1); + } + byte[] decodeBuffer = Base64Utils.decode(keyBytes); ByteBuffer bb = ByteBuffer.wrap(decodeBuffer); int len = bb.getInt(); byte[] type = new byte[len]; @@ -157,7 +164,7 @@ public class SftpServerTests { } session.write(new ByteArrayInputStream("foo".getBytes()), "bar"); list = session.list("."); - assertEquals("bar", list[0].getFilename()); + assertEquals("bar", list[1].getFilename()); ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); session.read("bar", outputStream); assertEquals("foo", new String(outputStream.toByteArray())); diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java index 1187f4049f..980cea69ab 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2014-2016 the original author or authors. + * Copyright 2014-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,15 +28,16 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.io.File; import java.io.IOException; import java.net.ConnectException; import java.util.Collections; -import org.apache.sshd.SshServer; import org.apache.sshd.common.NamedFactory; import org.apache.sshd.server.Command; +import org.apache.sshd.server.SshServer; import org.apache.sshd.server.keyprovider.SimpleGeneratorHostKeyProvider; -import org.apache.sshd.server.sftp.SftpSubsystem; +import org.apache.sshd.server.subsystem.sftp.SftpSubsystemFactory; import org.junit.Test; import org.springframework.core.io.ClassPathResource; @@ -63,7 +64,7 @@ public class SftpSessionFactoryTests { try { server.setPasswordAuthenticator((arg0, arg1, arg2) -> true); server.setPort(0); - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser")); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser"))); server.start(); DefaultSftpSessionFactory f = new DefaultSftpSessionFactory(); @@ -212,8 +213,8 @@ public class SftpSessionFactoryTests { private DefaultSftpSessionFactory createServerAndClient(SshServer server) throws IOException { server.setPublickeyAuthenticator((username, key, session) -> true); server.setPort(0); - server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystem.Factory())); - server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider("hostkey.ser")); + server.setSubsystemFactories(Collections.>singletonList(new SftpSubsystemFactory())); + server.setKeyPairProvider(new SimpleGeneratorHostKeyProvider(new File("hostkey.ser"))); server.start(); DefaultSftpSessionFactory f = new DefaultSftpSessionFactory();