⚡️ Update flag parsing: FLAGS, LIST, PERMANENTFLAGS

Rather than treat all three identically, this uses a more strict parser
tailored to each one.  One of my earlier tests was invalid, so that has
also been fixed.

Rather than use `String#scan`, the flags lists are matched with a single
regexp, and the match is split then mapped.  In my benchmarks, this is
_slightly_ faster than using `String#scan`.

Author: nevans

benchmarks/parser.yml

@@ -116,6 +116,11 @@ benchmark:
       response = load_response("../test/net/imap/fixtures/response_parser/fetch_responses.yml",
                                "test_fetch_msg_att_uid")
   script: parser.parse(response)
+- name: flags_with_various_flag_types
+  prelude: |2
+      response = load_response("../test/net/imap/fixtures/response_parser/flags_responses.yml",
+                               "test_flags_with_various_flag_types")
+  script: parser.parse(response)
 - name: id_gmail
   prelude: |2
       response = load_response("../test/net/imap/fixtures/response_parser/id_responses.yml",
@@ -191,10 +196,10 @@ benchmark:
       response = load_response("../test/net/imap/fixtures/response_parser/thread_responses.yml",
                                "test_invalid_thread_empty_response")
   script: parser.parse(response)
-- name: list_with_various_flag_types_and_capitalizations
+- name: list_with_various_flag_capitalizations
   prelude: |2
       response = load_response("../test/net/imap/fixtures/response_parser/list_responses.yml",
-                               "test_list_with_various_flag_types_and_capitalizations")
+                               "test_list_with_various_flag_capitalizations")
   script: parser.parse(response)
 - name: resp_code_ALREADYEXISTS_rfc9051_7.1_example
   prelude: |2
@@ -376,6 +381,11 @@ benchmark:
       response = load_response("../test/net/imap/fixtures/response_parser/resp_code_examples.yml",
                                "test_resp_code_UNSEEN_rfc3501_6.3.1_example")
   script: parser.parse(response)
