From 9039a1b4c546ba8053dac752a0fa0cb68ae1fc36 Mon Sep 17 00:00:00 2001
From: Artem Bilan <abilan@pivotal.io>
Date: Wed, 21 Oct 2015 21:47:06 -0400
Subject: [PATCH] INT-3859: Fix `NPE` in the `ImapMailReceiver`

JIRA: https://jira.spring.io/browse/INT-3859

The Java Mail `MessageCache.getMessageBySeqnum()` has the code:
````java
if (msgnum < 0) {		// XXX - < 1 ?
    if (logger.isLoggable(Level.FINE))
	logger.fine("no message seqnum " + seqnum);
        return null;
}
````
about which the `IMAPFolder.search` doesn't care:
````java
matchMsgs[i] = getMessageBySeqNumber(matches[i]);
````
therefore pops `null`s to the top for the `ImapMailReceiver`

* Fix `NPE` filtering the `Message[]` from `null` items

Note: its enough difficult to reproduce it because it isn't clear how we can end up with:
````java
if (seqnums[msgnum-1] > seqnum)
		break;		// message doesn't exist
````

in the `MessageCache`.
That's why there is no test-cases on the matter.

**Cherry-pick to 4.1.x**

Polishing

- Only create a new array if needed.
- Add a test case
---
 .../integration/mail/ImapMailReceiver.java    | 29 +++++++++++-
 .../mail/ImapMailReceiverTests.java           | 47 +++++++++++++++++++
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
index 9b764b0ade..9b1cf1d4d8 100755
--- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
+++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2014 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -16,7 +16,9 @@
 
 package org.springframework.integration.mail;
 
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.List;
 import java.util.Properties;
 import java.util.concurrent.ScheduledFuture;
 
@@ -197,11 +199,34 @@ public class ImapMailReceiver extends AbstractMailReceiver {
 		SearchTerm searchTerm = this.compileSearchTerms(supportedFlags);
 		Folder folder = this.getFolder();
 		if (folder.isOpen()) {
-			return searchTerm != null ? folder.search(searchTerm) : folder.getMessages();
+			return nullSafeMessages(searchTerm != null ? folder.search(searchTerm) : folder.getMessages());
 		}
 		throw new MessagingException("Folder is closed");
 	}
 
+	// INT-3859
+	private Message[] nullSafeMessages(Message[] messageArray) {
+		boolean hasNulls = false;
+		for (Message message : messageArray) {
+			if (message == null) {
+				hasNulls = true;
+				break;
+			}
+		}
+		if (!hasNulls) {
+			return messageArray;
+		}
+		else {
+			List<Message> messages = new ArrayList<Message>();
+			for (Message message : messageArray) {
+				if (message != null) {
+					messages.add(message);
+				}
+			}
+			return messages.toArray(new Message[messages.size()]);
+		}
+	}
+
 	private SearchTerm compileSearchTerms(Flags supportedFlags) {
 		return this.searchTermStrategy.generateSearchTerm(supportedFlags, this.getFolder());
 	}
diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
index 08361bbb62..2131835739 100644
--- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
+++ b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java
@@ -870,6 +870,53 @@ public class ImapMailReceiverTests {
 		assertTrue(exec.isShutdown());
 	}
 
+	@Test
+	public void testNullMessages() throws Exception {
+		Message message1 = mock(Message.class);
+		Message message2 = mock(Message.class);
+		final Message[] messages1 = new Message[] { null, null, message1 };
+		final Message[] messages2 = new Message[] { message2 };
+		final SearchTermStrategy searchTermStrategy = mock(SearchTermStrategy.class);
+		class TestReceiver extends ImapMailReceiver {
+
+			private boolean firstDone;
+
+			public TestReceiver() {
+				setSearchTermStrategy(searchTermStrategy);
+			}
+
+			@Override
+			protected Folder getFolder() {
+				Folder folder = mock(Folder.class);
+				when(folder.isOpen()).thenReturn(true);
+				try {
+					when(folder.getMessages())
+							.thenReturn(!this.firstDone ? messages1 : messages2);
+				}
+				catch (MessagingException e) {
+				}
+				return folder;
+			}
+
+			@Override
+			public Message[] receive() throws MessagingException {
+				Message[] messages = searchForNewMessages();
+				this.firstDone = true;
+				return messages;
+			}
+
+
+		};
+		ImapMailReceiver receiver = new TestReceiver();
+		Message[] received = receiver.receive();
+		assertEquals(1, received.length);
+		assertSame(message1, received[0]);
+		received = receiver.receive();
+		assertEquals(1, received.length);
+		assertSame(messages2, received);
+		assertSame(message2, received[0]);
+	}
+
 	private void setUpScheduler(ImapMailReceiver mailReceiver, ThreadPoolTaskScheduler taskScheduler) {
 		taskScheduler.setPoolSize(5);
 		taskScheduler.initialize();