Skip to content

Commit

Permalink
spring-projectsGH-3247: Fix SftpSession.exists for error code
Browse files Browse the repository at this point in the history
Fixes spring-projects#3247

When there is no path on the SFTP server, a `ChannelSftp.SSH_FX_NO_SUCH_FILE`
error is returned in the thrown `SftpException`.

* Fix `SftpSession.exists()` to check for the `SSH_FX_NO_SUCH_FILE` to
return `false` and re-throw an exception otherwise
* Add mock test for `SftpSession.exists()`
* Add `org.mockito.AdditionalMatchers` to `checkstyle.xml` exclusions

**Cherry-pick to 5.2.x & 5.1.x**
  • Loading branch information
artembilan committed Apr 13, 2020
1 parent 97702ae commit 050094a
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand Down Expand Up @@ -127,16 +127,15 @@ public LsEntry[] list(String path) throws IOException {
@Override
public String[] listNames(String path) throws IOException {
LsEntry[] entries = this.list(path);
List<String> names = new ArrayList<String>();
for (int i = 0; i < entries.length; i++) {
String fileName = entries[i].getFilename();
SftpATTRS attrs = entries[i].getAttrs();
List<String> names = new ArrayList<>();
for (LsEntry entry : entries) {
String fileName = entry.getFilename();
SftpATTRS attrs = entry.getAttrs();
if (!attrs.isDir() && !attrs.isLink()) {
names.add(fileName);
}
}
String[] fileNames = new String[names.size()];
return names.toArray(fileNames);
return names.toArray(new String[0]);
}


Expand Down Expand Up @@ -270,15 +269,19 @@ public boolean rmdir(String remoteDirectory) throws IOException {
}

@Override
public boolean exists(String path) {
public boolean exists(String path) throws IOException {
try {
this.channel.lstat(path);
return true;
}
catch (@SuppressWarnings("unused") SftpException e) {
// ignore
catch (SftpException ex) {
if (ex.id == ChannelSftp.SSH_FX_NO_SUCH_FILE) {
return false;
}
else {
throw new NestedIOException("Cannot check 'lstat' for path " + path, ex);
}
}
return false;
}

void connect() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
Expand All @@ -17,8 +17,14 @@
package org.springframework.integration.sftp.outbound;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.AdditionalMatchers.and;
import static org.mockito.AdditionalMatchers.not;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.willAnswer;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.BDDMockito.willThrow;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
Expand All @@ -30,6 +36,7 @@

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
Expand All @@ -38,12 +45,13 @@
import java.util.Vector;
import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.context.support.ClassPathXmlApplicationContext;
import org.springframework.core.NestedIOException;
import org.springframework.expression.common.LiteralExpression;
import org.springframework.integration.file.DefaultFileNameGenerator;
import org.springframework.integration.file.remote.FileInfo;
Expand All @@ -67,6 +75,7 @@
import com.jcraft.jsch.JSch;
import com.jcraft.jsch.JSchException;
import com.jcraft.jsch.SftpATTRS;
import com.jcraft.jsch.SftpException;

/**
* @author Oleg Zhurakousky
Expand All @@ -76,15 +85,15 @@
*/
public class SftpOutboundTests {

private static com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);
private static final com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);

@Test
public void testHandleFileMessage() throws Exception {
File targetDir = new File("remote-target-dir");
assertThat(targetDir.exists()).as("target directory does not exist: " + targetDir.getName()).isTrue();

SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
handler.setRemoteDirectoryExpression(new LiteralExpression(targetDir.getName()));
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
fGenerator.setBeanFactory(mock(BeanFactory.class));
Expand Down Expand Up @@ -133,7 +142,7 @@ public void testHandleBytesMessage() throws Exception {
file.delete();
}
SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
fGenerator.setBeanFactory(mock(BeanFactory.class));
fGenerator.setExpression("'foo.txt'");
Expand All @@ -142,7 +151,7 @@ public void testHandleBytesMessage() throws Exception {
handler.setBeanFactory(mock(BeanFactory.class));
handler.afterPropertiesSet();

handler.handleMessage(new GenericMessage<byte[]>("byte[] data".getBytes()));
handler.handleMessage(new GenericMessage<>("byte[] data".getBytes()));
assertThat(new File("remote-target-dir", "foo.txt").exists()).isTrue();
byte[] inFile = FileCopyUtils.copyToByteArray(file);
assertThat(new String(inFile)).isEqualTo("byte[] data");
Expand Down Expand Up @@ -171,7 +180,7 @@ public void testSftpOutboundChannelAdapterInsideChain() throws Exception {
}

@Test //INT-2275
public void testFtpOutboundGatewayInsideChain() throws Exception {
public void testFtpOutboundGatewayInsideChain() {
ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext(
"SftpOutboundInsideChainTests-context.xml", getClass());

Expand Down Expand Up @@ -202,17 +211,17 @@ public void testMkDir() throws Exception {
@SuppressWarnings("unchecked")
SessionFactory<LsEntry> sessionFactory = mock(SessionFactory.class);
when(sessionFactory.getSession()).thenReturn(session);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
handler.setAutoCreateDirectory(true);
handler.setRemoteDirectoryExpression(new LiteralExpression("/foo/bar/baz"));
handler.setBeanFactory(mock(BeanFactory.class));
handler.afterPropertiesSet();
final List<String> madeDirs = new ArrayList<String>();
final List<String> madeDirs = new ArrayList<>();
doAnswer(invocation -> {
madeDirs.add(invocation.getArgument(0));
return null;
}).when(session).mkdir(anyString());
handler.handleMessage(new GenericMessage<String>("qux"));
handler.handleMessage(new GenericMessage<>("qux"));
assertThat(madeDirs.size()).isEqualTo(3);
assertThat(madeDirs.get(0)).isEqualTo("/foo");
assertThat(madeDirs.get(1)).isEqualTo("/foo/bar");
Expand Down Expand Up @@ -380,6 +389,34 @@ public void testSharedSessionCachedReset() throws Exception {
verify(jschSession2).disconnect();
}

@Test
public void testExists() throws SftpException, IOException {
ChannelSftp channelSftp = mock(ChannelSftp.class);

willReturn(mock(SftpATTRS.class))
.given(channelSftp)
.lstat(eq("exist"));

willThrow(new SftpException(ChannelSftp.SSH_FX_NO_SUCH_FILE, "Path does not exist."))
.given(channelSftp)
.lstat(eq("notExist"));

willThrow(new SftpException(ChannelSftp.SSH_FX_CONNECTION_LOST, "Connection lost."))
.given(channelSftp)
.lstat(and(not(eq("exist")), not(eq("notExist"))));

SftpSession sftpSession = new SftpSession(mock(com.jcraft.jsch.Session.class));
DirectFieldAccessor fieldAccessor = new DirectFieldAccessor(sftpSession);
fieldAccessor.setPropertyValue("channel", channelSftp);

assertThat(sftpSession.exists("exist")).isTrue();

assertThat(sftpSession.exists("notExist")).isFalse();

assertThatExceptionOfType(NestedIOException.class).
isThrownBy(() -> sftpSession.exists("foo"));
}

private void noopConnect(ChannelSftp channel1) throws JSchException {
doNothing().when(channel1).connect(5000);
}
Expand Down
1 change: 1 addition & 0 deletions src/checkstyle/checkstyle.xml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
org.mockito.BDDMockito.*,
org.mockito.AdditionalAnswers.*,
org.mockito.ArgumentMatchers.*,
org.mockito.AdditionalMatchers.*,
org.springframework.integration.gemfire.config.xml.ParserTestUtil.*,
org.springframework.integration.test.mock.MockIntegration.*,
org.springframework.integration.test.util.TestUtils.*,
Expand Down

0 comments on commit 050094a

Please sign in to comment.