Skip to content

Commit a9efc9b

Browse files
committed
spring-projectsGH-3827: Fix RemoteFile GET STREAM session leak
Fixes spring-projects#3827 The `AbstractRemoteFileOutboundGateway.doGet()` for a `STREAM` option does not close the `session` in case of error. This may lead to some leaks or exhausted caches * Close `session` in the `catch()` of the `AbstractRemoteFileOutboundGateway.doGet()` * Adjust the `SftpServerOutboundTests` to configure a `CachingSessionFactory` for the `testStream()` to verify there is no leaks attempting to `GET STREAM` non-existing remote file twice **Cherry-pick to `5.5.x`**
1 parent 1c461a3 commit a9efc9b

File tree

3 files changed

+22
-5
lines changed

3 files changed

+22
-5
lines changed

spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -673,6 +673,7 @@ private Object doGet(final Message<?> requestMessage) {
673673
.setHeader(IntegrationMessageHeaderAccessor.CLOSEABLE_RESOURCE, session);
674674
}
675675
catch (IOException e) {
676+
session.close();
676677
throw new MessageHandlingException(requestMessage,
677678
"Error handling message in the [" + this
678679
+ "]. Failed to get the remote file [" + remoteFilePath + "] as a stream", e);

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,13 @@
191191
auto-create-directory="true"
192192
remote-file-separator="/" />
193193

194-
<int-sftp:outbound-gateway session-factory="sftpSessionFactory"
194+
<bean id="cachingSessionFactory" class="org.springframework.integration.file.remote.session.CachingSessionFactory">
195+
<constructor-arg name="sessionFactory" ref="sftpSessionFactory"/>
196+
<constructor-arg name="sessionCacheSize" value="1"/>
197+
<property name="sessionWaitTimeout" value="100"/>
198+
</bean>
199+
200+
<int-sftp:outbound-gateway session-factory="cachingSessionFactory"
195201
request-channel="inboundGetStream"
196202
command="get"
197203
command-options="-stream"

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
import org.springframework.integration.test.util.TestUtils;
7171
import org.springframework.messaging.Message;
7272
import org.springframework.messaging.MessageChannel;
73+
import org.springframework.messaging.MessageHandlingException;
7374
import org.springframework.messaging.MessagingException;
7475
import org.springframework.messaging.PollableChannel;
7576
import org.springframework.messaging.support.GenericMessage;
@@ -273,7 +274,7 @@ public void testInt3172LocalDirectoryExpressionMGETRecursive() throws IOExceptio
273274

274275
@Test
275276
@SuppressWarnings("unchecked")
276-
void testLSRecursive() throws IOException {
277+
void testLSRecursive() {
277278
String dir = "sftpSource/";
278279
this.inboundLSRecursive.send(new GenericMessage<Object>(dir));
279280
Message<?> result = this.output.receive(1000);
@@ -291,7 +292,7 @@ void testLSRecursive() throws IOException {
291292

292293
@Test
293294
@SuppressWarnings("unchecked")
294-
void testLSRecursiveALL() throws IOException {
295+
void testLSRecursiveALL() {
295296
String dir = "sftpSource/";
296297
this.inboundLSRecursiveALL.send(new GenericMessage<Object>(dir));
297298
Message<?> result = this.output.receive(1000);
@@ -481,7 +482,7 @@ public void testInt3088MPutNotRecursive() throws Exception {
481482
while (output.receive(0) != null) {
482483
// drain
483484
}
484-
this.inboundMPut.send(new GenericMessage<File>(getSourceLocalDirectory()));
485+
this.inboundMPut.send(new GenericMessage<>(getSourceLocalDirectory()));
485486
@SuppressWarnings("unchecked")
486487
Message<List<String>> out = (Message<List<String>>) this.output.receive(1000);
487488
assertThat(out).isNotNull();
@@ -655,6 +656,15 @@ public void testStream() {
655656
.containsEntry(FileHeaders.REMOTE_DIRECTORY, "sftpSource/")
656657
.containsEntry(FileHeaders.REMOTE_FILE, " sftpSource1.txt");
657658
verify(session).close();
659+
660+
assertThatExceptionOfType(MessageHandlingException.class)
661+
.isThrownBy(() -> this.inboundGetStream.send(new GenericMessage<>(dir + "doesNotExist.txt")))
662+
.withStackTraceContaining("No such file or directory");
663+
664+
// No leak for not closed session after the previous failure
665+
assertThatExceptionOfType(MessageHandlingException.class)
666+
.isThrownBy(() -> this.inboundGetStream.send(new GenericMessage<>(dir + "doesNotExist.txt")))
667+
.withStackTraceContaining("No such file or directory");
658668
}
659669

660670
@Test

0 commit comments

Comments
 (0)