From 016d8dfb616e0ecd54c5e71f4db4d6510ab31c66 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 4 Jul 2026 16:02:43 -0400 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=85=20Add=20better=20test=20coverage?= =?UTF-8?q?=20for=20string=20arguments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We were missing any basic tests for sending UTF-8 strings! --- test/net/imap/test_imap.rb | 73 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 2140d5e1..4ecb5724 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -921,6 +921,79 @@ def test_raw_data end end + test "sending nil args" do + with_fake_server do |server, imap| + server.on "TEST", &:done_ok + def imap.test_args(*args) = send_command("TEST", *args) + + imap.test_args nil, [nil] + assert_equal "NIL (NIL)", server.commands.pop.args + end + end + + test "sending atom string args (astring-chars)" do + with_fake_server do |server, imap| + server.on "TEST", &:done_ok + def imap.test_args(*args) = send_command("TEST", *args) + + imap.test_args "valid-atoms", %w[foo=bar $baz] + assert_equal "valid-atoms (foo=bar $baz)", server.commands.pop.args + + imap.test_args "unquoted-astring", "[resp-specials]" + assert_equal "unquoted-astring [resp-specials]", server.commands.pop.args + end + end + + test "sending quoted string args" do + with_fake_server do |server, imap| + server.on "TEST", &:done_ok + def imap.test_args(*args) = send_command("TEST", *args) + + imap.test_args "empty", "", [""] + assert_equal 'empty "" ("")', server.commands.pop.args + + imap.test_args "simple-quotable-specials", "() {} %*" + assert_equal('simple-quotable-specials "() {} %*"'.b, + server.commands.pop.args) + + imap.test_args "ascii-ctrl-chars", "\b\x7f" + assert_equal("ascii-ctrl-chars \"\b\x7f\"".b, server.commands.pop.args) + + imap.test_args "quoted-specials", ["backslash=\\", 'dquotes=""'] + assert_equal('quoted-specials ("backslash=\\\\" "dquotes=\\"\\"")'.b, + server.commands.pop.args) + end + end + + test "sending UTF-8 string args" do + with_fake_server( + with_extensions: %w[UTF8=ACCEPT LITERAL-], + greeting_capabilities: true, + capabilities_enablable: %w[UTF8=ACCEPT], + ) do |server, imap| + server.on "TEST", &:done_ok + def imap.test_args(*args) = send_command("TEST", *args) + + # Before enabling UTF-8 strings, with non-synchronizing literals + imap.test_args "sync-literal-utf8", ["αβγδε"] + assert_equal("sync-literal-utf8 ({10+}\r\nαβγδε)".b, + server.commands.pop.args) + + # Before enabling UTF-8 strings, without non-synchronizing literals + imap.config.max_non_synchronizing_literal = -1 + imap.test_args "sync-literal-utf8", ["αβγδε"] + assert_equal("sync-literal-utf8 ({10}\r\nαβγδε)".b, + server.commands.pop.args) + + # After enabling UTF-8 strings + imap.enable(:utf8) + server.commands.pop.args => ["UTF8=ACCEPT"] + + imap.test_args "quoted-utf8", "αβγδε" + assert_equal 'quoted-utf8 "αβγδε"'.b, server.commands.pop.args + end + end + test("send literal args") do with_fake_server(with_extensions: %w[LITERAL-]) do |server, imap| # disable automatic non-synchronizing literals From b239ea19e8c915b36c385a2a146762faba10ed94 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sun, 5 Jul 2026 18:18:24 -0400 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9D=20Clarify=20atom=20vs=20astrin?= =?UTF-8?q?g=20in=20internal=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also dropped some redundant comments --- lib/net/imap/command_data.rb | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index e46125b9..61ef7206 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -55,24 +55,28 @@ def send_data(data, tag = nil) end end + # Sends generic strings formatted as +astring+: + # * as an atom (note: this is based on +ASTRING-CHAR+, not +ATOM-CHAR+) + # * as a quoted string + # * as a literal (may send non-synchronizing) + # + # NOTE: This does not validate that +str+ contains no +NULL+ bytes. def send_string_data(str, tag = nil) if str.empty? + # same as send_quoted_string(str), but incompatible encoding is allowed put_string('""') elsif str.match?(/[\r\n]/n) - # literal, because multiline send_literal(str, tag) elsif !str.ascii_only? if @utf8_strings - # quoted string send_quoted_string(str) else - # literal, because of non-ASCII bytes send_literal(str, tag) end elsif str.match?(/[(){ \x00-\x1f\x7f%*"\\]/n) - # quoted string send_quoted_string(str) else + # valid +astring+ atom: non-empty, ASCII only, no ASTRING_SPECIALS put_string(str) end end @@ -273,6 +277,17 @@ def self.extract_literal(data, binary:, bytesize:, non_sync:) private_class_method :extract_literal end + # Please note that +ATOM-CHAR+ and +atom+ are not the same as +ASTRING-char+ + # and the atom form of +astring+. +astring+ allows ], but +atom+ + # does not. + # + # Generic string arguments in Net::IMAP use +astring+, so they will send as + # atoms even if they contain ]. + # + # This class is used by: + # * to validate generic symbol arguments, as the superclass of Flag + # * to validate #enable +capabilities+ + # * to validate #store (and #uid_store) +attr+ class Atom < CommandData # :nodoc: def initialize(**) super From a742a1099fd4689d4c8a52db4a3d2ab1be10ae8c Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 6 Jul 2026 10:44:32 -0400 Subject: [PATCH 3/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20regexp=20con?= =?UTF-8?q?stants=20in=20send=5Fstring=5Fdata?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are identical regexps, make the code better self-documenting. Note that ASTRING_SPECIALS is identical to ResponseParser::Patterns::ASTRING_SPECIALS. --- lib/net/imap/command_data.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index 61ef7206..eda03866 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -55,6 +55,10 @@ def send_data(data, tag = nil) end end + UNQUOTABLE_CHARS = /\r\n/n + ASTRING_SPECIALS = /[(){ \x00-\x1f\x7f%*"\\]/n + private_constant :UNQUOTABLE_CHARS, :ASTRING_SPECIALS + # Sends generic strings formatted as +astring+: # * as an atom (note: this is based on +ASTRING-CHAR+, not +ATOM-CHAR+) # * as a quoted string @@ -65,7 +69,7 @@ def send_string_data(str, tag = nil) if str.empty? # same as send_quoted_string(str), but incompatible encoding is allowed put_string('""') - elsif str.match?(/[\r\n]/n) + elsif str.match?(UNQUOTABLE_CHARS) send_literal(str, tag) elsif !str.ascii_only? if @utf8_strings @@ -73,7 +77,7 @@ def send_string_data(str, tag = nil) else send_literal(str, tag) end - elsif str.match?(/[(){ \x00-\x1f\x7f%*"\\]/n) + elsif str.match?(ASTRING_SPECIALS) send_quoted_string(str) else # valid +astring+ atom: non-empty, ASCII only, no ASTRING_SPECIALS From 6a4dc11ddb99be091873355d017aa562a47d8ac1 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 8 Jul 2026 13:51:27 -0400 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A5=85=20Test=20for=20NULL=20bytes=20?= =?UTF-8?q?before=20sending=20string=20args?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to raise the exception during argument validation, rather than waiting until we are sending the command (which is too late to save the connection from being corrupted). --- lib/net/imap/command_data.rb | 5 ++++- test/net/imap/test_imap.rb | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lib/net/imap/command_data.rb b/lib/net/imap/command_data.rb index eda03866..f7f22955 100644 --- a/lib/net/imap/command_data.rb +++ b/lib/net/imap/command_data.rb @@ -15,6 +15,9 @@ def validate_data(data) case data when nil when String + if data.include?("\0") + raise DataFormatError, "String argument contains NULL byte" + end when Integer # Covers modseq-valzer, which is the largest valid IMAP integer if data.negative? @@ -55,7 +58,7 @@ def send_data(data, tag = nil) end end - UNQUOTABLE_CHARS = /\r\n/n + UNQUOTABLE_CHARS = /\0\r\n/n ASTRING_SPECIALS = /[(){ \x00-\x1f\x7f%*"\\]/n private_constant :UNQUOTABLE_CHARS, :ASTRING_SPECIALS diff --git a/test/net/imap/test_imap.rb b/test/net/imap/test_imap.rb index 4ecb5724..6bd9aada 100644 --- a/test/net/imap/test_imap.rb +++ b/test/net/imap/test_imap.rb @@ -944,6 +944,21 @@ def imap.test_args(*args) = send_command("TEST", *args) end end + test "string args don't allow NULL bytes" do + with_fake_server do |server, imap| + server.on "TEST", &:done_ok + def imap.test_args(*args) = send_command("TEST", *args) + + assert_raise_with_message(Net::IMAP::DataFormatError, /NULL byte/) do + imap.test_args "NULL=\0" + end + + assert_raise_with_message(Net::IMAP::DataFormatError, /NULL byte/) do + imap.test_args ["ok", "also ok", "not ok: \0"] + end + end + end + test "sending quoted string args" do with_fake_server do |server, imap| server.on "TEST", &:done_ok