From 12a57545690aec0a263305442769fe0cf726aa44 Mon Sep 17 00:00:00 2001
From: Michael O'Brien <mobrien@hubspot.com>
Date: Tue, 23 May 2017 11:44:29 +0100
Subject: [PATCH] Don't add null to SmtpResponse.details()

Motivation:

If the remote server returns an invalid response in the form "000 \r\n"
(i.e. a three digit code, then space, but no details), null is added
as a singletonList to the response being constructed.

This seems unexpected and it would be easier to handle an empty
details list in client code.

Modifications:

If detail is null (because frame.isReadable() returned false after
reading the separator), initialise DefaultSmtpResponse with an empty
list instead of a list containing a single null value.

Result:

When encountering this malformed server response, a DefaultSmtpResponse
with a code but no details will be created.
---
 .../handler/codec/smtp/SmtpResponseDecoder.java      |  6 +++++-
 .../handler/codec/smtp/SmtpResponseDecoderTest.java  | 12 ++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpResponseDecoder.java b/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpResponseDecoder.java
index 13587b36bc..eadea32654 100644
--- a/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpResponseDecoder.java
+++ b/codec-smtp/src/main/java/io/netty/handler/codec/smtp/SmtpResponseDecoder.java
@@ -69,7 +69,11 @@ public final class SmtpResponseDecoder extends LineBasedFrameDecoder {
                         details.add(detail);
                     }
                 } else {
-                    details = Collections.singletonList(detail);
+                    if (detail == null) {
+                        details = Collections.emptyList();
+                    } else {
+                        details = Collections.singletonList(detail);
+                    }
                 }
                 return new DefaultSmtpResponse(code, details);
             case '-':
diff --git a/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpResponseDecoderTest.java b/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpResponseDecoderTest.java
index 7ccc993acb..24044fa287 100644
--- a/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpResponseDecoderTest.java
+++ b/codec-smtp/src/test/java/io/netty/handler/codec/smtp/SmtpResponseDecoderTest.java
@@ -43,6 +43,18 @@ public class SmtpResponseDecoderTest {
         assertNull(channel.readInbound());
     }
 
+    @Test
+    public void testDecodeOneLineResponseNoDetails() {
+        EmbeddedChannel channel = newChannel();
+        assertTrue(channel.writeInbound(newBuffer("250 \r\n")));
+        assertTrue(channel.finish());
+
+        SmtpResponse response = channel.readInbound();
+        assertEquals(250, response.code());
+        List<CharSequence> sequences = response.details();
+        assertEquals(0, sequences.size());
+    }
+
     @Test
     public void testDecodeOneLineResponseChunked() {
         EmbeddedChannel channel = newChannel();