From 1d9afd502bf8e29af5bef7180d73268cd00022fb Mon Sep 17 00:00:00 2001
From: nicholas evans <nicholas.evans@gmail.com>
Date: Mon, 1 Jan 2024 21:50:18 -0500
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Enable=20parenthesized=20lists=20in?=
 =?UTF-8?q?=20search=20criteria?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This affects `#search`, `#uid_search`, `#sort`, `#uid_sort`, `#thread`,
and `#uid_thread`.

Prior to this, sending a parenthesized list in the search criteria for
any of these commands required the use of strings, which are converted
to `RawData`, which has security implications with untrusted inputs.

With this change, arrays will only be converted into SequenceSet when
_every_ element in the array is a valid SequenceSet input.  Otherwise,
the array will be left alone, which allows us to send parenthesized
lists without using strings and RawData.

For example, some searches this change enables:

* Combining criteria to pass into `OR`, `NOT`, `FUZZY`, etc.
  * `search(["not", %w(flagged unread)])`
    converts to: `SEARCH not (flagged unread)`
* Adding return options (we should also add a return kwarg).
  * `uid_search(["RETURN", ["PARTIAL", 1..50], "UID", 12345..67890])`
    converts to: `UID SEARCH RETURN (PARTIAL 1:50) UID 12345:67890`
  * Note that `PARTIAL` supports negative ranges, which can't be coerced
    to SequenceSet.  They'll need to be sent as strings, for now.
  * Note that searches with return options should return ESEARCH
    results, which are currently unsupported.  See #333.

This _should_ be backward compatible: previously these inputs would
raise an exception.
---
 lib/net/imap.rb            | 25 +++++++++++++++++++------
 test/net/imap/test_imap.rb | 12 ++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/lib/net/imap.rb b/lib/net/imap.rb
index 5b19558f..4efe99e0 100644
--- a/lib/net/imap.rb
+++ b/lib/net/imap.rb
@@ -1949,9 +1949,13 @@ def uid_expunge(uid_set)
     #     * +Range+
     #     * <tt>-1</tt> and +:*+ -- both translate to <tt>*</tt>
     #     * responds to +#to_sequence_set+
-    #     * nested +Array+
+    #     * +Array+, when each element is one of the above types, a positive
+    #       +Integer+, a sequence-set formatted +String+, or a deeply nested
+    #       +Array+ of these same types.
     #   * Any +String+ is sent verbatim when it is a valid \IMAP atom,
     #     and encoded as an \IMAP quoted or literal string otherwise.
+    #   * Any other nested +Array+ is encoded as a parenthesized list, to group
+    #     multiple search keys (e.g., for use with +OR+ and +NOT+).
     #   * Any other +Integer+ (besides <tt>-1</tt>) will be sent as +#to_s+.
     #   * +Date+ objects will be encoded as an \IMAP date (see ::encode_date).
     #
@@ -1976,13 +1980,13 @@ def uid_expunge(uid_set)
     # The following searches send the exact same command to the server:
     #
     #    # criteria array, charset arg
-    #    imap.search(%w[OR UNSEEN FLAGGED SUBJECT foo], "UTF-8")
+    #    imap.search(["OR", "UNSEEN", %w(FLAGGED SUBJECT foo)], "UTF-8")
     #    # criteria string, charset arg
-    #    imap.search("OR UNSEEN FLAGGED SUBJECT foo", "UTF-8")
+    #    imap.search("OR UNSEEN (FLAGGED SUBJECT foo)", "UTF-8")
     #    # criteria array contains charset arg
-    #    imap.search(%w[CHARSET UTF-8 OR UNSEEN FLAGGED SUBJECT foo])
+    #    imap.search([*%w[CHARSET UTF-8], "OR", "UNSEEN", %w(FLAGGED SUBJECT foo)])
     #    # criteria string contains charset arg
-    #    imap.search("CHARSET UTF-8 OR UNSEEN FLAGGED SUBJECT foo")
+    #    imap.search("CHARSET UTF-8 OR UNSEEN (FLAGGED SUBJECT foo)")
     #
     # ===== Search keys
     #
@@ -3208,11 +3212,20 @@ def coerce_search_arg_to_seqset?(obj)
       case obj
       when Set, -1, :* then true
       when Range       then true
-      when Array       then true
+      when Array       then obj.all? { coerce_search_array_arg_to_seqset? _1 }
       else                  obj.respond_to?(:to_sequence_set)
       end
     end
 
+    def coerce_search_array_arg_to_seqset?(obj)
+      case obj
+      when Integer then obj.positive? || obj == -1
+      when String  then ResponseParser::Patterns::SEQUENCE_SET_STR.match?(obj.b)
+      else
+        coerce_search_arg_to_seqset?(obj)
+      end
+    end
+
     def build_ssl_ctx(ssl)
       if ssl
         params = (Hash.try_convert(ssl) || {}).freeze
diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb
index 895892d9..c9fe5ae1 100644
--- a/test/net/imap/test_imap.rb
+++ b/test/net/imap/test_imap.rb
@@ -1242,6 +1242,18 @@ def seqset_coercible.to_sequence_set
                                                    [1..22, 30..-1]])
       cmd = server.commands.pop
       assert_equal ["UID SEARCH", "subject hello 1:22,30:*"], [cmd.name, cmd.args]
+
+      assert_equal search_result, imap.search(
+        "RETURN (COUNT) NOT (FLAGGED (OR SEEN ANSWERED))"
+      )
+      cmd = server.commands.pop
+      assert_equal "RETURN (COUNT) NOT (FLAGGED (OR SEEN ANSWERED))", cmd.args
+
+      assert_equal search_result, imap.search([
+        "RETURN", %w(MIN MAX COUNT), "NOT", ["FLAGGED", %w(OR SEEN ANSWERED)]
+      ])
+      cmd = server.commands.pop
+      assert_equal "RETURN (MIN MAX COUNT) NOT (FLAGGED (OR SEEN ANSWERED))", cmd.args
     end
   end