svn commit: r1918229 - in /geronimo/specs/trunk/geronimo-mail_2.1_spec/src: main/java/jakarta/mail/internet/ test/java/jakarta/mail/internet/

[email protected] Mon, 10 Jun 2024 10:49:39 -0000
Newsgroups gmane.comp.java.geronimo.cvs
Message-ID <[email protected]>
Author: jlmonteiro
Date: Mon Jun 10 10:49:39 2024
New Revision: 1918229

URL: http://svn.apache.org/viewvc?rev=1918229&view=rev
Log:
feat(GERONIMO-6869): fix toString() for InternetAddress when multiple addresses are provided
feat(GERONIMO-6870): introduce logic to avoid duplicate headers.

Modified:
    geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetAddress.java
    geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetHeaders.java
    geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/MimeMessage.java
    geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/InternetAddressTest.java
    geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/MimeMessageTest.java

Modified: geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetAddress.java
URL: http://svn.apache.org/viewvc/geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetAddress.java?rev=1918229&r1=1918228&r2=1918229&view=diff
==============================================================================
--- geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetAddress.java (original)
+++ geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetAddress.java Mon Jun 10 10:49:39 2024
@@ -448,20 +448,7 @@ public class InternetAddress extends Add
      * @return a one-line String of comma-separated addresses
      */
     public static String toString(final Address[] addresses) {
-        if (addresses == null || addresses.length == 0) {
-            return null;
-        }
-        if (addresses.length == 1) {
-            return addresses[0].toString();
-        } else {
-            final StringBuffer buf = new StringBuffer(addresses.length * 32);
-            buf.append(addresses[0].toString());
-            for (int i = 1; i < addresses.length; i++) {
-                buf.append(", ");
-                buf.append(addresses[i].toString());
-            }
-            return buf.toString();
-        }
+        return toString(addresses, 0);
     }
 
     /**
@@ -488,7 +475,7 @@ public class InternetAddress extends Add
         } else {
             final StringBuffer buf = new StringBuffer(addresses.length * 32);
             for (int i = 0; i < addresses.length; i++) {
-                final String s = addresses[1].toString();
+                final String s = addresses[i].toString();
                 if (i == 0) {
                     if (used + s.length() + 1 > 72) {
                         buf.append("\r\n  ");

Modified: geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetHeaders.java
URL: http://svn.apache.org/viewvc/geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetHeaders.java?rev=1918229&r1=1918228&r2=1918229&view=diff
==============================================================================
--- geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetHeaders.java (original)
+++ geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/InternetHeaders.java Mon Jun 10 10:49:39 2024
@@ -39,7 +39,39 @@ import jakarta.mail.MessagingException;
  */
 public class InternetHeaders {
     // the list of headers (to preserve order);
-    protected List<InternetHeader> headers = new ArrayList();
+    // From RFC822, outside Received and Return-Path, there should be no duplicate header otherwise, it's probably a
+    // bug on our side. CC and BCC could theoretically be present multiple times, even though it's more common to
+    // have one with multiple address similar to.
+    protected List<InternetHeader> headers = new ArrayList<InternetHeader>() {
+        @Override
+        public boolean add(final InternetHeader o) {
+            if ("Received".equals(o.getName()) || "Return-Path".equals(o.getName())) {
+                super.add(o);
+            }
+            assertNoDuplicates(o);
+            return super.add(o);
+        }
+
+        private void assertNoDuplicates(final InternetHeader o) {
+            for (InternetHeader header : this) {
+                if (header.getName() != null && header.getName().equalsIgnoreCase(o.getName())) {
+                    if (header.getValue() != null && !header.getValue().isEmpty()) {
+                        throw new IllegalStateException("InternetHeaders cannot contain more than one value for header: " + o.getName());
+                    }
+                    break;
+                }
+            }
+        }
+
+        @Override
+        public void add(final int index, final InternetHeader o) {
+            if ("Received".equals(o.getName()) || "Return-Path".equals(o.getName())) {
+                super.add(o);
+            }
+            assertNoDuplicates(o);
+            super.add(index, o);
+        }
+    };
 
     /**
      * Create an empty InternetHeaders
@@ -383,7 +415,7 @@ public class InternetHeaders {
             if (pos != -1) {
                 // this could be a placeholder header with a null value.  If it is, just update
                 // the value.  Otherwise, insert in front of the existing header.
-                final InternetHeader oldHeader = (InternetHeader)headers.get(pos);
+                final InternetHeader oldHeader = headers.get(pos);
                 if (oldHeader.getValue() == null) {
                     oldHeader.setValue(value);
                 }
@@ -403,7 +435,7 @@ public class InternetHeaders {
 
             // either insert before an existing header, or insert at the very beginning
             if (pos != -1) {
-                final InternetHeader oldHeader = (InternetHeader)headers.get(pos);
+                final InternetHeader oldHeader = headers.get(pos);
                 // if the existing header is a place holder, we can just update the value
                 if (oldHeader.getValue() == null) {
                     oldHeader.setValue(value);
@@ -446,7 +478,7 @@ public class InternetHeaders {
         final int pos = findHeader(name);
 
         if (pos != -1) {
-            final InternetHeader oldHeader = (InternetHeader)headers.get(pos);
+            final InternetHeader oldHeader = headers.get(pos);
             // keep the header in the list, but with a null value
             oldHeader.setValue(null);
             // now remove all other headers with this name
@@ -464,7 +496,7 @@ public class InternetHeaders {
         final List<Header> result = new ArrayList<>();
 
         for (int i = 0; i < headers.size(); i++) {
-            final InternetHeader header = (InternetHeader)headers.get(i);
+            final InternetHeader header = headers.get(i);
             // we only include headers with real values, no placeholders
             if (header.getValue() != null) {
                 result.add(header);
@@ -508,7 +540,7 @@ public class InternetHeaders {
         final List<Header> result = new ArrayList<>();
 
         for (int i = 0; i < headers.size(); i++) {
-            final InternetHeader header = (InternetHeader)headers.get(i);
+            final InternetHeader header = headers.get(i);
             // we only include headers with real values, no placeholders
             if (header.getValue() != null) {
                 // only add the matching ones
@@ -528,7 +560,7 @@ public class InternetHeaders {
         final List<Header> result = new ArrayList<>();
 
         for (int i = 0; i < headers.size(); i++) {
-            final InternetHeader header = (InternetHeader)headers.get(i);
+            final InternetHeader header = headers.get(i);
             // we only include headers with real values, no placeholders
             if (header.getValue() != null) {
                 // only add the non-matching ones
@@ -565,7 +597,7 @@ public class InternetHeaders {
             final int size = headers.size();
             // it's possible that we have a leading blank line.
             if (size > 0) {
-                final InternetHeader header = (InternetHeader)headers.get(size - 1);
+                final InternetHeader header = headers.get(size - 1);
                 header.appendValue(line);
             }
         }
@@ -612,13 +644,7 @@ public class InternetHeaders {
         } else {
 
             // replace the first header
-            setHeader(name, addresses[0].toString());
-
-            // now add the rest as extra headers.
-            for (int i = 1; i < addresses.length; i++) {
-                final Address address = addresses[i];
-                addHeader(name, address.toString());
-            }
+            setHeader(name, InternetAddress.toString(addresses, name.length() + 2));
         }
     }
 
@@ -636,7 +662,7 @@ public class InternetHeaders {
         if (ignore == null) {
             // write out all header lines with non-null values
             for (int i = 0; i < headers.size(); i++) {
-                final InternetHeader header = (InternetHeader)headers.get(i);
+                final InternetHeader header = headers.get(i);
                 // we only include headers with real values, no placeholders
                 if (header.getValue() != null) {
                     header.writeTo(out);
@@ -646,7 +672,7 @@ public class InternetHeaders {
         else {
             // write out all matching header lines with non-null values
             for (int i = 0; i < headers.size(); i++) {
-                final InternetHeader header = (InternetHeader)headers.get(i);
+                final InternetHeader header = headers.get(i);
                 // we only include headers with real values, no placeholders
                 if (header.getValue() != null) {
                     if (!matchHeader(header.getName(), ignore)) {

Modified: geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/MimeMessage.java
URL: http://svn.apache.org/viewvc/geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/MimeMessage.java?rev=1918229&r1=1918228&r2=1918229&view=diff
==============================================================================
--- geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/MimeMessage.java (original)
+++ geronimo/specs/trunk/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/internet/MimeMessage.java Mon Jun 10 10:49:39 2024
@@ -531,7 +531,8 @@ public class MimeMessage extends Message
      * @exception MessagingException
      */
     public void addRecipients(final Message.RecipientType type, final String address) throws MessagingException {
-        addHeader(getHeaderForRecipientType(type), address);
+        final InternetAddress[] addresses = InternetAddress.parse(address, true);
+        addHeader(getHeaderForRecipientType(type), addresses);
     }
 
     /**
@@ -1696,8 +1697,34 @@ public class MimeMessage extends Message
         }
     }
 
+    /**
+     * For addresses, it's a bit tricky, because there can be only one To/Cc/Bcc. Si if the header already exists with
+     * one or more address, we need to parse it first, create a new array and then set the header to avoid duplication
+     *
+     * @param header
+     * @param addresses
+     * @throws MessagingException
+     */
     private void addHeader(final String header, final Address[] addresses) throws MessagingException {
-        headers.addHeader(header, InternetAddress.toString(addresses));
+
+        if (addresses == null || addresses.length == 0) {
+            return;
+        }
+        final Address[] a = getHeaderAsInternetAddresses(header, true);
+        Address[] anew;
+        if (a == null || a.length == 0) {
+            anew = addresses;
+        } else {
+            anew = new Address[a.length + addresses.length];
+            System.arraycopy(a, 0, anew, 0, a.length);
+            System.arraycopy(addresses, 0, anew, a.length, addresses.length);
+        }
+        final String s = InternetAddress.toString(anew, header.length() + 2);
+        if (s == null) {
+            return;
+        }
+
+        setHeader(header, s);
     }
 
     private String getHeaderForRecipientType(final Message.RecipientType type) throws MessagingException {

Modified: geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/InternetAddressTest.java
URL: http://svn.apache.org/viewvc/geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/InternetAddressTest.java?rev=1918229&r1=1918228&r2=1918229&view=diff
==============================================================================
--- geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/InternetAddressTest.java (original)
+++ geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/InternetAddressTest.java Mon Jun 10 10:49:39 2024
@@ -442,6 +442,29 @@ public class InternetAddressTest extends
         assertEquals(InternetAddress.getLocalAddress(session), new InternetAddress("[email protected]"));
     }
 
+    public void testToStringStaticHelper() throws AddressException {
+        final InternetAddress[] addresses = new InternetAddress[]{
+                new InternetAddress("test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com"),
+                new InternetAddress("[email protected]"),
+                new InternetAddress("[email protected]")
+        };
+        {
+            final String actual = InternetAddress.toString(addresses);
+
+            assertEquals("\r\n" +
+                    "  test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com,\r\n" +
+                    "  [email protected], [email protected]", actual);
+        }
+        {
+            final String actual = InternetAddress.toString(addresses, MimeMessage.RecipientType.TO.toString().length() + 2);
+
+            assertEquals("\r\n" +
+                    "  test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com,\r\n" +
+                    "  [email protected], [email protected]", actual);
+        }
+
+    }
+
     private InternetAddress[] getGroup(final String address, final boolean strict) throws AddressException
     {
         final InternetAddress group = new InternetAddress(address);

Modified: geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/MimeMessageTest.java
URL: http://svn.apache.org/viewvc/geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/MimeMessageTest.java?rev=1918229&r1=1918228&r2=1918229&view=diff
==============================================================================
--- geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/MimeMessageTest.java (original)
+++ geronimo/specs/trunk/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/internet/MimeMessageTest.java Mon Jun 10 10:49:39 2024
@@ -44,6 +44,35 @@ public class MimeMessageTest extends Tes
     private CommandMap defaultMap;
     private Session session;
 
+    public void testNoDuplicateTo() throws MessagingException, IOException {
+        final InternetAddress[] addresses = new InternetAddress[]{
+                new InternetAddress("test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com"),
+                new InternetAddress("[email protected]"),
+                new InternetAddress("[email protected]")
+        };
+        {
+            final MimeMessage msg = new MimeMessage(session);
+            msg.setContent("Hello World", "text/plain");
+            msg.setRecipients(Message.RecipientType.TO, addresses);
+            final ByteArrayOutputStream out = new ByteArrayOutputStream();
+            msg.writeTo(out);
+
+            final String textMessage = new String(out.toByteArray());
+            assertTrue(textMessage, textMessage.contains(
+                    "To: \r\n" +
+                    "  test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com,\r\n" +
+                    "  [email protected], [email protected]"));
+
+        }
+        {
+            final String actual = InternetAddress.toString(addresses, MimeMessage.RecipientType.TO.toString().length() + 2);
+
+            assertEquals("\r\n" +
+                    "  test1ofaveryveryverylongemailaddressover71charwhichseemscrazyatfirstglance@example.com,\r\n" +
+                    "  [email protected], [email protected]", actual);
+        }
+    }
+
     public void testWriteTo() throws MessagingException, IOException {
         final MimeMessage msg = new MimeMessage(session);
         msg.setSender(new InternetAddress("foo"));