+- name: resp_text_PERMANENTFLAGS_with_various_flag_types
+  prelude: |2
+      response = load_response("../test/net/imap/fixtures/response_parser/resp_code_examples.yml",
+                               "test_resp_text_PERMANENTFLAGS_with_various_flag_types")
+  script: parser.parse(response)
 - name: resp_text_with_T_LBRA
   prelude: |2
       response = load_response("../test/net/imap/fixtures/response_parser/resp_text_responses.yml",

lib/net/imap/response_parser.rb

@@ -185,6 +185,54 @@ module RFC3629
         CODE_TEXT_CHAR    = TEXT_CHAR - RESP_SPECIALS
         CODE_TEXT         = /#{CODE_TEXT_CHAR}+/n
 
+        # flag            = "\Answered" / "\Flagged" / "\Deleted" /
+        #                   "\Seen" / "\Draft" / flag-keyword / flag-extension
+        #                     ; Does not include "\Recent"
+        # flag-extension  = "\" atom
+        #                     ; Future expansion.  Client implementations
+        #                     ; MUST accept flag-extension flags.  Server
+        #                     ; implementations MUST NOT generate
+        #                     ; flag-extension flags except as defined by
+        #                     ; a future Standard or Standards Track
+        #                     ; revisions of this specification.
+        # flag-keyword    = "$MDNSent" / "$Forwarded" / "$Junk" /
+        #                   "$NotJunk" / "$Phishing" / atom
+        # flag-perm       = flag / "\*"
+        #
+        # Not checking for max one mbx-list-sflag in the parser.
+        # >>>
+        # mbx-list-oflag  = "\Noinferiors" / child-mbox-flag /
+        #                   "\Subscribed" / "\Remote" / flag-extension
+        #                    ; Other flags; multiple from this list are
+        #                    ; possible per LIST response, but each flag
+        #                    ; can only appear once per LIST response
+        # mbx-list-sflag  = "\NonExistent" / "\Noselect" / "\Marked" /
+        #                   "\Unmarked"
+        #                    ; Selectability flags; only one per LIST response
+        # child-mbox-flag =  "\HasChildren" / "\HasNoChildren"
+        #                    ; attributes for the CHILDREN return option, at most
+        #                    ; one possible per LIST response
+        FLAG              = /\\?#{ATOM}/n
+        FLAG_EXTENSION    = /\\#{ATOM}/n
+        FLAG_KEYWORD      = ATOM
+        FLAG_PERM         = Regexp.union(FLAG, "\\*")
+        MBX_FLAG          = FLAG_EXTENSION
+
+        # flag-list       = "(" [flag *(SP flag)] ")"
+        #
+        # part of resp-text-code:
+        # >>>
+        #   "PERMANENTFLAGS" SP "(" [flag-perm *(SP flag-perm)] ")"
+        #
+        # parens from mailbox-list are included in the regexp:
+        # >>>
+        #   mbx-list-flags  = *(mbx-list-oflag SP) mbx-list-sflag
+        #                     *(SP mbx-list-oflag) /
+        #                     mbx-list-oflag *(SP mbx-list-oflag)
+        FLAG_LIST      = /\G\((#{FLAG     }(?:#{SP}#{FLAG     })*|)\)/ni
+        FLAG_PERM_LIST = /\G\((#{FLAG_PERM}(?:#{SP}#{FLAG_PERM})*|)\)/ni
+        MBX_LIST_FLAGS = /\G\((#{MBX_FLAG }(?:#{SP}#{MBX_FLAG })*|)\)/ni
+
         # RFC3501:
         #   QUOTED-CHAR   = <any TEXT-CHAR except quoted-specials> /
         #                   "\" quoted-specials
@@ -383,6 +431,14 @@ def label(word)
         parse_error("unexpected atom %p, expected %p instead", val, word)
       end
 
+      # Use #label or #label_in to assert specific known labels
+      # (+tagged-ext-label+ only, not +atom+).
+      def label_in(*labels)
+        lbl = tagged_ext_label and labels.include?(lbl) and return lbl
+        parse_error("unexpected atom %p, expected one of %s instead",
+                    lbl, labels.join(" or "))
+      end
+
       # expects "OK" or "PREAUTH" and raises InvalidResponseError on failure
       def resp_cond_auth__name
         lbl = tagged_ext_label and AUTH_CONDS.include? lbl and return lbl
@@ -1084,18 +1140,21 @@ def header_fld_name
         end
       end
 
+      # mailbox-data    =  "FLAGS" SP flag-list / "LIST" SP mailbox-list /
+      #                    "LSUB" SP mailbox-list / "SEARCH" *(SP nz-number) /
+      #                    "STATUS" SP mailbox SP "(" [status-att-list] ")" /
+      #                    number SP "EXISTS" / number SP "RECENT"
+
       def mailbox_data__flags
-        token = match(T_ATOM)
-        name = token.value.upcase
-        match(T_SPACE)
-        return UntaggedResponse.new(name, flag_list, @str)
+        name = label("FLAGS")
+        SP!
+        UntaggedResponse.new(name, flag_list, @str)
       end
 
       def mailbox_data__list
-        token = match(T_ATOM)
-        name = token.value.upcase
-        match(T_SPACE)
-        return UntaggedResponse.new(name, mailbox_list, @str)
+        name = label_in("LIST", "LSUB", "XLIST")
+        SP!
+        UntaggedResponse.new(name, mailbox_list, @str)
       end
       alias mailbox_data__lsub  mailbox_data__list
       alias mailbox_data__xlist mailbox_data__list
@@ -1645,28 +1704,38 @@ def address
         return Address.new(name, route, mailbox, host)
       end
 
-      FLAG_REGEXP = /\
-(?# FLAG        )\\([^\x80-\xff(){ \x00-\x1f\x7f%"\\]+)|\
-(?# ATOM        )([^\x80-\xff(){ \x00-\x1f\x7f%*"\\]+)/n
-
+      # flag-list       = "(" [flag *(SP flag)] ")"
       def flag_list
-        if @str.index(/\(([^)]*)\)/ni, @pos)
-          @pos = $~.end(0)
-          return $1.scan(FLAG_REGEXP).collect { |flag, atom|
-            if atom
-              atom
-            else
-              flag.capitalize.intern
-            end
-          }
-        else
-          parse_error("invalid flag list")
-        end
+        match_re(Patterns::FLAG_LIST, "flag-list")[1]
+          .split(nil)
+          .map! { _1.start_with?("\\") ? _1[1..].capitalize.to_sym : _1 }
+      end
+
+      #   "(" [flag-perm *(SP flag-perm)] ")"
+      def flag_perm__list
+        match_re(Patterns::FLAG_PERM_LIST, "PERMANENTFLAGS flag-perm list")[1]
+          .split(nil)
+          .map! { _1.start_with?("\\") ? _1[1..].capitalize.to_sym : _1 }
+      end
+
+      # Not checking for max one mbx-list-sflag in the parser.
+      # >>>
+      #   mbx-list-flags  = *(mbx-list-oflag SP) mbx-list-sflag
+      #                     *(SP mbx-list-oflag) /
+      #                     mbx-list-oflag *(SP mbx-list-oflag)
+      #   mbx-list-oflag  = "\Noinferiors" / child-mbox-flag /
+      #                     "\Subscribed" / "\Remote" / flag-extension
+      #                  ; Other flags; multiple from this list are
+      #                  ; possible per LIST response, but each flag
+      #                  ; can only appear once per LIST response
+      #   mbx-list-sflag  = "\NonExistent" / "\Noselect" / "\Marked" /
+      #                     "\Unmarked"
+      #                  ; Selectability flags; only one per LIST response
+      def parens__mbx_list_flags
+        match_re(Patterns::MBX_LIST_FLAGS, "mbx-list-flags")[1]
+          .split(nil).map! { _1.capitalize.to_sym }
       end
 
-      # TODO: not quite correct.  flag-perm != flag
-      alias flag_perm__list flag_list
-
       # See https://www.rfc-editor.org/errata/rfc3501
       #
       # charset = atom / quoted

test/net/imap/fixtures/response_parser/flags_responses.yml

@@ -14,3 +14,14 @@
       - :Seen
       - :Draft
       raw_data: "* FLAGS (\\Answered \\Flagged \\Deleted \\Seen \\Draft)\r\n"
+
+  test_flags_with_various_flag_types:
+    :response: "* FLAGS (\\foo bAR $Etc \\baz)\r\n"
+    :expected: !ruby/struct:Net::IMAP::UntaggedResponse
+      name: FLAGS
+      data:
+      - :Foo
+      - bAR
+      - $Etc
+      - :Baz
+      raw_data: "* FLAGS (\\foo bAR $Etc \\baz)\r\n"

test/net/imap/fixtures/response_parser/list_responses.yml

@@ -34,19 +34,19 @@
         name: "#news.comp.mail.misc"
       raw_data: "* LSUB () \".\" #news.comp.mail.misc\r\n"
 
-  test_list_with_various_flag_types_and_capitalizations:
-    :response: "* LIST (\\foo bAR $Etc \\baz) \".\" \"INBOX\"\r\n"
+  test_list_with_various_flag_capitalizations:
+    :response: "* LIST (\\foo \\bAR \\Etc \\baz) \".\" \"INBOX\"\r\n"
     :expected: !ruby/struct:Net::IMAP::UntaggedResponse
       name: LIST
       data: !ruby/struct:Net::IMAP::MailboxList
         attr:
         - :Foo
-        - bAR
-        - $Etc
+        - :Bar
+        - :Etc
         - :Baz
         delim: "."
         name: INBOX
-      raw_data: "* LIST (\\foo bAR $Etc \\baz) \".\" \"INBOX\"\r\n"
+      raw_data: "* LIST (\\foo \\bAR \\Etc \\baz) \".\" \"INBOX\"\r\n"
 
   test_xlist_inbox:
     :response: "* XLIST (\\Inbox) \".\" \"INBOX\"\r\n"

test/net/imap/fixtures/response_parser/resp_code_examples.yml

@@ -86,6 +86,22 @@
         text: No permanent flags permitted
       raw_data: "* OK [PERMANENTFLAGS ()] No permanent flags permitted\r\n"
 
+  test_resp_text_PERMANENTFLAGS_with_various_flag_types:
+    :response: "* ok [PERMANENTFLAGS (\\foo \\* bAR $Etc \\baz)] flags saved\r\n"
+    :expected: !ruby/struct:Net::IMAP::UntaggedResponse
+      name: OK
+      data: !ruby/struct:Net::IMAP::ResponseText
+        text: flags saved
+        code: !ruby/struct:Net::IMAP::ResponseCode
+          name: PERMANENTFLAGS
+          data:
+          - :Foo
+          - :*
+          - bAR
+          - $Etc
+          - :Baz
+      raw_data: "* ok [PERMANENTFLAGS (\\foo \\* bAR $Etc \\baz)] flags saved\r\n"
+
   test_resp_code_READ-ONLY_rfc3501_6.3.2_example:
     :response: "A932 OK [READ-ONLY] EXAMINE completed\r\n"
     :expected: !ruby/struct:Net::IMAP::TaggedResponse