Fix some UIDPLUS bugs and other UIDPLUS tweaks

* 🐛 Bug fixed: `uid-range` should behave the same, regardless of order.
  i.e. "2:4" and "4:2" are equivalent.
* 🐛 Reversed a breaking change: The return values of Net::IMAP#copy,
  #move, #uid_copy, #uid_move, and #append are incompatible with all
  previously released versions.  That would break any apps that expect
  the tagged response.  TaggedResponse is more broadly useful anyway,
  since it contains the response code with its data.
* Forward compatibility with `MULTIAPPEND`:  I converted "append-uid"
  to parse a "uid-set" instead of a "uniqueid".  This also provides a
  consistent interface: assigned_uids will always be an array of ints.
* Replaced arrays with a `UIDPlusData` struct.  In my opinion, it's much
  nicer to use a Struct than a "tuple". This also provides a home for
  documentation and utility methods.  E.g. I also added
  UIDPlusData#uid_mapping, which returns a hash of {src_uid => dst_uid}.
* Number parsing was converted from `to_i` to `Integer`.  We matched the
  `uid-set` with a single `T_ATOM` token, so we haven't validated that
  it follows the rest of the grammar.  This will still parse some
  invalid strings (e.g. `",:1,,:::,,"`), but in practice it's much more
  likely that a bogus atom would have at least one non-numeric character
  besides ":" and ",".  (Although I've seen some crazy server bugs...)

Author: nevans

lib/net/imap.rb

@@ -778,12 +778,7 @@ def append(mailbox, message, flags = nil, date_time = nil)
       end
       args.push(date_time) if date_time
       args.push(Literal.new(message))
-      synchronize do
-        resp = send_command("APPEND", mailbox, *args)
-        if resp.data.code && resp.data.code.name == "APPENDUID"
-          return resp.data.code.data
-        end
-      end
+      send_command("APPEND", mailbox, *args)
     end
 
     # Sends a CHECK command to request a checkpoint of the currently
@@ -1459,12 +1454,7 @@ def store_internal(cmd, set, attr, flags)
     end
 
     def copy_internal(cmd, set, mailbox)
-      synchronize do
-        resp = send_command(cmd, MessageSet.new(set), mailbox)
-        if resp.data.code && resp.data.code.name == "COPYUID"
-          return resp.data.code.data
-        end
-      end
+      send_command(cmd, MessageSet.new(set), mailbox)
     end
 
     def sort_internal(cmd, sort_keys, search_keys, charset)

lib/net/imap/response_data.rb

@@ -113,6 +113,65 @@ class ResponseText < Struct.new(:code, :text)
     class ResponseCode < Struct.new(:name, :data)
     end
 
+    # Net::IMAP::UIDPlusData represents the ResponseCode#data that accompanies
+    # the +APPENDUID+ and +COPYUID+ response codes.
+    #
+    # ==== Capability requirement
+    #
+    # The +UIDPLUS+ capability[rdoc-ref:Net::IMAP#capability] must be supported.
+    # A server that supports +UIDPLUS+ should send a UIDPlusData object inside
+    # every TaggedResponse returned by the append[rdoc-ref:Net::IMAP#append],
+    # copy[rdoc-ref:Net::IMAP#copy], move[rdoc-ref:Net::IMAP#move], {uid
+    # copy}[rdoc-ref:Net::IMAP#uid_copy], and {uid
+    # move}[rdoc-ref:Net::IMAP#uid_move] commands---unless the destination
+    # mailbox reports +UIDNOTSTICKY+.
+    #
+    #--
+    # TODO: support MULTIAPPEND
+    #++
+    #
+    # ==== References
+    #
+    # [UIDPLUS[https://www.rfc-editor.org/rfc/rfc4315.html]]::
+    #   Crispin, M., "Internet Message Access Protocol (IMAP) - UIDPLUS
+    #   extension", RFC 4315, DOI 10.17487/RFC4315, December 2005,
+    #   <https://www.rfc-editor.org/info/rfc4315>.
+    #
+    class UIDPlusData < Struct.new(:uidvalidity, :source_uids, :assigned_uids)
+      ##
+      # method: uidvalidity
+      # :call-seq: uidvalidity -> nonzero uint32
+      #
+      # The UIDVALIDITY of the destination mailbox.
+
+      ##
+      # method: source_uids
+      # :call-seq: source_uids -> nil or an array of nonzero uint32
+      #
+      # The UIDs of the copied or moved messages.
+      #
+      # Note:: Returns +nil+ for Net::IMAP#append.
+
+      ##
+      # method: assigned_uids
+      # :call-seq: assigned_uids -> an array of nonzero uint32
+      #
+      # The newly assigned UIDs of the copied, moved, or appended messages.
+      #
+      # Note:: This always returns an array, even when it contains only one UID.
+
+      ##
+      # :call-seq: uid_mapping -> nil or a hash
+      #
+      # Returns a hash mapping each source UID to the newly assigned destination
+      # UID.
+      #
+      # Note:: Returns +nil+ for Net::IMAP#append.
+      def uid_mapping
+        source_uids&.zip(assigned_uids)&.to_h
+      end
+    end
+
     # Net::IMAP::MailboxList represents contents of the LIST response.
     #
     #   mailbox_list    ::= "(" #("\Marked" / "\Noinferiors" /

lib/net/imap/response_parser.rb

@@ -1104,11 +1104,8 @@ def resp_text
       #                   "UNSEEN" SP nz-number /
       #                   atom [SP 1*<any TEXT-CHAR except "]">]
       #
-      # See https://datatracker.ietf.org/doc/html/rfc4315#section-6.4 for UIDPLUS extension
-      #
-      # resp-code-apnd  = "APPENDUID" SP nz-number SP append-uid
-      # resp-code-copy  = "COPYUID" SP nz-number SP uid-set SP uid-set
-      # resp-text-code  =/ resp-code-apnd / resp-code-copy / "UIDNOTSTICKY"
+      # +UIDPLUS+ ABNF:: https://www.rfc-editor.org/rfc/rfc4315.html#section-4
+      #   resp-text-code  =/ resp-code-apnd / resp-code-copy / "UIDNOTSTICKY"
       def resp_text_code
         token = match(T_ATOM)
         name = token.value.upcase
@@ -1126,19 +1123,9 @@ def resp_text_code
           match(T_SPACE)
           result = ResponseCode.new(name, number)
         when /\A(?:APPENDUID)\z/n
-          match(T_SPACE)
-          uidvalidity = number
-          match(T_SPACE)
-          append_uid = number
-          result = ResponseCode.new(name, [uidvalidity, append_uid])
+          result = ResponseCode.new(name, resp_code_apnd__data)
         when /\A(?:COPYUID)\z/n
-          match(T_SPACE)
-          uidvalidity = number
-          match(T_SPACE)
-          from_uid = uid_set
-          match(T_SPACE)
-          to_uid = uid_set
-          result = ResponseCode.new(name, [uidvalidity, from_uid, to_uid])
+          result = ResponseCode.new(name, resp_code_copy__data)
         else
           token = lookahead
           if token.symbol == T_SPACE
@@ -1165,6 +1152,34 @@ def charset_list
         result
       end
 
+      # already matched:  "APPENDUID"
+      #
+      # +UIDPLUS+ ABNF:: https://www.rfc-editor.org/rfc/rfc4315.html#section-4
+      #   resp-code-apnd  = "APPENDUID" SP nz-number SP append-uid
+      #   append-uid      = uniqueid
+      #   append-uid      =/ uid-set
+      #                     ; only permitted if client uses [MULTIAPPEND]
+      #                     ; to append multiple messages.
+      #
+      # n.b, uniqueid ⊂ uid-set.  To avoid inconsistent return types, we always
+      # match uid_set even if that returns a single-member array.
+      #
+      def resp_code_apnd__data
+        match(T_SPACE); validity = number
+        match(T_SPACE); dst_uids = uid_set # uniqueid ⊂ uid-set
+        UIDPlusData.new(validity, nil, dst_uids)
+      end
+
+      # already matched:  "COPYUID"
+      #
+      # resp-code-copy  = "COPYUID" SP nz-number SP uid-set SP uid-set
+      def resp_code_copy__data
+        match(T_SPACE); validity = number
+        match(T_SPACE); src_uids = uid_set
+        match(T_SPACE); dst_uids = uid_set
+        UIDPlusData.new(validity, src_uids, dst_uids)
+      end
+
       def address_list
         token = lookahead
         if token.symbol == T_NIL
@@ -1350,19 +1365,14 @@ def number
       #      uniqueid        = nz-number
       #                          ; Strictly ascending
       def uid_set
-        case lookahead.symbol
-        when T_NUMBER then [match(T_NUMBER).value.to_i]
+        token = match(T_NUMBER, T_ATOM)
+        case token.symbol
+        when T_NUMBER then [Integer(token.value)]
         when T_ATOM
-          match(T_ATOM).value.split(',').flat_map do |element|
-            if element.include?(':')
-              Range.new(*element.split(':').map(&:to_i)).to_a
-            else
-              element.to_i
-            end
-          end
-        else
-          shift_token
-          nil
+          token.value.split(",").flat_map {|range|
+            range = range.split(":").map {|uniqueid| Integer(uniqueid) }
+            range.size == 1 ? range : Range.new(range.min, range.max).to_a
+          }
         end
       end
 

test/net/imap/test_imap.rb

@@ -848,16 +848,16 @@ def test_uidplus_responses
 
         hello world
       EOF
-      assert_equal(resp, [38505, 3955])
+      assert_equal([38505, nil, [3955]], resp.data.code.data.to_a)
       resp = imap.uid_copy([3955,3960..3962], 'trash')
       assert_equal(requests.pop, "RUBY0002 UID COPY 3955,3960:3962 trash\r\n")
       assert_equal(
-        resp,
-        [38505, [3955, 3960, 3961, 3962], [3963, 3964, 3965, 3966]]
+        [38505, [3955, 3960, 3961, 3962], [3963, 3964, 3965, 3966]],
+        resp.data.code.data.to_a
       )
       resp = imap.uid_copy(3955, 'trash')
       assert_equal(requests.pop, "RUBY0003 UID COPY 3955 trash\r\n")
-      assert_equal(resp, [38505, [3955], [3967]])
+      assert_equal([38505, [3955], [3967]], resp.data.code.data.to_a)
       imap.select('trash')
       assert_equal(
         imap.responses["NO"].last.code,

test/net/imap/test_imap_response_parser.rb

@@ -363,4 +363,79 @@ def test_namespace
     assert_equal("/", namespace.delim)
     assert_equal({"X-PARAM" => ["FLAG1", "FLAG2"]}, namespace.extensions)
   end
+
+  def test_uidplus_appenduid
+    parser = Net::IMAP::ResponseParser.new
+    # RFC4315 example
+    response = parser.parse(
+      "A003 OK [APPENDUID 38505 3955] APPEND completed\r\n"
+    )
+    code = response.data.code
+    assert_equal   "APPENDUID", code.name
+    assert_kind_of Net::IMAP::UIDPlusData, code.data
+    assert_equal   Net::IMAP::UIDPlusData.new(38505, nil, [3955]), code.data
+    assert_equal   "APPENDUID", code.name
+    assert_kind_of Net::IMAP::UIDPlusData, code.data
+    assert_equal   Net::IMAP::UIDPlusData.new(38505, nil, [3955]), code.data
+    # MULTIAPPEND compatibility:
+    response = parser.parse(
+      "A003 OK [APPENDUID 2 4,6:7,9] APPEND completed\r\n"
+    )
+    code = response.data.code
+    assert_equal   "APPENDUID", code.name
+    assert_kind_of Net::IMAP::UIDPlusData, code.data
+    assert_equal   Net::IMAP::UIDPlusData.new(2, nil, [4, 6, 7, 9]), code.data
+  end
+
+  def test_uidplus_copyuid
+    parser = Net::IMAP::ResponseParser.new
+    # RFC4315 example, but using mixed case "copyUID".
+    response = parser.parse(
+      "A004 OK [copyUID 38505 304,319:320 3956:3958] Done\r\n"
+    )
+    code = response.data.code
+    assert_equal   "COPYUID", code.name
+    assert_kind_of Net::IMAP::UIDPlusData, code.data
+    assert_equal   Net::IMAP::UIDPlusData.new(
+      38505, [304, 319, 320], [3956, 3957, 3958]
+    ), code.data
+  end
+
+  # From RFC4315 ABNF:
+  # > and all values between these two *regardless of order*.
+  # > Example: 2:4 and 4:2 are equivalent.
+  def test_uidplus_copyuid__reversed_ranges
+    parser = Net::IMAP::ResponseParser.new
+    response = parser.parse(
+      "A004 OK [copyUID 9999 20:19,500:495 92:97,101:100] Done\r\n"
+    )
+    code = response.data.code
+    assert_equal Net::IMAP::UIDPlusData.new(
+      9999,
+      [19, 20, 495, 496, 497, 498, 499, 500],
+      [92, 93, 94, 95, 96, 97, 100, 101]
+    ), code.data
+  end
+
+  def test_uidplus_copyuid__uid_mapping
+    parser = Net::IMAP::ResponseParser.new
+    response = parser.parse(
+      "A004 OK [copyUID 9999 20:19,500:495 92:97,101:100] Done\r\n"
+    )
+    code = response.data.code
+    assert_equal(
+      {
+         19 =>  92,
+         20 =>  93,
+        495 =>  94,
+        496 =>  95,
+        497 =>  96,
+        498 =>  97,
+        499 => 100,
+        500 => 101,
+      },
+      code.data.uid_mapping
+    )
+  end
+
 end