From e4f1ee14aa300454f096b0e7c559ea9aa3f60f2f Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 21 Oct 2014 13:57:21 -0400 Subject: [PATCH] INT-3537 RemoteFileTemplate Close Stream JIRA: https://jira.spring.io/browse/INT-3537 Stream not closed if session cannot be created. Add `try {} finally {}`. __cherry-pick to 4.0.x, 3.0.x__ --- .../file/remote/RemoteFileTemplate.java | 83 ++++++++++--------- .../session/FtpRemoteFileTemplateTests.java | 33 ++++++++ 2 files changed, 79 insertions(+), 37 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileTemplate.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileTemplate.java index eeff5a674d..83d45e61da 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileTemplate.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileTemplate.java @@ -205,48 +205,57 @@ public class RemoteFileTemplate implements RemoteFileOperations, Initializ "Cannot append when using a temporary file name"); final StreamHolder inputStreamHolder = this.payloadToInputStream(message); if (inputStreamHolder != null) { - return this.execute(new SessionCallback() { + try { + return this.execute(new SessionCallback() { - @Override - public String doInSession(Session session) throws IOException { - String fileName = inputStreamHolder.getName(); - try { - String remoteDirectory = RemoteFileTemplate.this.directoryExpressionProcessor - .processMessage(message); - remoteDirectory = RemoteFileTemplate.this.normalizeDirectoryPath(remoteDirectory); - if (StringUtils.hasText(subDirectory)) { - if (subDirectory.startsWith(RemoteFileTemplate.this.remoteFileSeparator)) { - remoteDirectory += subDirectory.substring(1); - } - else { - remoteDirectory += RemoteFileTemplate.this.normalizeDirectoryPath(subDirectory); - } - } - String temporaryRemoteDirectory = remoteDirectory; - if (RemoteFileTemplate.this.temporaryDirectoryExpressionProcessor != null) { - temporaryRemoteDirectory = RemoteFileTemplate.this.temporaryDirectoryExpressionProcessor + @Override + public String doInSession(Session session) throws IOException { + String fileName = inputStreamHolder.getName(); + try { + String remoteDirectory = RemoteFileTemplate.this.directoryExpressionProcessor .processMessage(message); + remoteDirectory = RemoteFileTemplate.this.normalizeDirectoryPath(remoteDirectory); + if (StringUtils.hasText(subDirectory)) { + if (subDirectory.startsWith(RemoteFileTemplate.this.remoteFileSeparator)) { + remoteDirectory += subDirectory.substring(1); + } + else { + remoteDirectory += RemoteFileTemplate.this.normalizeDirectoryPath(subDirectory); + } + } + String temporaryRemoteDirectory = remoteDirectory; + if (RemoteFileTemplate.this.temporaryDirectoryExpressionProcessor != null) { + temporaryRemoteDirectory = RemoteFileTemplate.this.temporaryDirectoryExpressionProcessor + .processMessage(message); + } + fileName = RemoteFileTemplate.this.fileNameGenerator.generateFileName(message); + RemoteFileTemplate.this.sendFileToRemoteDirectory(inputStreamHolder.getStream(), + temporaryRemoteDirectory, remoteDirectory, fileName, session, mode); + return remoteDirectory + fileName; + } + catch (FileNotFoundException e) { + throw new MessageDeliveryException(message, "File [" + inputStreamHolder.getName() + + "] not found in local working directory; it was moved or deleted unexpectedly.", e); + } + catch (IOException e) { + throw new MessageDeliveryException(message, "Failed to transfer file [" + + inputStreamHolder.getName() + " -> " + fileName + + "] from local directory to remote directory.", e); + } + catch (Exception e) { + throw new MessageDeliveryException(message, "Error handling message for file [" + + inputStreamHolder.getName() + " -> " + fileName + "]", e); } - fileName = RemoteFileTemplate.this.fileNameGenerator.generateFileName(message); - RemoteFileTemplate.this.sendFileToRemoteDirectory(inputStreamHolder.getStream(), - temporaryRemoteDirectory, remoteDirectory, fileName, session, mode); - return remoteDirectory + fileName; - } - catch (FileNotFoundException e) { - throw new MessageDeliveryException(message, "File [" + inputStreamHolder.getName() - + "] not found in local working directory; it was moved or deleted unexpectedly.", e); - } - catch (IOException e) { - throw new MessageDeliveryException(message, "Failed to transfer file [" - + inputStreamHolder.getName() + " -> " + fileName - + "] from local directory to remote directory.", e); - } - catch (Exception e) { - throw new MessageDeliveryException(message, "Error handling message for file [" - + inputStreamHolder.getName() + " -> " + fileName + "]", e); } + }); + } + finally { + try { + inputStreamHolder.getStream().close(); } - }); + catch (IOException e) { + } + } } else { // A null holder means a File payload that does not exist. diff --git a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/session/FtpRemoteFileTemplateTests.java b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/session/FtpRemoteFileTemplateTests.java index 97bfb67924..e8f4dd96af 100644 --- a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/session/FtpRemoteFileTemplateTests.java +++ b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/session/FtpRemoteFileTemplateTests.java @@ -18,8 +18,14 @@ package org.springframework.integration.ftp.session; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; +import java.util.UUID; import org.apache.commons.net.ftp.FTPClient; import org.apache.commons.net.ftp.FTPFile; @@ -35,7 +41,9 @@ import org.springframework.integration.file.remote.ClientCallbackWithoutResult; import org.springframework.integration.file.remote.SessionCallback; import org.springframework.integration.file.remote.SessionCallbackWithoutResult; import org.springframework.integration.file.remote.session.Session; +import org.springframework.integration.file.remote.session.SessionFactory; import org.springframework.integration.ftp.TestFtpServer; +import org.springframework.messaging.MessagingException; import org.springframework.messaging.support.GenericMessage; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -109,4 +117,29 @@ public class FtpRemoteFileTemplateTests { assertFalse(template.exists("foo")); } + @Test + public void testFileCloseOnBadConnect() throws Exception { + @SuppressWarnings("unchecked") + SessionFactory sessionFactory = mock(SessionFactory.class); + when(sessionFactory.getSession()).thenThrow(new RuntimeException("bar")); + FtpRemoteFileTemplate template = new FtpRemoteFileTemplate(sessionFactory); + template.setRemoteDirectoryExpression(new LiteralExpression("foo")); + template.afterPropertiesSet(); + File file = new File(System.getProperty("java.io.tmpdir"), UUID.randomUUID().toString()); + FileOutputStream fileOutputStream = new FileOutputStream(file); + fileOutputStream.write("foo".getBytes()); + fileOutputStream.close(); + try { + template.send(new GenericMessage(file)); + fail("exception expected"); + } + catch(MessagingException e) { + assertEquals("bar", e.getCause().getMessage()); + } + File newFile = new File(System.getProperty("java.io.tmpdir"), UUID.randomUUID().toString()); + assertTrue(file.renameTo(newFile)); + file.delete(); + newFile.delete(); + } + }