🔀 Merge pull request #616 from ruby/add-binary-append

✨ Support `BINARY` extention to `#append` (RFC3516)

Author: nevans

lib/net/imap.rb

@@ -486,9 +486,7 @@ module Net
   # IMAP4rev2[https://www.rfc-editor.org/rfc/rfc9051].
   # - Updates #fetch and #uid_fetch with the +BINARY+, +BINARY.PEEK+, and
   #   +BINARY.SIZE+ items.  See FetchData#binary and FetchData#binary_size.
-  #
-  # >>>
-  #   *NOTE:* The binary extension the #append command is _not_ supported yet.
+  # - Updates #append to allow binary messages containing +NULL+ bytes.
   #
   # ==== RFC3691: +UNSELECT+
   # Folded into IMAP4rev2[https://www.rfc-editor.org/rfc/rfc9051] and also included
@@ -2044,6 +2042,11 @@ def status(mailbox, attr)
     #
     # ==== Capabilities
     #
+    # If +BINARY+ [RFC3516[https://www.rfc-editor.org/rfc/rfc3516.html]] is
+    # supported by the server, +message+ may contain +NULL+ characters and
+    # be sent as a binary literal.  Otherwise, binary message parts must be
+    # encoded appropriately (for example, +base64+).
+    #
     # If +UIDPLUS+ [RFC4315[https://www.rfc-editor.org/rfc/rfc4315.html]] is
     # supported and the destination supports persistent UIDs, the server's
     # response should include an +APPENDUID+ response code with AppendUIDData.
@@ -2054,12 +2057,11 @@ def status(mailbox, attr)
     # TODO: add MULTIAPPEND support
     #++
     def append(mailbox, message, flags = nil, date_time = nil)
+      message = StringFormatter.literal_or_literal8(message, name: "message")
       args = []
-      if flags
-        args.push(flags)
-      end
+      args.push(flags)     if flags
       args.push(date_time) if date_time
-      args.push(Literal.new(message))
+      args.push(message)
       send_command("APPEND", mailbox, *args)
     end
 

lib/net/imap/command_data.rb

