This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch ftp in repository https://gitbox.apache.org/repos/asf/camel.git
commit 86130ef53d4644c4b6f0d0b2d42d57743711f486 Author: Claus Ibsen <claus.ib...@gmail.com> AuthorDate: Thu Aug 8 07:13:37 2024 +0200 CAMEL-21051: camel-ftp - Do not swallow exception due to finally block can trigger another exception during ftp opertations. --- .../camel/component/file/remote/FtpOperations.java | 38 ++++++----- .../component/file/remote/SftpOperations.java | 73 ++++++++++------------ 2 files changed, 52 insertions(+), 59 deletions(-) diff --git a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/FtpOperations.java b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/FtpOperations.java index c4d3d5df486..8a34515511d 100644 --- a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/FtpOperations.java +++ b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/FtpOperations.java @@ -381,26 +381,24 @@ public class FtpOperations implements RemoteFileOperations<FTPFile> { String originalDirectory = client.printWorkingDirectory(); boolean success; - try { - // maybe the full directory already exists - success = client.changeWorkingDirectory(directory); + // maybe the full directory already exists + success = client.changeWorkingDirectory(directory); + if (!success) { + log.trace("Trying to build remote directory: {}", directory); + success = client.makeDirectory(directory); if (!success) { - log.trace("Trying to build remote directory: {}", directory); - success = client.makeDirectory(directory); - if (!success) { - // we are here if the server side doesn't create - // intermediate folders so create the folder one by one - success = buildDirectoryChunks(directory); - } + // we are here if the server side doesn't create + // intermediate folders so create the folder one by one + success = buildDirectoryChunks(directory); } + } - return success; - } finally { - // change back to original directory - if (originalDirectory != null) { - changeCurrentDirectory(originalDirectory); - } + // change back to original directory + if (originalDirectory != null) { + changeCurrentDirectory(originalDirectory); } + + return success; } catch (IOException e) { throw new GenericFileOperationFailedException(client.getReplyCode(), client.getReplyString(), e.getMessage(), e); } @@ -698,14 +696,14 @@ public class FtpOperations implements RemoteFileOperations<FTPFile> { // store the file answer = doStoreFile(name, targetName, exchange); - } catch (GenericFileOperationFailedException e) { - clientActivityListener.onGeneralError(endpoint.getConfiguration().remoteServerInformation(), e.getMessage()); - throw e; - } finally { + // change back to current directory if we changed directory if (currentDir != null) { changeCurrentDirectory(currentDir); } + } catch (GenericFileOperationFailedException e) { + clientActivityListener.onGeneralError(endpoint.getConfiguration().remoteServerInformation(), e.getMessage()); + throw e; } if (answer) { diff --git a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java index 71e3689d8e2..7b4d77700b3 100644 --- a/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java +++ b/components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/SftpOperations.java @@ -584,7 +584,6 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { if (!success) { LOG.debug("Trying to build remote directory: {}", directory); - try { channel.mkdir(directory); success = true; @@ -600,14 +599,15 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { chmodOfDirectory(directory); } } - } catch (SftpException e) { - throw new GenericFileOperationFailedException("Cannot build directory: " + directory, e); - } finally { + // change back to original directory if (originalDirectory != null) { changeCurrentDirectory(originalDirectory); } + } catch (SftpException e) { + throw new GenericFileOperationFailedException("Cannot build directory: " + directory, e); } + return success; } @@ -807,8 +807,8 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { @SuppressWarnings("unchecked") private boolean retrieveFileToStreamInBody(String name, Exchange exchange) throws GenericFileOperationFailedException { - String currentDir = null; try { + String currentDir = null; GenericFile<ChannelSftp.LsEntry> target = (GenericFile<ChannelSftp.LsEntry>) exchange.getProperty(FileComponent.FILE_EXCHANGE_FILE); ObjectHelper.notNull(target, "Exchange should have the " + FileComponent.FILE_EXCHANGE_FILE + " set"); @@ -847,17 +847,17 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { } createResultHeadersFromExchange(null, exchange); + + // change back to current directory if we changed directory + if (currentDir != null) { + changeCurrentDirectory(currentDir); + } return true; } catch (SftpException e) { createResultHeadersFromExchange(e, exchange); throw new GenericFileOperationFailedException("Cannot retrieve file: " + name, e); } catch (IOException e) { throw new GenericFileOperationFailedException("Cannot retrieve file: " + name, e); - } finally { - // change back to current directory if we changed directory - if (currentDir != null) { - changeCurrentDirectory(currentDir); - } } } @@ -906,8 +906,8 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { } catch (Exception e) { throw new GenericFileOperationFailedException("Cannot create new local work file: " + local, e); } - String currentDir = null; try { + String currentDir = null; // store the java.io.File handle as the body file.setBody(local); @@ -930,6 +930,11 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { channel.get(remoteName, os); + // change back to current directory if we changed directory + if (currentDir != null) { + changeCurrentDirectory(currentDir); + } + } catch (SftpException e) { createResultHeadersFromExchange(e, exchange); LOG.trace("Error occurred during retrieving file: {} to local directory. Deleting local work file: {}", name, temp); @@ -945,11 +950,6 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { throw new GenericFileOperationFailedException("Cannot retrieve file: " + name, e); } finally { IOHelper.close(os, "retrieve: " + name, LOG); - - // change back to current directory if we changed directory - if (currentDir != null) { - changeCurrentDirectory(currentDir); - } } createResultHeadersFromExchange(null, exchange); @@ -982,27 +982,25 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { String path = FileUtil.onlyPath(name); String targetName = name; - try { - if (path != null && endpoint.getConfiguration().isStepwise()) { - // must remember current dir so we stay in that directory after - // the write - currentDir = getCurrentDirectory(); + if (path != null && endpoint.getConfiguration().isStepwise()) { + // must remember current dir so we stay in that directory after + // the write + currentDir = getCurrentDirectory(); - // change to path of name - changeCurrentDirectory(path); + // change to path of name + changeCurrentDirectory(path); - // the target name should be without path, as we have changed - // directory - targetName = FileUtil.stripPath(name); - } + // the target name should be without path, as we have changed + // directory + targetName = FileUtil.stripPath(name); + } - // store the file - answer = doStoreFile(name, targetName, exchange); - } finally { - // change back to current directory if we changed directory - if (currentDir != null) { - changeCurrentDirectory(currentDir); - } + // store the file + answer = doStoreFile(name, targetName, exchange); + + // change back to current directory if we changed directory + if (currentDir != null) { + changeCurrentDirectory(currentDir); } return answer; @@ -1149,7 +1147,6 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { // otherwise its a more serious error so rethrow throw new GenericFileOperationFailedException(e.getMessage(), e); } - } @Override @@ -1178,7 +1175,7 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { * against the connection should bind */ static Socket createSocketUtil(final String host, final int port, final String bindAddress, final int timeout) { - Socket socket = null; + Socket socket; if (timeout == 0) { try { socket = new Socket(InetAddress.getByName(host), port, InetAddress.getByName(bindAddress), 0); @@ -1226,10 +1223,9 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { /** * Helper method which gets result code and message from sftpException and puts it into header. In case that - * exception is null, it sets successfull response. + * exception is null, it sets successfully response. */ private void createResultHeadersFromExchange(SftpException sftpException, Exchange exchange) { - // if exception is null, it means that result was ok if (sftpException == null) { exchange.getIn().setHeader(FtpConstants.FTP_REPLY_CODE, OK_STATUS); @@ -1245,7 +1241,6 @@ public class SftpOperations implements RemoteFileOperations<SftpRemoteFile> { * Helper method which sets the path permissions */ private void chmodOfDirectory(String directory) { - String chmodDirectory = endpoint.getConfiguration().getChmodDirectory(); if (ObjectHelper.isNotEmpty(chmodDirectory)) { LOG.trace("Setting permission: {} on directory: {}", chmodDirectory, directory);