(geronimo-mail) 02/02: GERONIMO-6894 - Flags.retainAll adds flags from the argument and removes user flags despite Flags.Flag.USER

[email protected] Sat, 18 Jul 2026 18:22:27 +0000
Newsgroups gmane.comp.java.geronimo.cvs
Message-ID <[email protected]>
This is an automated email from the ASF dual-hosted git repository.

rzo1 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/geronimo-mail.git

commit e9acad02b4e322ee77f658f36a65d13e768b762b
Author: Richard Zowalla <[email protected]>
AuthorDate: Sat Jul 18 20:22:11 2026 +0200

    GERONIMO-6894 - Flags.retainAll adds flags from the argument and removes user flags despite Flags.Flag.USER
---
 .../src/main/java/jakarta/mail/Flags.java          | 21 +++++++----
 .../src/test/java/jakarta/mail/FlagsTest.java      | 41 ++++++++++++++++++++++
 geronimo-mail_2.1_tck/src/tck/geronimo.jtx         |  1 -
 3 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/Flags.java b/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/Flags.java
index 1988cde..951289d 100644
--- a/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/Flags.java
+++ b/geronimo-mail_2.1_spec/src/main/java/jakarta/mail/Flags.java
@@ -311,16 +311,23 @@ public class Flags implements Cloneable, Serializable {
     public boolean retainAll(Flags f) {
         boolean changed = false;
 
-        if (this.system_flags != f.system_flags) {
-            this.system_flags = f.system_flags;
+        // retain only the system flags both sets carry -- an intersection,
+        // never an assignment (that could add flags we did not have)
+        final int retained = this.system_flags & f.system_flags;
+        if (this.system_flags != retained) {
+            this.system_flags = retained;
             changed = true;
         }
 
-        final Set<String> keys = new HashSet<>(this.user_flags.keySet());
-        for (final String user_flag : keys) {
-            if (! f.user_flags.containsKey(user_flag)) {
-                this.user_flags.remove(user_flag);
-                changed = true;
+        // if the argument carries the special USER flag, all user flags are
+        // retained regardless of the argument's individual user flags
+        if ((f.system_flags & Flag.USER.mask) == 0) {
+            final Set<String> keys = new HashSet<>(this.user_flags.keySet());
+            for (final String user_flag : keys) {
+                if (! f.user_flags.containsKey(user_flag)) {
+                    this.user_flags.remove(user_flag);
+                    changed = true;
+                }
             }
         }
 
diff --git a/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/FlagsTest.java b/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/FlagsTest.java
index fced105..5a4495d 100644
--- a/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/FlagsTest.java
+++ b/geronimo-mail_2.1_spec/src/test/java/jakarta/mail/FlagsTest.java
@@ -236,4 +236,45 @@ public class FlagsTest {
         assertEquals(1, f.getUserFlags().length);
         assertEquals("TEST", f.getUserFlags()[0]);
     }
+
+    @Test
+    public void testRetainAllNeverAddsFlags() {
+        Flags f = new Flags();
+        f.add(Flags.Flag.SEEN);
+
+        Flags retain = new Flags();
+        retain.add(Flags.Flag.SEEN);
+        retain.add(Flags.Flag.DELETED);
+        retain.add("EXTRA");
+
+        // retaining is an intersection: flags only present in the argument
+        // must not appear, and nothing changes here
+        assertFalse(f.retainAll(retain));
+
+        assertEquals(1, f.getSystemFlags().length);
+        assertEquals(Flags.Flag.SEEN, f.getSystemFlags()[0]);
+        assertEquals(0, f.getUserFlags().length);
+    }
+
+    @Test
+    public void testRetainAllWithUserFlagKeepsUserFlags() {
+        Flags f = new Flags();
+        f.add(Flags.Flag.SEEN);
+        f.add(Flags.Flag.DELETED);
+        f.add("ONE");
+        f.add("TWO");
+
+        // an argument carrying Flags.Flag.USER retains all user flags
+        Flags retain = new Flags();
+        retain.add(Flags.Flag.SEEN);
+        retain.add(Flags.Flag.USER);
+
+        assertTrue(f.retainAll(retain));
+
+        assertEquals(1, f.getSystemFlags().length);
+        assertEquals(Flags.Flag.SEEN, f.getSystemFlags()[0]);
+        assertEquals(2, f.getUserFlags().length);
+        assertTrue(Arrays.asList(f.getUserFlags()).contains("ONE"));
+        assertTrue(Arrays.asList(f.getUserFlags()).contains("TWO"));
+    }
 }
diff --git a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
index 0b9b927..deef334 100644
--- a/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
+++ b/geronimo-mail_2.1_tck/src/tck/geronimo.jtx
@@ -45,7 +45,6 @@ javasoft/sqe/tests/jakarta/mail/exception/testlist.html#folderNotFoundExp_Test
 javasoft/sqe/tests/jakarta/mail/exception/testlist.html#msgRemoveExp_Test
 javasoft/sqe/tests/jakarta/mail/FetchProfile/testlist.html#add_Test
 javasoft/sqe/tests/jakarta/mail/FetchProfile/testlist.html#contains_Test
-javasoft/sqe/tests/jakarta/mail/Flags/testlist.html#retainAll_Test
 javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#create_Test
 javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#delete_Test
 javasoft/sqe/tests/jakarta/mail/Folder/testlist.html#fetch_Test