GH-2974: Fix race in TcpNetConnection.getPayload()
Fixes https://github.com/spring-projects/spring-integration/issues/2974 There is a race in that we could get a `SocketException` in `inputStream`. Since this is between payloads, it needs to be thrown as a `SoftEndOfStreamException`. * * Add javadocs to SoftEndOfStreamException * * SEOSE is now a RuntimeException (can't be an UncheckedIOException - no default CTOR
This commit is contained in:
committed by
Artem Bilan
parent
59cdc4ddae
commit
175223d0e0
@@ -125,9 +125,16 @@ public class TcpNetConnection extends TcpConnectionSupport implements Scheduling
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Object getPayload() {
|
public Object getPayload() {
|
||||||
|
InputStream inputStream;
|
||||||
|
try {
|
||||||
|
inputStream = inputStream();
|
||||||
|
}
|
||||||
|
catch (IOException e1) {
|
||||||
|
throw new SoftEndOfStreamException("Socket closed when getting input stream", e1);
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
return getDeserializer()
|
return getDeserializer()
|
||||||
.deserialize(inputStream());
|
.deserialize(inputStream);
|
||||||
}
|
}
|
||||||
catch (IOException e) {
|
catch (IOException e) {
|
||||||
throw new UncheckedIOException(e);
|
throw new UncheckedIOException(e);
|
||||||
@@ -291,6 +298,7 @@ public class TcpNetConnection extends TcpConnectionSupport implements Scheduling
|
|||||||
* @since 5.2
|
* @since 5.2
|
||||||
* @see Socket#shutdownInput()
|
* @see Socket#shutdownInput()
|
||||||
*/
|
*/
|
||||||
|
@Override
|
||||||
public void shutdownInput() throws IOException {
|
public void shutdownInput() throws IOException {
|
||||||
this.socket.shutdownInput();
|
this.socket.shutdownInput();
|
||||||
}
|
}
|
||||||
@@ -301,6 +309,7 @@ public class TcpNetConnection extends TcpConnectionSupport implements Scheduling
|
|||||||
* @since 5.2
|
* @since 5.2
|
||||||
* @see Socket#shutdownOutput()
|
* @see Socket#shutdownOutput()
|
||||||
*/
|
*/
|
||||||
|
@Override
|
||||||
public void shutdownOutput() throws IOException {
|
public void shutdownOutput() throws IOException {
|
||||||
this.socket.shutdownOutput();
|
this.socket.shutdownOutput();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -16,8 +16,6 @@
|
|||||||
|
|
||||||
package org.springframework.integration.ip.tcp.serializer;
|
package org.springframework.integration.ip.tcp.serializer;
|
||||||
|
|
||||||
import java.io.IOException;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Used to communicate that a stream has closed, but between logical
|
* Used to communicate that a stream has closed, but between logical
|
||||||
* messages.
|
* messages.
|
||||||
@@ -26,16 +24,33 @@ import java.io.IOException;
|
|||||||
* @since 2.0
|
* @since 2.0
|
||||||
*
|
*
|
||||||
*/
|
*/
|
||||||
public class SoftEndOfStreamException extends IOException {
|
public class SoftEndOfStreamException extends RuntimeException {
|
||||||
|
|
||||||
private static final long serialVersionUID = 7309907445617226978L;
|
private static final long serialVersionUID = -2209857413498073058L;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Default constructor.
|
||||||
|
*/
|
||||||
public SoftEndOfStreamException() {
|
public SoftEndOfStreamException() {
|
||||||
super();
|
super();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Construct an instance with the message.
|
||||||
|
* @param message the message.
|
||||||
|
*/
|
||||||
public SoftEndOfStreamException(String message) {
|
public SoftEndOfStreamException(String message) {
|
||||||
super(message);
|
super(message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Construct an instance with the message and cause.
|
||||||
|
* @param message the message.
|
||||||
|
* @param cause the cause.
|
||||||
|
* @since 4.3.21.
|
||||||
|
*/
|
||||||
|
public SoftEndOfStreamException(String message, Throwable cause) {
|
||||||
|
super(message, cause);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -282,8 +282,7 @@ public class TcpOutboundGatewayTests {
|
|||||||
fail("Unexpected " + e.getMessage());
|
fail("Unexpected " + e.getMessage());
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
assertThat(e.getCause()).isNotNull();
|
assertThat(e.getCause()).isInstanceOf(MessageTimeoutException.class);
|
||||||
assertThat(e.getCause() instanceof MessageTimeoutException).isTrue();
|
|
||||||
}
|
}
|
||||||
timeouts++;
|
timeouts++;
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
@@ -17,28 +17,37 @@
|
|||||||
package org.springframework.integration.ip.tcp.connection;
|
package org.springframework.integration.ip.tcp.connection;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||||
import static org.mockito.BDDMockito.given;
|
import static org.mockito.BDDMockito.given;
|
||||||
import static org.mockito.Mockito.doAnswer;
|
import static org.mockito.Mockito.doAnswer;
|
||||||
import static org.mockito.Mockito.mock;
|
import static org.mockito.Mockito.mock;
|
||||||
import static org.mockito.Mockito.when;
|
import static org.mockito.Mockito.when;
|
||||||
|
|
||||||
import java.io.ByteArrayOutputStream;
|
import java.io.ByteArrayOutputStream;
|
||||||
|
import java.io.IOException;
|
||||||
import java.io.InputStream;
|
import java.io.InputStream;
|
||||||
import java.io.PipedInputStream;
|
import java.io.PipedInputStream;
|
||||||
import java.io.PipedOutputStream;
|
import java.io.PipedOutputStream;
|
||||||
import java.net.Socket;
|
import java.net.Socket;
|
||||||
import java.nio.ByteBuffer;
|
import java.nio.ByteBuffer;
|
||||||
import java.nio.channels.SocketChannel;
|
import java.nio.channels.SocketChannel;
|
||||||
|
import java.util.concurrent.CountDownLatch;
|
||||||
|
import java.util.concurrent.TimeUnit;
|
||||||
|
import java.util.concurrent.atomic.AtomicInteger;
|
||||||
import java.util.concurrent.atomic.AtomicReference;
|
import java.util.concurrent.atomic.AtomicReference;
|
||||||
|
|
||||||
|
import javax.net.SocketFactory;
|
||||||
|
|
||||||
import org.apache.commons.logging.Log;
|
import org.apache.commons.logging.Log;
|
||||||
import org.junit.Test;
|
import org.junit.Test;
|
||||||
import org.mockito.Mockito;
|
import org.mockito.Mockito;
|
||||||
|
|
||||||
import org.springframework.beans.DirectFieldAccessor;
|
import org.springframework.beans.DirectFieldAccessor;
|
||||||
|
import org.springframework.context.ApplicationEventPublisher;
|
||||||
import org.springframework.integration.ip.tcp.connection.TcpNioConnection.ChannelInputStream;
|
import org.springframework.integration.ip.tcp.connection.TcpNioConnection.ChannelInputStream;
|
||||||
import org.springframework.integration.ip.tcp.serializer.ByteArrayStxEtxSerializer;
|
import org.springframework.integration.ip.tcp.serializer.ByteArrayStxEtxSerializer;
|
||||||
import org.springframework.integration.ip.tcp.serializer.MapJsonSerializer;
|
import org.springframework.integration.ip.tcp.serializer.MapJsonSerializer;
|
||||||
|
import org.springframework.integration.ip.tcp.serializer.SoftEndOfStreamException;
|
||||||
import org.springframework.integration.support.MessageBuilder;
|
import org.springframework.integration.support.MessageBuilder;
|
||||||
import org.springframework.integration.support.converter.MapMessageConverter;
|
import org.springframework.integration.support.converter.MapMessageConverter;
|
||||||
import org.springframework.integration.test.util.TestUtils;
|
import org.springframework.integration.test.util.TestUtils;
|
||||||
@@ -137,4 +146,30 @@ public class TcpNetConnectionTests {
|
|||||||
assertThat(inboundMessage.get().getHeaders().get("bar")).isEqualTo("baz");
|
assertThat(inboundMessage.get().getHeaders().get("bar")).isEqualTo("baz");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void socketClosedNextRead() throws InterruptedException, IOException {
|
||||||
|
TcpNetServerConnectionFactory server = new TcpNetServerConnectionFactory(0);
|
||||||
|
AtomicInteger port = new AtomicInteger();
|
||||||
|
CountDownLatch latch = new CountDownLatch(1);
|
||||||
|
ApplicationEventPublisher publisher = ev -> {
|
||||||
|
if (ev instanceof TcpConnectionServerListeningEvent) {
|
||||||
|
port.set(((TcpConnectionServerListeningEvent) ev).getPort());
|
||||||
|
latch.countDown();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
server.setApplicationEventPublisher(publisher);
|
||||||
|
server.registerListener(message -> {
|
||||||
|
return false;
|
||||||
|
});
|
||||||
|
server.afterPropertiesSet();
|
||||||
|
server.start();
|
||||||
|
assertThat(latch.await(10, TimeUnit.SECONDS)).isTrue();
|
||||||
|
Socket socket = SocketFactory.getDefault().createSocket("localhost", port.get());
|
||||||
|
TcpNetConnection connection = new TcpNetConnection(socket, false, false, publisher, "socketClosedNextRead");
|
||||||
|
socket.close();
|
||||||
|
assertThatThrownBy(() -> connection.getPayload())
|
||||||
|
.isInstanceOf(SoftEndOfStreamException.class);
|
||||||
|
server.stop();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -498,6 +498,8 @@ To implement a custom serializer and deserializer pair, implement the `org.sprin
|
|||||||
When the deserializer detects a closed input stream between messages, it must throw a `SoftEndOfStreamException`; this is a signal to the framework to indicate that the close was "normal".
|
When the deserializer detects a closed input stream between messages, it must throw a `SoftEndOfStreamException`; this is a signal to the framework to indicate that the close was "normal".
|
||||||
If the stream is closed while decoding a message, some other exception should be thrown instead.
|
If the stream is closed while decoding a message, some other exception should be thrown instead.
|
||||||
|
|
||||||
|
Starting with version 5.2, `SoftEndOfStreamException` is now a `RuntimeException` instead of extending `IOException`.
|
||||||
|
|
||||||
[[caching-cf]]
|
[[caching-cf]]
|
||||||
==== TCP Caching Client Connection Factory
|
==== TCP Caching Client Connection Factory
|
||||||
|
|
||||||
|
|||||||
@@ -87,6 +87,8 @@ See <<./ip.adoc#tcp-gateways, TCP Gateways>> for more information.
|
|||||||
The client connection factories now support `connectTimeout` which causes an exception to be thrown if the connection is not established in that time.
|
The client connection factories now support `connectTimeout` which causes an exception to be thrown if the connection is not established in that time.
|
||||||
See <<./ip.adoc#tcp-connection-factory, TCP Connection Factories>> for more information.
|
See <<./ip.adoc#tcp-connection-factory, TCP Connection Factories>> for more information.
|
||||||
|
|
||||||
|
`SoftEndOfStreamException` is now a `RuntimeException` instead of extending `IOException`.
|
||||||
|
|
||||||
[[x5.2-mail]]
|
[[x5.2-mail]]
|
||||||
==== Mail Changes
|
==== Mail Changes
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user