@@ -77,9 +77,12 @@ def send_quoted_string(str)
       put_string('"' + str.gsub(/["\\]/, "\\\\\\&") + '"')
     end
 
-    def send_literal(str, tag = nil)
+    def send_binary_literal(str, tag) = send_literal(str, tag, binary: true)
+
+    def send_literal(str, tag = nil, binary: false)
       synchronize do
-        put_string("{" + str.bytesize.to_s + "}" + CRLF)
+        prefix = "~" if binary
+        put_string("#{prefix}{#{str.bytesize}}\r\n")
         @continued_command_tag = tag
         @continuation_request_exception = nil
         begin
@@ -147,11 +150,35 @@ def send_data(imap, tag)
     end
 
     class Literal < CommandData # :nodoc:
+      def initialize(data:)
+        data = -String(data.to_str).b or
+          raise DataFormatError, "#{self.class} expects string input"
+        super
+        validate
+      end
+
+      def bytesize = data.bytesize
+
+      def validate
+        if data.include?("\0")
+          raise DataFormatError, "NULL byte not allowed in #{self.class}.  " \
+            "Use #{Literal8} or a null-safe encoding."
+        end
+      end
+
       def send_data(imap, tag)
         imap.__send__(:send_literal, data, tag)
       end
     end
 
+    class Literal8 < Literal # :nodoc:
+      def validate = nil # all bytes are okay
+
+      def send_data(imap, tag)
+        imap.__send__(:send_binary_literal, data, tag)
+      end
+    end
+
     class PartialRange < CommandData # :nodoc:
       uint32_max = 2**32 - 1
       POS_RANGE = 1..uint32_max
@@ -221,6 +248,14 @@ module StringFormatter
 
       module_function
 
+      def literal_or_literal8(input, name: "argument")
+        return input if input in Literal | Literal8
+        data = String.try_convert(input) \
+          or raise TypeError, "expected #{name} to be String, got #{input.class}"
+        type = data.include?("\0") ? Literal8 : Literal
+        type.new(data:)
+      end
+
       # Allows symbols in addition to strings
       def valid_string?(str)
         str.is_a?(Symbol) || str.respond_to?(:to_str)

test/net/imap/test_command_data.rb

@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+require "net/imap"
+require "test/unit"
+
+class CommandDataTest < Net::IMAP::TestCase
+  DataFormatError = Net::IMAP::DataFormatError
+
+  Literal = Net::IMAP::Literal
+  Literal8 = Net::IMAP::Literal8
+
+  Output = Data.define(:name, :args)
+  TAG = Module.new.freeze
+
+  class FakeCommandWriter
+    def self.def_printer(name)
+      unless Net::IMAP.instance_methods.include?(name) ||
+          Net::IMAP.private_instance_methods.include?(name)
+        raise NoMethodError, "#{name} is not a method on Net::IMAP"
+      end
+      define_method(name) do |*args|
+        output << Output[name:, args:]
+      end
+      Output.define_singleton_method(name) do |*args|
+        new(name:, args:)
+      end
+    end
+
+    attr_reader :output
+
+    def initialize
+      @output = []
+    end
+
+    def clear = @output.clear
+    def validate(*data) = data.each(&:validate)
+    def send_data(*data, tag: TAG)
+      validate(*data)
+      data.each do _1.send_data(self, tag) end
+    end
+
+    def_printer :put_string
+    def_printer :send_string_data
+    def_printer :send_number_data
+    def_printer :send_list_data
+    def_printer :send_time_data
+    def_printer :send_date_data
+    def_printer :send_quoted_string
+    def_printer :send_literal
+    def_printer :send_binary_literal
+  end
+
+  test "Literal" do
+    imap = FakeCommandWriter.new
+    imap.send_data Literal["foo\r\nbar"]
+    assert_equal [
+      Output.send_literal("foo\r\nbar", TAG),
+    ], imap.output
+
+    imap.clear
+    assert_raise_with_message(Net::IMAP::DataFormatError, /\bNULL byte\b/i) do
+      imap.send_data Literal["contains NULL char: \0"]
+    end
+    assert_empty imap.output
+  end
+
+  test "Literal8" do
+    imap = FakeCommandWriter.new
+    imap.send_data Literal8["foo\r\nbar"], Literal8["foo\0bar"]
+    assert_equal [
+      Output.send_binary_literal("foo\r\nbar", TAG),
+      Output.send_binary_literal("foo\0bar", TAG),
+    ], imap.output
+  end
+
+  class StringFormatterTest < Net::IMAP::TestCase
+    include Net::IMAP::StringFormatter
+
+    test "literal_or_literal8" do
+      assert_kind_of Literal,  literal_or_literal8("simple\r\n")
+      assert_kind_of Literal8, literal_or_literal8("has NULL \0")
+      assert_kind_of Literal,  literal_or_literal8(Literal["foo"])
+      assert_kind_of Literal8, literal_or_literal8(Literal8["foo"])
+    end
+  end
+
+end

test/net/imap/test_imap.rb

@@ -736,93 +736,6 @@ def test_disconnect
     end
   end
 
-  def test_append
-    server = create_tcp_server
-    port = server.addr[1]
-    mail = <<EOF.gsub(/\n/, "\r\n")
-From: shugo@example.com
-To: matz@example.com
-Subject: hello
-
-hello world
-EOF
-    requests = []
-    received_mail = nil
-    start_server do
-      sock = server.accept
-      begin
-        sock.print("* OK test server\r\n")
-        line = sock.gets
-        requests.push(line)
-        size = line.slice(/{(\d+)}\r\n/, 1).to_i
-        sock.print("+ Ready for literal data\r\n")
-        received_mail = sock.read(size)
-        sock.gets
-        sock.print("RUBY0001 OK APPEND completed\r\n")
-        requests.push(sock.gets)
-        sock.print("* BYE terminating connection\r\n")
-        sock.print("RUBY0002 OK LOGOUT completed\r\n")
-      ensure
-        sock.close
-        server.close
-      end
-    end
-
-    begin
-      imap = Net::IMAP.new(server_addr, :port => port)
-      imap.append("INBOX", mail)
-      assert_equal(1, requests.length)
-      assert_equal("RUBY0001 APPEND INBOX {#{mail.size}}\r\n", requests[0])
-      assert_equal(mail, received_mail)
-      imap.logout
-      assert_equal(2, requests.length)
-      assert_equal("RUBY0002 LOGOUT\r\n", requests[1])
-    ensure
-      imap.disconnect if imap
-    end
-  end
-
-  def test_append_fail
-    server = create_tcp_server
-    port = server.addr[1]
-    mail = <<EOF.gsub(/\n/, "\r\n")
-From: shugo@example.com
-To: matz@example.com
-Subject: hello
-
-hello world
-EOF
-    requests = []
-    start_server do
-      sock = server.accept
-      begin
-        sock.print("* OK test server\r\n")
-        requests.push(sock.gets)
-        sock.print("RUBY0001 NO Mailbox doesn't exist\r\n")
-        requests.push(sock.gets)
-        sock.print("* BYE terminating connection\r\n")
-        sock.print("RUBY0002 OK LOGOUT completed\r\n")
-      ensure
-        sock.close
-        server.close
-      end
-    end
-
-    begin
-      imap = Net::IMAP.new(server_addr, :port => port)
-      assert_raise(Net::IMAP::NoResponseError) do
-        imap.append("INBOX", mail)
-      end
-      assert_equal(1, requests.length)
-      assert_equal("RUBY0001 APPEND INBOX {#{mail.size}}\r\n", requests[0])
-      imap.logout
-      assert_equal(2, requests.length)
-      assert_equal("RUBY0002 LOGOUT\r\n", requests[1])
-    ensure
-      imap.disconnect if imap
-    end
-  end
-
   def test_id
     server = create_tcp_server
     port = server.addr[1]