From 8466f09f40ea2601cde5dcbf45bd87803742c569 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 1 Feb 2021 16:04:08 -0500 Subject: [PATCH] GH-3482: (S)FTP: Fix Recursive LS (ARFOG) Resolves https://github.com/spring-projects/spring-integration/issues/3482 `.` and `..` should be ignored when recursing. **cherry-pick to 5.4.x, 5.3.x** * Fix checkstyle. * Fix test in `file` module - test was incorrect; it would have detected this problem. --- .../AbstractRemoteFileOutboundGateway.java | 15 ++-- .../RemoteFileOutboundGatewayTests.java | 7 +- .../SftpServerOutboundTests-context.xml | 33 +++++++++ .../outbound/SftpServerOutboundTests.java | 70 ++++++++++++++++++- 4 files changed, 116 insertions(+), 9 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java index 1c141dc5cd2..3bf418a78bc 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java @@ -977,18 +977,23 @@ protected final List filterFiles(F[] files) { private void processFile(Session session, String directory, String subDirectory, List lsFiles, boolean recursion, F file) throws IOException { + String fileName = getFilename(file); + String fileSep = this.remoteFileTemplate.getRemoteFileSeparator(); + boolean isDots = ".".equals(fileName) + || "..".equals(fileName) + || fileName.endsWith(fileSep + ".") + || fileName.endsWith(fileSep + ".."); if (this.options.contains(Option.SUBDIRS) || !isDirectory(file)) { - if (recursion && StringUtils.hasText(subDirectory)) { + if (recursion && StringUtils.hasText(subDirectory) && (!isDots || this.options.contains(Option.ALL))) { lsFiles.add(enhanceNameWithSubDirectory(file, subDirectory)); } - else { + else if (this.options.contains(Option.ALL) || !isDots) { lsFiles.add(file); } } - String fileName = getFilename(file); - if (recursion && isDirectory(file) && !(".".equals(fileName)) && !("..".equals(fileName))) { + if (recursion && isDirectory(file) && !isDots) { lsFiles.addAll(listFilesInRemoteDir(session, directory, - subDirectory + fileName + this.remoteFileTemplate.getRemoteFileSeparator())); + subDirectory + fileName + fileSep)); } } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java index 55c73cdbe2f..d3943f27320 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java @@ -406,12 +406,13 @@ public void testLs_f_R_dirs() throws Exception { MessageBuilder> out = (MessageBuilder>) gw .handleRequestMessage(new GenericMessage<>("testremote/x")); assertThat(out).isNotNull(); - assertThat(out.getPayload()).hasSize(5); + assertThat(out.getPayload()).hasSize(6); assertThat(out.getPayload().get(0).getFilename()).isEqualTo("f1"); assertThat(out.getPayload().get(1).getFilename()).isEqualTo("d1"); assertThat(out.getPayload().get(2).getFilename()).isEqualTo("d1/d2"); - assertThat(out.getPayload().get(3).getFilename()).isEqualTo("d1/f3"); - assertThat(out.getPayload().get(4).getFilename()).isEqualTo("f2"); + assertThat(out.getPayload().get(3).getFilename()).isEqualTo("d1/d2/f4"); + assertThat(out.getPayload().get(4).getFilename()).isEqualTo("d1/f3"); + assertThat(out.getPayload().get(5).getFilename()).isEqualTo("f2"); assertThat(out.getHeaders().get(FileHeaders.REMOTE_DIRECTORY)).isEqualTo("testremote/x/"); } diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml index 3b711f4a537..4fed9784e6d 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml @@ -59,6 +59,39 @@ local-filename-generator-expression="#remoteFileName.replaceFirst('sftpSource', 'localTarget')" reply-channel="output"/> + + + + + + diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests.java b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests.java index 04a05d17377..fee9cbeab7a 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests.java +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2020 the original author or authors. + * Copyright 2013-2021 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. @@ -35,6 +35,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.regex.Matcher; +import java.util.stream.Collectors; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeEach; @@ -60,6 +61,7 @@ import org.springframework.integration.sftp.server.SessionClosedEvent; import org.springframework.integration.sftp.server.SessionOpenedEvent; import org.springframework.integration.sftp.session.DefaultSftpSessionFactory; +import org.springframework.integration.sftp.session.SftpFileInfo; import org.springframework.integration.sftp.session.SftpRemoteFileTemplate; import org.springframework.integration.support.MessageBuilder; import org.springframework.integration.test.util.TestUtils; @@ -100,6 +102,15 @@ public class SftpServerOutboundTests extends SftpTestSupport { @Autowired private DirectChannel inboundMGetRecursive; + @Autowired + private DirectChannel inboundLSRecursive; + + @Autowired + private DirectChannel inboundLSRecursiveALL; + + @Autowired + private DirectChannel inboundLSRecursiveNoDirs; + @Autowired private DirectChannel inboundMGetRecursiveFiltered; @@ -254,6 +265,63 @@ public void testInt3172LocalDirectoryExpressionMGETRecursive() throws IOExceptio FileUtils.copyInputStreamToFile(new ByteArrayInputStream(localAsString.getBytes()), secondRemote); } + @Test + @SuppressWarnings("unchecked") + void testLSRecursive() throws IOException { + String dir = "sftpSource/"; + this.inboundLSRecursive.send(new GenericMessage(dir)); + Message result = this.output.receive(1000); + assertThat(result).isNotNull(); + List files = (List) result.getPayload(); + assertThat(files).hasSize(4); + assertThat(files.stream() + .map(fi -> fi.getFilename()) + .collect(Collectors.toList())).contains( + " sftpSource1.txt", + "sftpSource2.txt", + "subSftpSource", + "subSftpSource/subSftpSource1.txt"); + } + + @Test + @SuppressWarnings("unchecked") + void testLSRecursiveALL() throws IOException { + String dir = "sftpSource/"; + this.inboundLSRecursiveALL.send(new GenericMessage(dir)); + Message result = this.output.receive(1000); + assertThat(result).isNotNull(); + List files = (List) result.getPayload(); + assertThat(files).hasSize(8); + assertThat(files.stream() + .map(fi -> fi.getFilename()) + .collect(Collectors.toList())).contains( + " sftpSource1.txt", + "sftpSource2.txt", + "subSftpSource", + "subSftpSource/subSftpSource1.txt", + ".", + "..", + "subSftpSource/.", + "subSftpSource/.."); + } + + @Test + @SuppressWarnings("unchecked") + void testLSRecursiveNoDirs() throws IOException { + String dir = "sftpSource/"; + this.inboundLSRecursiveNoDirs.send(new GenericMessage(dir)); + Message result = this.output.receive(1000); + assertThat(result).isNotNull(); + List files = (List) result.getPayload(); + assertThat(files).hasSize(3); + assertThat(files.stream() + .map(fi -> fi.getFilename()) + .collect(Collectors.toList())).contains( + " sftpSource1.txt", + "sftpSource2.txt", + "subSftpSource/subSftpSource1.txt"); + } + private long setModifiedOnSource1() { File firstRemote = new File(getSourceRemoteDirectory(), " sftpSource1.txt"); firstRemote.setLastModified(System.currentTimeMillis() - 1_000_000);