Skip to content

Commit c8a2894

Browse files
mjd507mjd507
committed
GH-10101: Mitigate constant access to the nullable fields in AbstractMailReceiver.
Fixes: #10101 Signed-off-by: Jiandong Ma <[email protected]>
1 parent cbb0c6f commit c8a2894

File tree

7 files changed

+103
-91
lines changed

7 files changed

+103
-91
lines changed

‎spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java‎

Lines changed: 58 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -316,65 +316,78 @@ protected int getFolderOpenMode() {
316316

317317
/**
318318
* Subclasses must implement this method to return new mail messages.
319+
* @param folder the folder for fetching mail messages.
319320
* @return An array of messages.
320321
* @throws MessagingException Any MessagingException.
321322
*/
322-
protected abstract Message[] searchForNewMessages() throws MessagingException;
323+
protected abstract Message[] searchForNewMessages(Folder folder) throws MessagingException;
323324

324-
private void openSession() {
325-
if (this.session == null) {
325+
private Session openSession() {
326+
Session sessionToUse = this.session;
327+
if (sessionToUse == null) {
326328
if (this.javaMailAuthenticator != null) {
327-
this.session = Session.getInstance(this.javaMailProperties, this.javaMailAuthenticator);
329+
sessionToUse = Session.getInstance(this.javaMailProperties, this.javaMailAuthenticator);
328330
}
329331
else {
330-
this.session = Session.getInstance(this.javaMailProperties);
332+
sessionToUse = Session.getInstance(this.javaMailProperties);
331333
}
332334
}
335+
this.session = sessionToUse;
336+
return sessionToUse;
333337
}
334338

335-
private void connectStoreIfNecessary() throws MessagingException {
336-
if (this.store == null) {
339+
private Store connectStoreIfNecessary(Session session) throws MessagingException {
340+
Store storeToUse = this.store;
341+
if (storeToUse == null) {
337342
if (this.url != null) {
338-
this.store = this.session.getStore(this.url);
343+
storeToUse = session.getStore(this.url);
339344
}
340345
else if (this.protocol != null) {
341-
this.store = this.session.getStore(this.protocol);
346+
storeToUse = session.getStore(this.protocol);
342347
}
343348
else {
344-
this.store = this.session.getStore();
349+
storeToUse = session.getStore();
345350
}
346351
}
347-
if (!this.store.isConnected()) {
348-
this.logger.debug(() -> "connecting to store [" + this.store.getURLName() + "]");
349-
this.store.connect();
352+
if (!storeToUse.isConnected()) {
353+
URLName urlName = storeToUse.getURLName();
354+
this.logger.debug(() -> "connecting to store [" + urlName + "]");
355+
storeToUse.connect();
350356
}
357+
this.store = storeToUse;
358+
return storeToUse;
351359
}
352360

353-
protected void openFolder() throws MessagingException {
354-
if (this.folder == null) {
355-
openSession();
356-
connectStoreIfNecessary();
357-
this.folder = obtainFolderInstance();
361+
protected Folder openFolder() throws MessagingException {
362+
Folder folderToUse = this.folder;
363+
if (folderToUse == null) {
364+
Session session = openSession();
365+
Store storeToUse = connectStoreIfNecessary(session);
366+
folderToUse = obtainFolderInstance(storeToUse);
358367
}
359368
else {
360-
connectStoreIfNecessary();
369+
connectStoreIfNecessary(this.session);
361370
}
362-
if (this.folder == null || !this.folder.exists()) {
363-
throw new IllegalStateException("no such folder [" + this.url.getFile() + "]");
371+
if (folderToUse == null || !folderToUse.exists()) {
372+
String file = this.url != null ? this.url.getFile() : "";
373+
throw new IllegalStateException("no such folder [" + file + "]");
364374
}
365-
if (this.folder.isOpen()) {
366-
return;
375+
if (folderToUse.isOpen()) {
376+
this.folder = folderToUse;
377+
return folderToUse;
367378
}
368-
URLName urlName = this.folder.getURLName();
379+
URLName urlName = folderToUse.getURLName();
369380
this.logger.debug(() -> "opening folder [" + urlName + "]");
370-
this.folder.open(this.folderOpenMode);
381+
folderToUse.open(this.folderOpenMode);
382+
this.folder = folderToUse;
383+
return folderToUse;
371384
}
372385

373-
private Folder obtainFolderInstance() throws MessagingException {
386+
private Folder obtainFolderInstance(Store store) throws MessagingException {
374387
if (this.url == null) {
375-
return this.store.getDefaultFolder();
388+
return store.getDefaultFolder();
376389
}
377-
return this.store.getFolder(this.url);
390+
return store.getFolder(this.url);
378391
}
379392

380393
@Override
@@ -388,14 +401,14 @@ public Object[] receive() throws jakarta.mail.MessagingException {
388401
this.folderReadLock.unlock();
389402
this.folderWriteLock.lock();
390403
try {
391-
openFolder();
404+
folderToCheck = openFolder();
392405
this.folderReadLock.lock();
393406
}
394407
finally {
395408
this.folderWriteLock.unlock();
396409
}
397410
}
398-
messagesToReturn = convertMessagesIfNecessary(searchAndFilterMessages());
411+
messagesToReturn = convertMessagesIfNecessary(searchAndFilterMessages(folderToCheck));
399412
return messagesToReturn;
400413
}
401414
finally {
@@ -421,10 +434,10 @@ protected void closeFolder() {
421434
}
422435
}
423436

424-
private MimeMessage[] searchAndFilterMessages() throws MessagingException {
425-
this.logger.debug(() -> "attempting to receive mail from folder [" + this.folder.getFullName() + "]");
437+
private MimeMessage[] searchAndFilterMessages(Folder folder) throws MessagingException {
438+
this.logger.debug(() -> "attempting to receive mail from folder [" + folder.getFullName() + "]");
426439
Message[] messagesToProcess;
427-
Message[] messages = searchForNewMessages();
440+
Message[] messages = searchForNewMessages(folder);
428441
if (this.maxFetchSize > 0 && messages.length > this.maxFetchSize) {
429442
Message[] reducedMessages = new Message[this.maxFetchSize];
430443
System.arraycopy(messages, 0, reducedMessages, 0, this.maxFetchSize);
@@ -435,14 +448,14 @@ private MimeMessage[] searchAndFilterMessages() throws MessagingException {
435448
}
436449
this.logger.debug(() -> "found " + messagesToProcess.length + " new messages");
437450
if (messagesToProcess.length > 0) {
438-
fetchMessages(messagesToProcess);
451+
fetchMessages(messagesToProcess, folder);
439452
}
440453

441454
this.logger.debug(() -> "Received " + messagesToProcess.length + " messages");
442455

443456
MimeMessage[] filteredMessages = filterMessagesThruSelector(messagesToProcess);
444457

445-
postProcessFilteredMessages(filteredMessages);
458+
postProcessFilteredMessages(filteredMessages, folder);
446459
return filteredMessages;
447460
}
448461

@@ -519,7 +532,7 @@ private Object byteArrayToContent(Map<String, Object> headers, ByteArrayOutputSt
519532
return baos.toByteArray();
520533
}
521534

522-
private void postProcessFilteredMessages(Message[] filteredMessages) throws MessagingException {
535+
private void postProcessFilteredMessages(Message[] filteredMessages, Folder folder) throws MessagingException {
523536
// Copy messages to cause an eager fetch
524537
Message[] messages = filteredMessages;
525538
if (this.headerMapper == null && (this.autoCloseFolder || this.simpleContent)) {
@@ -532,25 +545,23 @@ private void postProcessFilteredMessages(Message[] filteredMessages) throws Mess
532545
}
533546
}
534547

535-
setMessageFlagsAndMaybeDeleteMessages(messages);
548+
setMessageFlagsAndMaybeDeleteMessages(messages, folder);
536549
if (filteredMessages.length > 0 && filteredMessages[0] instanceof IntegrationMimeMessage) {
537-
setMessageFlagsAndMaybeDeleteMessages(filteredMessages);
550+
setMessageFlagsAndMaybeDeleteMessages(filteredMessages, folder);
538551
}
539552
}
540553

541-
private void setMessageFlagsAndMaybeDeleteMessages(Message[] messages) throws MessagingException {
542-
setMessageFlags(messages);
554+
private void setMessageFlagsAndMaybeDeleteMessages(Message[] messages, Folder folder) throws MessagingException {
555+
setMessageFlags(messages, folder.getPermanentFlags());
543556

544557
if (shouldDeleteMessages()) {
545558
deleteMessages(messages);
546559
}
547560
}
548561

549-
private void setMessageFlags(Message[] filteredMessages) throws MessagingException {
562+
private void setMessageFlags(Message[] filteredMessages, Flags flags) throws MessagingException {
550563
boolean recentFlagSupported = false;
551564

552-
Flags flags = getFolder().getPermanentFlags();
553-
554565
if (flags != null) {
555566
recentFlagSupported = flags.contains(Flags.Flag.RECENT);
556567
}
@@ -605,14 +616,15 @@ private MimeMessage[] filterMessagesThruSelector(Message[] messages) throws Mess
605616
* implementation {@link Folder#fetch(Message[], FetchProfile) fetches}
606617
* every {@link jakarta.mail.FetchProfile.Item}.
607618
* @param messages the messages to fetch
619+
* @param folder the folder to fetch from
608620
* @throws MessagingException in case of JavaMail errors
609621
*/
610-
protected void fetchMessages(Message[] messages) throws MessagingException {
622+
protected void fetchMessages(Message[] messages, Folder folder) throws MessagingException {
611623
FetchProfile contentsProfile = new FetchProfile();
612624
contentsProfile.add(FetchProfile.Item.ENVELOPE);
613625
contentsProfile.add(FetchProfile.Item.CONTENT_INFO);
614626
contentsProfile.add(FetchProfile.Item.FLAGS);
615-
this.folder.fetch(messages, contentsProfile);
627+
folder.fetch(messages, contentsProfile);
616628
}
617629

618630
/**
@@ -706,7 +718,7 @@ public Folder getFolder() {
706718
}
707719
else {
708720
try {
709-
return obtainFolderInstance();
721+
return obtainFolderInstance(AbstractMailReceiver.this.store);
710722
}
711723
catch (MessagingException e) {
712724
throw new org.springframework.messaging.MessagingException("Unable to obtain the mail folder", e);

‎spring-integration-mail/src/main/java/org/springframework/integration/mail/ImapMailReceiver.java‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
* @author Gary Russell
5353
* @author Artem Bilan
5454
* @author Alexander Pinske
55+
* @author Jiandong Ma
5556
*/
5657
public class ImapMailReceiver extends AbstractMailReceiver {
5758

@@ -174,15 +175,14 @@ public void cancelPing() {
174175
* @throws MessagingException Any MessagingException.
175176
*/
176177
public void waitForNewMessages() throws MessagingException {
177-
openFolder();
178-
Folder folder = getFolder();
178+
Folder folder = openFolder();
179179
Assert.state(folder instanceof IMAPFolder,
180180
() -> "folder is not an instance of [" + IMAPFolder.class.getName() + "]");
181181
IMAPFolder imapFolder = (IMAPFolder) folder;
182182
if (imapFolder.hasNewMessages()) {
183183
return;
184184
}
185-
else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT) && searchForNewMessages().length > 0) {
185+
else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT) && searchForNewMessages(folder).length > 0) {
186186
return;
187187
}
188188
try {
@@ -210,12 +210,11 @@ else if (!folder.getPermanentFlags().contains(Flags.Flag.RECENT) && searchForNew
210210
* @throws MessagingException in case of JavaMail errors
211211
*/
212212
@Override
213-
protected Message[] searchForNewMessages() throws MessagingException {
214-
Folder folderToUse = getFolder();
215-
Flags supportedFlags = folderToUse.getPermanentFlags();
216-
SearchTerm searchTerm = compileSearchTerms(supportedFlags);
217-
if (folderToUse.isOpen()) {
218-
return nullSafeMessages(searchTerm != null ? folderToUse.search(searchTerm) : folderToUse.getMessages());
213+
protected Message[] searchForNewMessages(Folder folder) throws MessagingException {
214+
Flags supportedFlags = folder.getPermanentFlags();
215+
SearchTerm searchTerm = compileSearchTerms(supportedFlags, folder);
216+
if (folder.isOpen()) {
217+
return nullSafeMessages(searchTerm != null ? folder.search(searchTerm) : folder.getMessages());
219218
}
220219
throw new MessagingException("Folder is closed");
221220
}
@@ -238,8 +237,8 @@ private Message[] nullSafeMessages(Message[] messageArray) {
238237
}
239238
}
240239

241-
private SearchTerm compileSearchTerms(Flags supportedFlags) {
242-
return this.searchTermStrategy.generateSearchTerm(supportedFlags, this.getFolder());
240+
private SearchTerm compileSearchTerms(Flags supportedFlags, Folder folder) {
241+
return this.searchTermStrategy.generateSearchTerm(supportedFlags, folder);
243242
}
244243

245244
@Override

‎spring-integration-mail/src/main/java/org/springframework/integration/mail/Pop3MailReceiver.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
*
3131
* @author Arjen Poutsma
3232
* @author Mark Fisher
33+
* @author Jiandong Ma
3334
*/
3435
public class Pop3MailReceiver extends AbstractMailReceiver {
3536

@@ -61,13 +62,12 @@ public Pop3MailReceiver(String host, int port, String username, String password)
6162
}
6263

6364
@Override
64-
protected Message[] searchForNewMessages() throws MessagingException {
65-
Folder folderToUse = getFolder();
66-
int messageCount = folderToUse.getMessageCount();
65+
protected Message[] searchForNewMessages(Folder folder) throws MessagingException {
66+
int messageCount = folder.getMessageCount();
6767
if (messageCount == 0) {
6868
return new Message[0];
6969
}
70-
return folderToUse.getMessages();
70+
return folder.getMessages();
7171
}
7272

7373
/**

‎spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@
111111
* @author Alexander Pinske
112112
* @author Dominik Simmen
113113
* @author Filip Hrisafov
114+
* @author Jiandong Ma
114115
*/
115116
@SpringJUnitConfig
116117
@ContextConfiguration(
@@ -334,7 +335,7 @@ private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailRece
334335

335336
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
336337

337-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
338+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
338339
if (receive) {
339340
receiver.receive();
340341
}
@@ -443,7 +444,7 @@ public void receiveMarkAsReadAndDelete() throws Exception {
443444

444445
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
445446

446-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
447+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
447448
receiver.receive();
448449

449450
assertThat(msg1.getFlags().contains(Flag.SEEN)).isTrue();
@@ -474,7 +475,7 @@ public void receiveAndDontMarkAsRead() throws Exception {
474475

475476
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
476477

477-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
478+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
478479
receiver.afterPropertiesSet();
479480
receiver.receive();
480481
assertThat(msg1.getFlags().contains(Flag.SEEN)).isFalse();
@@ -511,7 +512,7 @@ public void receiveAndDontMarkAsReadButDelete() throws Exception {
511512

512513
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
513514

514-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
515+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
515516
receiver.afterPropertiesSet();
516517
receiver.receive();
517518

@@ -549,7 +550,7 @@ public void receiveAndIgnoreMarkAsReadDontDelete() throws Exception {
549550

550551
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
551552

552-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
553+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
553554
receiver.receive();
554555
assertThat(msg1.getFlags().contains(Flag.SEEN)).isTrue();
555556
assertThat(msg2.getFlags().contains(Flag.SEEN)).isTrue();
@@ -580,12 +581,12 @@ public void testMessageHistory() throws Exception {
580581
willAnswer(invocation -> {
581582
DirectFieldAccessor accessor = new DirectFieldAccessor((invocation.getMock()));
582583
accessor.setPropertyValue("folder", folder);
583-
return null;
584+
return folder;
584585
}).given(receiver).openFolder();
585586

586587
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
587588

588-
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
589+
willAnswer(invocation -> null).given(receiver).fetchMessages(messages, folder);
589590

590591
PollableChannel channel = this.context.getBean("channel", PollableChannel.class);
591592

@@ -926,7 +927,7 @@ protected Folder getFolder() {
926927

927928
@Override
928929
public Message[] receive() throws MessagingException {
929-
Message[] messages = searchForNewMessages();
930+
Message[] messages = searchForNewMessages(getFolder());
930931
this.firstDone = true;
931932
return messages;
932933
}

0 commit comments

Comments
 (0)