Skip to content

Commit 62a24af

Browse files
garyrussellartembilan
authored andcommitted
GH-1339: Fix NPEs with returns after conversion ex
Resolves #1339 Previous fix calls the error handler after a conversion exception, but sending a reply returned by the eh (or returning the exception) caused NPEs because the return logic referenced the inbound converted message. **cherry-pick to 2.2.x**
1 parent 29c6673 commit 62a24af

File tree

3 files changed

+75
-11
lines changed

3 files changed

+75
-11
lines changed

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/AbstractAdaptableMessageListener.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2020 the original author or authors.
2+
* Copyright 2014-2021 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.
@@ -176,9 +176,11 @@ public void setResponseExchange(String responseExchange) {
176176
* that return result objects, which will be wrapped in
177177
* a response message and sent to a response destination.
178178
* <p>
179+
* It is parsed in {@link Address} so should be of the form exchange/rk.
180+
* <p>
179181
* It can be a string surrounded by "!{...}" in which case the expression is
180182
* evaluated at runtime; see the reference manual for more information.
181-
* @param defaultReplyTo The exchange.
183+
* @param defaultReplyTo The replyTo address.
182184
* @since 1.6
183185
*/
184186
public void setResponseAddress(String defaultReplyTo) {

spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/adapter/MessagingMessageListenerAdapter.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,11 @@ private void handleException(org.springframework.amqp.core.Message amqpMessage,
161161
}
162162
Object errorResult = this.errorHandler.handleError(amqpMessage, messageWithChannel, e);
163163
if (errorResult != null) {
164-
handleResult(this.handlerAdapter.getInvocationResultFor(errorResult, message.getPayload()),
165-
amqpMessage, channel, message);
164+
Object payload = message == null ? null : message.getPayload();
165+
InvocationResult invResult = payload == null
166+
? new InvocationResult(errorResult, null, null, null, null)
167+
: this.handlerAdapter.getInvocationResultFor(errorResult, payload);
168+
handleResult(invResult, amqpMessage, channel, message);
166169
}
167170
else {
168171
logger.trace("Error handler returned no result");
@@ -203,15 +206,17 @@ private void returnOrThrow(org.springframework.amqp.core.Message amqpMessage, Ch
203206
if (!this.returnExceptions) {
204207
throw exceptionToThrow;
205208
}
209+
Object payload = message == null ? null : message.getPayload();
206210
try {
211+
207212
handleResult(new InvocationResult(new RemoteInvocationResult(throwableToReturn), null,
208-
this.handlerAdapter.getReturnTypeFor(message.getPayload()),
213+
payload == null ? Object.class : this.handlerAdapter.getReturnTypeFor(payload),
209214
this.handlerAdapter.getBean(),
210-
this.handlerAdapter.getMethodFor(message.getPayload())),
215+
payload == null ? null : this.handlerAdapter.getMethodFor(payload)),
211216
amqpMessage, channel, message);
212217
}
213218
catch (ReplyFailureException rfe) {
214-
if (void.class.equals(this.handlerAdapter.getReturnTypeFor(message.getPayload()))) {
219+
if (payload == null || void.class.equals(this.handlerAdapter.getReturnTypeFor(payload))) {
215220
throw exceptionToThrow;
216221
}
217222
else {

spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/adapter/MessagingMessageListenerAdapterTests.java

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.assertj.core.api.Assertions.fail;
2121
import static org.mockito.ArgumentMatchers.any;
22+
import static org.mockito.ArgumentMatchers.anyBoolean;
2223
import static org.mockito.ArgumentMatchers.eq;
2324
import static org.mockito.Mockito.mock;
2425
import static org.mockito.Mockito.verify;
@@ -293,7 +294,7 @@ public Object handleError(org.springframework.amqp.core.Message amqpMessage, Mes
293294
return null;
294295
}
295296

296-
}, String.class);
297+
}, false, String.class);
297298
listener.setMessageConverter(new MessageConverter() {
298299

299300
@Override
@@ -312,15 +313,71 @@ public Object fromMessage(org.springframework.amqp.core.Message message) throws
312313
assertThat(ehCalled.get()).isTrue();
313314
}
314315

316+
@Test
317+
void errorHandlerAfterConversionExWithResult() throws Exception {
318+
org.springframework.amqp.core.Message message = MessageTestUtils.createTextMessage("foo");
319+
Channel channel = mock(Channel.class);
320+
AtomicBoolean ehCalled = new AtomicBoolean();
321+
MessagingMessageListenerAdapter listener = getSimpleInstance("fail",
322+
new RabbitListenerErrorHandler() {
323+
324+
@Override
325+
public Object handleError(org.springframework.amqp.core.Message amqpMessage, Message<?> message,
326+
ListenerExecutionFailedException exception) throws Exception {
327+
328+
ehCalled.set(true);
329+
return "foo";
330+
}
331+
332+
}, false, String.class);
333+
listener.setMessageConverter(new MessageConverter() {
334+
335+
@Override
336+
public org.springframework.amqp.core.Message toMessage(Object object, MessageProperties messageProperties)
337+
throws MessageConversionException {
338+
339+
return new org.springframework.amqp.core.Message(((String) object).getBytes(), messageProperties);
340+
}
341+
342+
@Override
343+
public Object fromMessage(org.springframework.amqp.core.Message message) throws MessageConversionException {
344+
throw new MessageConversionException("test");
345+
}
346+
});
347+
listener.setResponseAddress("foo/bar");
348+
listener.onMessage(message, channel);
349+
verify(channel).basicPublish(any(), any(), anyBoolean(), any(), any());
350+
assertThat(ehCalled.get()).isTrue();
351+
}
352+
353+
@Test
354+
void errorHandlerAfterConversionExWithReturnEx() throws Exception {
355+
org.springframework.amqp.core.Message message = MessageTestUtils.createTextMessage("foo");
356+
Channel channel = mock(Channel.class);
357+
MessagingMessageListenerAdapter listener = getSimpleInstance("fail", null, true, String.class);
358+
class MC extends SimpleMessageConverter {
359+
360+
@Override
361+
public Object fromMessage(org.springframework.amqp.core.Message message) throws MessageConversionException {
362+
throw new MessageConversionException("test");
363+
}
364+
365+
}
366+
listener.setMessageConverter(new MC());
367+
listener.setResponseAddress("foo/bar");
368+
listener.onMessage(message, channel);
369+
verify(channel).basicPublish(any(), any(), anyBoolean(), any(), any());
370+
}
371+
315372
protected MessagingMessageListenerAdapter getSimpleInstance(String methodName, Class<?>... parameterTypes) {
316-
return getSimpleInstance(methodName, null, parameterTypes);
373+
return getSimpleInstance(methodName, null, false, parameterTypes);
317374
}
318375

319376
protected MessagingMessageListenerAdapter getSimpleInstance(String methodName, RabbitListenerErrorHandler eh,
320-
Class<?>... parameterTypes) {
377+
boolean returnEx, Class<?>... parameterTypes) {
321378

322379
Method m = ReflectionUtils.findMethod(SampleBean.class, methodName, parameterTypes);
323-
return createInstance(m, false, eh);
380+
return createInstance(m, returnEx, eh);
324381
}
325382

326383
protected MessagingMessageListenerAdapter getSimpleInstance(String methodName, boolean returnExceptions,

0 commit comments

Comments
 (0)