Skip to content

Commit

Permalink
Use bean CL for JdbcMessageStore.deserializer
Browse files Browse the repository at this point in the history
Related to https://stackoverflow.com/questions/72305387/spring-integration-delayer-starts-sending-null-message-payloads-when-switched

In some async use-cases (e.g. `DelayHandler`), the context classloader
might be different for the data to be deserialized from message store.

* Fix `JdbcMessageStore` to populate a bean `ClassLoader` into default
`AllowListDeserializingConverter` from the application context.
The provided `Deserializer` must ensure such a `ClassLoader` itself
* Add warning message to the `LambdaMessageProcessor` when converter
returns `null` for the payload it cannot convert to expected type.
Cannot be raised as error since some applications may already rely
on the `null` conversion result in their method arguments

**Cherry-pick to `5.5.x`**
  • Loading branch information
artembilan authored and garyrussell committed Jun 1, 2022
1 parent cdcc986 commit 4f49038
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.core.MethodIntrospector;
import org.springframework.core.log.LogMessage;
import org.springframework.integration.context.IntegrationContextUtils;
import org.springframework.lang.Nullable;
import org.springframework.messaging.Message;
Expand Down Expand Up @@ -151,7 +152,14 @@ else if (Map.class.isAssignableFrom(parameterType)) {
args[i] = message;
}
else {
args[i] = this.messageConverter.fromMessage(message, this.expectedType);
Object payload = this.messageConverter.fromMessage(message, this.expectedType);
if (payload == null && LOGGER.isWarnEnabled()) {
LOGGER.warn(LogMessage.format(
"The '%s' returned 'null' for the payload conversion from the " +
"'%s' and expected type '%s'.",
this.messageConverter, message, this.expectedType));
}
args[i] = payload;
}
}
else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
* classes/packages are deserialized. If you receive data from untrusted sources, consider
* adding trusted classes/packages using {@link #setAllowedPatterns(String...)} or
* {@link #addAllowedPatterns(String...)}.
* <p>
* If a delegate deserializer is a {@link DefaultDeserializer}, only its {@link ClassLoader}
* is used for a {@link ConfigurableObjectInputStream} logic.
*
* @author Gary Russell
* @author Mark Fisher
Expand Down Expand Up @@ -132,7 +135,13 @@ public Object convert(byte[] source) {
return deserialize(byteStream);
}
else {
return this.deserializer.deserialize(byteStream);
Object result = this.deserializer.deserialize(byteStream);
/* Even if there is no knowledge what is the target deserialization algorithm
and malicious code may be executed already, it is still better to fail
with untrusted data rather than just let it pass downstream.
*/
checkAllowList(result.getClass());
return result;
}
}
catch (Exception ex) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2016-2021 the original author or authors.
* Copyright 2016-2022 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 @@ -19,6 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

import java.util.Date;
import java.util.Objects;
import java.util.function.Function;

Expand All @@ -32,6 +33,7 @@
import org.springframework.core.convert.converter.Converter;
import org.springframework.integration.config.EnableIntegration;
import org.springframework.integration.config.IntegrationConverter;
import org.springframework.integration.core.GenericSelector;
import org.springframework.integration.handler.GenericHandler;
import org.springframework.integration.handler.LambdaMessageProcessor;
import org.springframework.integration.transformer.GenericTransformer;
Expand Down Expand Up @@ -86,6 +88,24 @@ public void testMessageAsArgumentLambda() {
.isThrownBy(() -> lmp.processMessage(testMessage));
}

@Test
public void testConversionToNull() {
LambdaMessageProcessor lmp = new LambdaMessageProcessor(
new GenericSelector<Date>() { // Must not be lambda

@Override
public boolean accept(Date payload) {
return payload == null;
}

}, Date.class);
lmp.setBeanFactory(this.beanFactory);
GenericMessage<String> testMessage = new GenericMessage<>("foo");
Object result = lmp.processMessage(testMessage);
assertThat(result).isEqualTo(Boolean.TRUE);
}


@Test
@Disabled("Until https://github.com/spring-projects/spring-integration/issues/3660")
public void testCustomConverter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@

import javax.sql.DataSource;

import org.springframework.beans.factory.BeanClassLoaderAware;
import org.springframework.core.serializer.Deserializer;
import org.springframework.core.serializer.Serializer;
import org.springframework.core.serializer.support.SerializingConverter;
Expand Down Expand Up @@ -79,7 +80,7 @@
*
* @since 2.0
*/
public class JdbcMessageStore extends AbstractMessageGroupStore implements MessageStore {
public class JdbcMessageStore extends AbstractMessageGroupStore implements MessageStore, BeanClassLoaderAware {

/**
* Default value for the table prefix property.
Expand Down Expand Up @@ -177,7 +178,10 @@ public String getSql() {

private String tablePrefix = DEFAULT_TABLE_PREFIX;

private AllowListDeserializingConverter deserializer;
private AllowListDeserializingConverter deserializer =
new AllowListDeserializingConverter(JdbcMessageStore.class.getClassLoader());

private boolean deserializerExplicitlySet;

private SerializingConverter serializer;

Expand All @@ -199,7 +203,6 @@ public JdbcMessageStore(DataSource dataSource) {
public JdbcMessageStore(JdbcOperations jdbcOperations) {
Assert.notNull(jdbcOperations, "'dataSource' must not be null");
this.jdbcTemplate = jdbcOperations;
this.deserializer = new AllowListDeserializingConverter();
this.serializer = new SerializingConverter();
try {
this.vendorName =
Expand All @@ -211,6 +214,13 @@ public JdbcMessageStore(JdbcOperations jdbcOperations) {
}
}

@Override
public void setBeanClassLoader(ClassLoader classLoader) {
if (!this.deserializerExplicitlySet) {
this.deserializer = new AllowListDeserializingConverter(classLoader);
}
}

/**
* Public setter for the table prefix property. This will be prefixed to all the table names before queries are
* executed. Defaults to {@link #DEFAULT_TABLE_PREFIX}.
Expand Down Expand Up @@ -249,12 +259,13 @@ public void setSerializer(Serializer<? super Message<?>> serializer) {
}

/**
* A converter for deserializing byte arrays to messages.
* A converter for deserializing byte arrays to message.
* @param deserializer the deserializer to set
*/
@SuppressWarnings({ "unchecked", "rawtypes" })
public void setDeserializer(Deserializer<? extends Message<?>> deserializer) {
this.deserializer = new AllowListDeserializingConverter((Deserializer) deserializer);
this.deserializerExplicitlySet = true;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2022 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 @@ -35,6 +35,7 @@
import org.springframework.integration.store.SimpleMessageGroup;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.integration.test.condition.LongRunningTest;
import org.springframework.integration.test.util.TestUtils;
import org.springframework.integration.util.UUIDConverter;
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabase;
import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
Expand Down Expand Up @@ -80,6 +81,10 @@ public void testDelayerHandlerRescheduleWithJdbcMessageStore() throws Exception
MessageChannel input = context.getBean("input", MessageChannel.class);
MessageGroupStore messageStore = context.getBean("messageStore", MessageGroupStore.class);

ClassLoader messageStoreDeserializerClassLoader =
TestUtils.getPropertyValue(messageStore, "deserializer.defaultDeserializerClassLoader",
ClassLoader.class);
assertThat(messageStoreDeserializerClassLoader).isSameAs(context.getClassLoader());
assertThat(messageStore.getMessageGroupCount()).isEqualTo(0);
Message<String> message1 = MessageBuilder.withPayload("test1").build();
input.send(message1);
Expand Down

0 comments on commit 4f49038

Please sign in to comment.