🔀 Merge pull request #363 from ruby/search-charset-conflict-raise-argument_error

🥅 Raise ArgumentError on multiple search charset args

Author: nevans

lib/net/imap.rb

@@ -3150,9 +3150,18 @@ def enforce_logindisabled?
       end
     end
 
-    def search_internal(cmd, keys, charset = nil)
-      keys = normalize_searching_criteria(keys)
-      args = charset ? ["CHARSET", charset, *keys] : keys
+    def search_args(keys, charset = nil)
+      # NOTE: not handling combined RETURN and CHARSET for raw strings
+      if charset && keys in /\ACHARSET\b/i | Array[/\ACHARSET\z/i, *]
+        raise ArgumentError, "multiple charset arguments"
+      end
+      args = normalize_searching_criteria(keys)
+      args.prepend("CHARSET", charset)     if charset
+      args
+    end
+
+    def search_internal(cmd, ...)
+      args = search_args(...)
       synchronize do
         send_command(cmd, *args)
         search_result = clear_responses("SEARCH").last
@@ -3223,7 +3232,7 @@ def thread_internal(cmd, algorithm, search_keys, charset)
     end
 
     def normalize_searching_criteria(criteria)
-      return RawData.new(criteria) if criteria.is_a?(String)
+      return [RawData.new(criteria)] if criteria.is_a?(String)
       criteria.map {|i|
         if coerce_search_arg_to_seqset?(i)
           SequenceSet[i]

test/net/imap/test_imap.rb

@@ -1229,6 +1229,12 @@ def test_unselect
       imap.search(["subject", "hello", Set[1, 2, 3, 4, 5, 8, *(10..100)]])
       assert_equal "subject hello 1:5,8,10:100", server.commands.pop.args
 
+      imap.search('SUBJECT "Hello world"', "UTF-8")
+      assert_equal 'CHARSET UTF-8 SUBJECT "Hello world"', server.commands.pop.args
+
+      imap.search('CHARSET UTF-8 SUBJECT "Hello world"')
+      assert_equal 'CHARSET UTF-8 SUBJECT "Hello world"', server.commands.pop.args
+
       imap.search([:*])
       assert_equal "*", server.commands.pop.args
 
@@ -1259,6 +1265,24 @@ def seqset_coercible.to_sequence_set
     end
   end
 
+  test("#search/#uid_search with invalid arguments") do
+    with_fake_server do |server, imap|
+      server.on "SEARCH"     do |cmd| cmd.fail_no "should fail before this" end
+      server.on "UID SEARCH" do |cmd| cmd.fail_no "should fail before this" end
+
+      assert_raise(ArgumentError) do
+        imap.search(["charset", "foo", "ALL"], "bar")
+      end
+      assert_raise(ArgumentError) do
+        imap.search("charset foo ALL", "bar")
+      end
+      # Parsing return opts is too complicated, for now.
+      # assert_raise(ArgumentError) do
+      #   imap.search("return () charset foo ALL", "bar")
+      # end
+    end
+  end
+
   test("missing server SEARCH response") do
     with_fake_server do |server, imap|
       server.on "SEARCH",     &:done_ok