From 21ef44cd4664bf79b3e32b62db3aa1ae8af5c4ed Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 16:45:05 -0500 Subject: [PATCH 01/36] =?UTF-8?q?=E2=AC=86=EF=B8=8F=20Bump=20step-security?= =?UTF-8?q?/harden-runner=20from=202.10.4=20to=202.11.0=20(#409)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.10.4 to 2.11.0. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](https://github.com/step-security/harden-runner/compare/cb605e52c26070c328afc4562f0b4ada7618a84e...4d991eb9b905ef189e4c376166672c3f2f230481) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/push_gem.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push_gem.yml b/.github/workflows/push_gem.yml index a085de5cd..68cad07b6 100644 --- a/.github/workflows/push_gem.yml +++ b/.github/workflows/push_gem.yml @@ -24,7 +24,7 @@ jobs: steps: # Set up - name: Harden Runner - uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4 + uses: step-security/harden-runner@4d991eb9b905ef189e4c376166672c3f2f230481 # v2.11.0 with: egress-policy: audit From d10881e9a07efdab138bd3ad34a1bb0bbd34aacc Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 18 Feb 2025 18:35:58 -0500 Subject: [PATCH 02/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Update=20versioned?= =?UTF-8?q?=20default=20configs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reorganizes the creation of `Config.version_defaults` to make it easier to cherry-pick backported config changes to stable branches: * Sets `Config[0.5]` as a diff from the previous versioned default, just like the other versioned defaults. * Sets `Config[:current] = Config[VERSION.to_f]`, so it will not need to be updated for any x.y.0 release. Likewise, sets `Config[:next]` and `Config[:future]` to `Config[VERSION.to_f + 0.1]` or `+ 0.2`, so they which will only rarely need to be changed. * Because `Config[:default]` and `Config[:current]` are now derived two different ways, both a warning and a test have been added to ensure they remain synchronized. * A few other `Config.version_defaults` tests were added or updated. --- lib/net/imap/config.rb | 28 +++++++++++++++++++++++----- test/net/imap/test_config.rb | 34 +++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 1e0300c5d..0e682c8e1 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -435,9 +435,8 @@ def defaults_hash @global = default.new version_defaults[:default] = Config[default.send(:defaults_hash)] - version_defaults[:current] = Config[:default] - version_defaults[0] = Config[:current].dup.update( + version_defaults[0] = Config[:default].dup.update( sasl_ir: false, responses_without_block: :silence_deprecation_warning, enforce_logindisabled: false, @@ -454,17 +453,36 @@ def defaults_hash parser_max_deprecated_uidplus_data_size: 1000, ).freeze - version_defaults[0.5] = Config[:current] + version_defaults[0.5] = Config[0.4].dup.update( + enforce_logindisabled: true, + responses_without_block: :warn, + parser_use_deprecated_uidplus_data: :up_to_max_size, + parser_max_deprecated_uidplus_data_size: 100, + ).freeze version_defaults[0.6] = Config[0.5].dup.update( responses_without_block: :frozen_dup, parser_use_deprecated_uidplus_data: false, parser_max_deprecated_uidplus_data_size: 0, ).freeze - version_defaults[:next] = Config[0.6] - version_defaults[:future] = Config[:next] + + version_defaults[0.7] = Config[0.6].dup.update( + ).freeze + + current = VERSION.to_f + version_defaults[:original] = Config[0] + version_defaults[:current] = Config[current] + version_defaults[:next] = Config[current + 0.1] + version_defaults[:future] = Config[current + 0.2] version_defaults.freeze + + if ($VERBOSE || $DEBUG) && self[:current].to_h != self[:default].to_h + warn "Misconfigured Net::IMAP::Config[:current] => %p,\n" \ + " not equal to Net::IMAP::Config[:default] => %p" % [ + self[:current].to_h, self[:default].to_h + ] + end end end end diff --git a/test/net/imap/test_config.rb b/test/net/imap/test_config.rb index fa11c74a0..2f5981699 100644 --- a/test/net/imap/test_config.rb +++ b/test/net/imap/test_config.rb @@ -5,6 +5,9 @@ class ConfigTest < Test::Unit::TestCase Config = Net::IMAP::Config + THIS_VERSION = Net::IMAP::VERSION.to_f + NEXT_VERSION = THIS_VERSION + 0.1 + FUTURE_VERSION = THIS_VERSION + 0.2 setup do Config.global.reset @@ -141,10 +144,26 @@ class ConfigTest < Test::Unit::TestCase assert_kind_of Config, config assert config.frozen?, "#{name} isn't frozen" assert config.inherited?(:debug), "#{name} doesn't inherit debug" + keys = config.to_h.keys - [:debug] + keys.each do |key| + refute config.inherited?(key) + end assert_same Config.global, config.parent end end + test "Config[:default] and Config[:current] both hold default config" do + defaults = Config.default.to_h + assert_equal(defaults, Config[:default].to_h) + assert_equal(defaults, Config[:current].to_h) + end + + test ".[] for all version_defaults" do + Config.version_defaults.each do |version, config| + assert_same Config[version], config + end + end + test ".[] for all x.y versions" do original = Config[0] assert_kind_of Config, original @@ -152,8 +171,9 @@ class ConfigTest < Test::Unit::TestCase assert_same original, Config[0.1] assert_same original, Config[0.2] assert_same original, Config[0.3] - assert_kind_of Config, Config[0.4] - assert_kind_of Config, Config[0.5] + ((0.4r..FUTURE_VERSION.to_r) % 0.1r).each do |version| + assert_kind_of Config, Config[version.to_f] + end end test ".[] range errors" do @@ -169,10 +189,10 @@ class ConfigTest < Test::Unit::TestCase end test ".[] with symbol names" do - assert_same Config[0.5], Config[:current] - assert_same Config[0.5], Config[:default] - assert_same Config[0.6], Config[:next] - assert_kind_of Config, Config[:future] + assert_equal Config[THIS_VERSION].to_h, Config[:default].to_h + assert_same Config[THIS_VERSION], Config[:current] + assert_same Config[NEXT_VERSION], Config[:next] + assert_same Config[FUTURE_VERSION], Config[:future] end test ".[] with a hash" do @@ -190,7 +210,7 @@ class ConfigTest < Test::Unit::TestCase assert_same Config.default, Config.new(Config.default).parent assert_same Config.global, Config.new(Config.global).parent assert_same Config[0.4], Config.new(0.4).parent - assert_same Config[0.6], Config.new(:next).parent + assert_same Config[NEXT_VERSION], Config.new(:next).parent assert_equal true, Config.new({debug: true}, debug: false).parent.debug? assert_equal true, Config.new({debug: true}, debug: false).parent.frozen? end From 78d46f24df740fd085cec110349f27156dd92426 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 6 Mar 2025 16:09:37 -0500 Subject: [PATCH 03/36] =?UTF-8?q?=E2=9C=85=20Make=20FakeServer=20more=20ro?= =?UTF-8?q?bust=20against=20disconnect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the client disconnects without a `LOGOUT`, that shouldn't cause a secondary error in the test output. Most of the time, it's just a distraction. In some cases, the test is intentionally triggering a bad disconnect, and the server's failure could break the test. --- test/net/imap/fake_server/command_reader.rb | 1 + test/net/imap/fake_server/connection.rb | 4 +++- test/net/imap/fake_server/test_helper.rb | 6 +++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/test/net/imap/fake_server/command_reader.rb b/test/net/imap/fake_server/command_reader.rb index b840384f5..b3b6ce172 100644 --- a/test/net/imap/fake_server/command_reader.rb +++ b/test/net/imap/fake_server/command_reader.rb @@ -21,6 +21,7 @@ def get_command $2 or socket.print "+ Continue\r\n" buf << socket.read(Integer($1)) end + throw :eof if buf.empty? @last_command = parse(buf) end diff --git a/test/net/imap/fake_server/connection.rb b/test/net/imap/fake_server/connection.rb index d8813bfd5..711ab6cc1 100644 --- a/test/net/imap/fake_server/connection.rb +++ b/test/net/imap/fake_server/connection.rb @@ -23,7 +23,9 @@ def unsolicited(...) writer.untagged(...) end def run writer.greeting - router << reader.get_command until state.logout? + catch(:eof) do + router << reader.get_command until state.logout? + end ensure close end diff --git a/test/net/imap/fake_server/test_helper.rb b/test/net/imap/fake_server/test_helper.rb index 1987d048f..4c7f63929 100644 --- a/test/net/imap/fake_server/test_helper.rb +++ b/test/net/imap/fake_server/test_helper.rb @@ -14,7 +14,11 @@ def run_fake_server_in_thread(ignore_io_error: false, timeout: 10, **opts) end yield server ensure - server&.shutdown + begin + server&.shutdown + rescue IOError + raise unless ignore_io_error + end end end From 647cc21aefbb3942302e9b7da0845f0dfab4034d Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 11 Mar 2025 11:56:21 -0400 Subject: [PATCH 04/36] =?UTF-8?q?=E2=9C=85=20Avoid=20warning=20on=20redefi?= =?UTF-8?q?ning=20FakeServer=20handlers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/command_router.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/net/imap/fake_server/command_router.rb b/test/net/imap/fake_server/command_router.rb index 82430316d..6865112ca 100644 --- a/test/net/imap/fake_server/command_router.rb +++ b/test/net/imap/fake_server/command_router.rb @@ -10,7 +10,9 @@ module Routable def on(*command_names, &handler) scope = self.is_a?(Module) ? self : singleton_class command_names.each do |command_name| - scope.define_method("handle_#{command_name.downcase}", &handler) + method_name = :"handle_#{command_name.downcase}" + scope.undef_method(method_name) if scope.method_defined?(method_name) + scope.define_method(method_name, &handler) end end end From b55b945216d9c04c6e485a75623524e8e974e5bd Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 11 Mar 2025 11:57:18 -0400 Subject: [PATCH 05/36] =?UTF-8?q?=E2=9C=85=20Change=20FakeServer=20parse?= =?UTF-8?q?=20error=20to=20IOError=20on=20EOF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/command_reader.rb | 5 ++++- test/net/imap/fake_server/socket.rb | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/net/imap/fake_server/command_reader.rb b/test/net/imap/fake_server/command_reader.rb index b840384f5..a38a80627 100644 --- a/test/net/imap/fake_server/command_reader.rb +++ b/test/net/imap/fake_server/command_reader.rb @@ -3,6 +3,7 @@ require "net/imap" class Net::IMAP::FakeServer + CommandParseError = RuntimeError class CommandReader attr_reader :last_command @@ -22,6 +23,8 @@ def get_command buf << socket.read(Integer($1)) end @last_command = parse(buf) + rescue CommandParseError => err + raise IOError, err.message if socket.eof? && !buf.end_with?("\r\n") end private @@ -31,7 +34,7 @@ def get_command # TODO: convert bad command exception to tagged BAD response, when possible def parse(buf) /\A([^ ]+) ((?:UID )?\w+)(?: (.+))?\r\n\z/min =~ buf or - raise "bad request: %p" [buf] + raise CommandParseError, "bad request: %p" [buf] case $2.upcase when "LOGIN", "SELECT", "EXAMINE", "ENABLE", "AUTHENTICATE" Command.new $1, $2, scan_astrings($3), buf diff --git a/test/net/imap/fake_server/socket.rb b/test/net/imap/fake_server/socket.rb index 21795aa59..65593d86f 100644 --- a/test/net/imap/fake_server/socket.rb +++ b/test/net/imap/fake_server/socket.rb @@ -18,6 +18,7 @@ def initialize(tcp_socket, config:) def tls?; !!@tls_socket end def closed?; @closed end + def eof?; socket.eof? end def gets(...) socket.gets(...) end def read(...) socket.read(...) end def print(...) socket.print(...) end From 84e75766affb9a7b34d0016b739183863c503ee1 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 11 Mar 2025 12:02:27 -0400 Subject: [PATCH 06/36] =?UTF-8?q?=E2=9C=85=20Configurable=20report=5Fon=5F?= =?UTF-8?q?exception=20for=20FakeServer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/test_helper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/net/imap/fake_server/test_helper.rb b/test/net/imap/fake_server/test_helper.rb index 1987d048f..78cf5b0bb 100644 --- a/test/net/imap/fake_server/test_helper.rb +++ b/test/net/imap/fake_server/test_helper.rb @@ -4,10 +4,14 @@ module Net::IMAP::FakeServer::TestHelper - def run_fake_server_in_thread(ignore_io_error: false, timeout: 10, **opts) + def run_fake_server_in_thread(ignore_io_error: false, + report_on_exception: true, + timeout: 10, **opts) Timeout.timeout(timeout) do server = Net::IMAP::FakeServer.new(timeout: timeout, **opts) @threads << Thread.new do + Thread.current.abort_on_exception = false + Thread.current.report_on_exception = report_on_exception server.run rescue IOError raise unless ignore_io_error From 487f1104b6711f1819935ca517e1fa3815815405 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 7 Mar 2025 10:41:42 -0500 Subject: [PATCH 07/36] =?UTF-8?q?=F0=9F=93=9A=20Document=20connection=20st?= =?UTF-8?q?ate=20more=20consistently?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because a `#connection_state` attribute will be added, I'd like to consistently name the connection states everywhere they are used. --- lib/net/imap.rb | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 1729f0b7b..2afc5c226 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -43,10 +43,16 @@ module Net # To work on the messages within a mailbox, the client must # first select that mailbox, using either #select or #examine # (for read-only access). Once the client has successfully - # selected a mailbox, they enter the "_selected_" state, and that + # selected a mailbox, they enter the +selected+ state, and that # mailbox becomes the _current_ mailbox, on which mail-item # related commands implicitly operate. # + # === Connection state + # + # Once an IMAP connection is established, the connection is in one of four + # states: not authenticated, +authenticated+, +selected+, and + # +logout+. Most commands are valid only in certain states. + # # === Sequence numbers and UIDs # # Messages have two sorts of identifiers: message sequence @@ -260,8 +266,9 @@ module Net # # - Net::IMAP.new: Creates a new \IMAP client which connects immediately and # waits for a successful server greeting before the method returns. + # - #connection_state: Returns the connection state. # - #starttls: Asks the server to upgrade a clear-text connection to use TLS. - # - #logout: Tells the server to end the session. Enters the "_logout_" state. + # - #logout: Tells the server to end the session. Enters the +logout+ state. # - #disconnect: Disconnects the connection (without sending #logout first). # - #disconnected?: True if the connection has been closed. # @@ -317,37 +324,36 @@ module Net # In general, #capable? should be used rather than explicitly sending a # +CAPABILITY+ command to the server. # - #noop: Allows the server to send unsolicited untagged #responses. - # - #logout: Tells the server to end the session. Enters the "_logout_" state. + # - #logout: Tells the server to end the session. Enters the +logout+ state. # # ==== Not Authenticated state # # In addition to the commands for any state, the following commands are valid - # in the "not authenticated" state: + # in the +not_authenticated+ state: # # - #starttls: Upgrades a clear-text connection to use TLS. # # Requires the +STARTTLS+ capability. # - #authenticate: Identifies the client to the server using the given # {SASL mechanism}[https://www.iana.org/assignments/sasl-mechanisms/sasl-mechanisms.xhtml] - # and credentials. Enters the "_authenticated_" state. + # and credentials. Enters the +authenticated+ state. # # The server should list "AUTH=#{mechanism}" capabilities for # supported mechanisms. # - #login: Identifies the client to the server using a plain text password. - # Using #authenticate is generally preferred. Enters the "_authenticated_" - # state. + # Using #authenticate is preferred. Enters the +authenticated+ state. # # The +LOGINDISABLED+ capability must NOT be listed. # # ==== Authenticated state # # In addition to the commands for any state, the following commands are valid - # in the "_authenticated_" state: + # in the +authenticated+ state: # # - #enable: Enables backwards incompatible server extensions. # Requires the +ENABLE+ or +IMAP4rev2+ capability. - # - #select: Open a mailbox and enter the "_selected_" state. - # - #examine: Open a mailbox read-only, and enter the "_selected_" state. + # - #select: Open a mailbox and enter the +selected+ state. + # - #examine: Open a mailbox read-only, and enter the +selected+ state. # - #create: Creates a new mailbox. # - #delete: Permanently remove a mailbox. # - #rename: Change the name of a mailbox. @@ -369,12 +375,12 @@ module Net # # ==== Selected state # - # In addition to the commands for any state and the "_authenticated_" - # commands, the following commands are valid in the "_selected_" state: + # In addition to the commands for any state and the +authenticated+ + # commands, the following commands are valid in the +selected+ state: # - # - #close: Closes the mailbox and returns to the "_authenticated_" state, + # - #close: Closes the mailbox and returns to the +authenticated+ state, # expunging deleted messages, unless the mailbox was opened as read-only. - # - #unselect: Closes the mailbox and returns to the "_authenticated_" state, + # - #unselect: Closes the mailbox and returns to the +authenticated+ state, # without expunging any messages. # Requires the +UNSELECT+ or +IMAP4rev2+ capability. # - #expunge: Permanently removes messages which have the Deleted flag set. @@ -395,7 +401,7 @@ module Net # # ==== Logout state # - # No \IMAP commands are valid in the "_logout_" state. If the socket is still + # No \IMAP commands are valid in the +logout+ state. If the socket is still # open, Net::IMAP will close it after receiving server confirmation. # Exceptions will be raised by \IMAP commands that have already started and # are waiting for a response, as well as any that are called after logout. @@ -449,7 +455,7 @@ module Net # ==== RFC3691: +UNSELECT+ # Folded into IMAP4rev2[https://www.rfc-editor.org/rfc/rfc9051] and also included # above with {Core IMAP commands}[rdoc-ref:Net::IMAP@Core+IMAP+commands]. - # - #unselect: Closes the mailbox and returns to the "_authenticated_" state, + # - #unselect: Closes the mailbox and returns to the +authenticated+ state, # without expunging any messages. # # ==== RFC4314: +ACL+ From 518ceb308b72d5f620744d4a2ecae6f2bb0f16f9 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 27 Feb 2025 18:25:35 -0500 Subject: [PATCH 08/36] =?UTF-8?q?=E2=9C=A8=20Track=20IMAP=20connection=20s?= =?UTF-8?q?tate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tracking connection state can have many uses. But in this version, we're only tracking the session state and exposing a `#connection_state` attribute reader. For forward-compatibility, the only documented methods on the connection state objects are (currently) `#name` and `#to_sym`, which will return one of `not_authenticated`, `authenticated`, `selected`, or `logout`. From RFC9051: ``` +----------------------+ |connection established| +----------------------+ || \/ +--------------------------------------+ | server greeting | +--------------------------------------+ || (1) || (2) || (3) \/ || || +-----------------+ || || |Not Authenticated| || || +-----------------+ || || || (7) || (4) || || || \/ \/ || || +----------------+ || || | Authenticated |<=++ || || +----------------+ || || || || (7) || (5) || (6) || || || \/ || || || || +--------+ || || || || |Selected|==++ || || || +--------+ || || || || (7) || \/ \/ \/ \/ +--------------------------------------+ | Logout | +--------------------------------------+ || \/ +-------------------------------+ |both sides close the connection| +-------------------------------+ ``` > Legend for the above diagram: > > (1) connection without pre-authentication (OK greeting) > (2) pre-authenticated connection (PREAUTH greeting) > (3) rejected connection (BYE greeting) > (4) successful LOGIN or AUTHENTICATE command > (5) successful SELECT or EXAMINE command > (6) CLOSE or UNSELECT command, unsolicited CLOSED response code, or > failed SELECT or EXAMINE command > (7) LOGOUT command, server shutdown, or connection closed Before the server greeting, the state is `not_authenticated`. After the connection closes, the state remains `logout`. --- lib/net/imap.rb | 119 ++++++++++- lib/net/imap/connection_state.rb | 48 +++++ test/net/imap/test_imap_connection_state.rb | 226 ++++++++++++++++++++ 3 files changed, 386 insertions(+), 7 deletions(-) create mode 100644 lib/net/imap/connection_state.rb create mode 100644 test/net/imap/test_imap_connection_state.rb diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 2afc5c226..50566d064 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -53,6 +53,8 @@ module Net # states: not authenticated, +authenticated+, +selected+, and # +logout+. Most commands are valid only in certain states. # + # See #connection_state. + # # === Sequence numbers and UIDs # # Messages have two sorts of identifiers: message sequence @@ -758,9 +760,10 @@ class IMAP < Protocol "UTF8=ONLY" => "UTF8=ACCEPT", }.freeze - autoload :SASL, File.expand_path("imap/sasl", __dir__) - autoload :SASLAdapter, File.expand_path("imap/sasl_adapter", __dir__) - autoload :StringPrep, File.expand_path("imap/stringprep", __dir__) + autoload :ConnectionState, File.expand_path("imap/connection_state", __dir__) + autoload :SASL, File.expand_path("imap/sasl", __dir__) + autoload :SASLAdapter, File.expand_path("imap/sasl_adapter", __dir__) + autoload :StringPrep, File.expand_path("imap/stringprep", __dir__) include MonitorMixin if defined?(OpenSSL::SSL) @@ -833,6 +836,67 @@ def idle_response_timeout; config.idle_response_timeout end # Returns +false+ for a plaintext connection. attr_reader :ssl_ctx_params + # Returns the current connection state. + # + # Once an IMAP connection is established, the connection is in one of four + # states: +not_authenticated+, +authenticated+, +selected+, and +logout+. + # Most commands are valid only in certain states. + # + # The connection state object responds to +to_sym+ and +name+ with the name + # of the current connection state, as a Symbol or String. Future versions + # of +net-imap+ may store additional information on the state object. + # + # From {RFC9051}[https://www.rfc-editor.org/rfc/rfc9051#section-3]: + # +----------------------+ + # |connection established| + # +----------------------+ + # || + # \/ + # +--------------------------------------+ + # | server greeting | + # +--------------------------------------+ + # || (1) || (2) || (3) + # \/ || || + # +-----------------+ || || + # |Not Authenticated| || || + # +-----------------+ || || + # || (7) || (4) || || + # || \/ \/ || + # || +----------------+ || + # || | Authenticated |<=++ || + # || +----------------+ || || + # || || (7) || (5) || (6) || + # || || \/ || || + # || || +--------+ || || + # || || |Selected|==++ || + # || || +--------+ || + # || || || (7) || + # \/ \/ \/ \/ + # +--------------------------------------+ + # | Logout | + # +--------------------------------------+ + # || + # \/ + # +-------------------------------+ + # |both sides close the connection| + # +-------------------------------+ + # + # >>> + # Legend for the above diagram: + # + # 1. connection without pre-authentication (+OK+ #greeting) + # 2. pre-authenticated connection (+PREAUTH+ #greeting) + # 3. rejected connection (+BYE+ #greeting) + # 4. successful #login or #authenticate command + # 5. successful #select or #examine command + # 6. #close or #unselect command, unsolicited +CLOSED+ response code, or + # failed #select or #examine command + # 7. #logout command, server shutdown, or connection closed + # + # Before the server greeting, the state is +not_authenticated+. + # After the connection closes, the state remains +logout+. + attr_reader :connection_state + # Creates a new Net::IMAP object and connects it to the specified # +host+. # @@ -952,6 +1016,8 @@ def initialize(host, port: nil, ssl: nil, @exception = nil @greeting = nil @capabilities = nil + @tls_verified = false + @connection_state = ConnectionState::NotAuthenticated.new # Client Protocol Receiver @parser = ResponseParser.new(config: @config) @@ -973,7 +1039,6 @@ def initialize(host, port: nil, ssl: nil, @logout_command_tag = nil # Connection - @tls_verified = false @sock = tcp_socket(@host, @port) start_tls_session if ssl_ctx start_imap_connection @@ -989,6 +1054,7 @@ def tls_verified?; @tls_verified end # Related: #logout, #logout! def disconnect return if disconnected? + state_logout! begin begin # try to call SSL::SSLSocket#io. @@ -1374,7 +1440,7 @@ def starttls(**options) # capabilities, they will be cached. def authenticate(*args, sasl_ir: config.sasl_ir, **props, &callback) sasl_adapter.authenticate(*args, sasl_ir: sasl_ir, **props, &callback) - .tap { @capabilities = capabilities_from_resp_code _1 } + .tap do state_authenticated! _1 end end # Sends a {LOGIN command [IMAP4rev1 §6.2.3]}[https://www.rfc-editor.org/rfc/rfc3501#section-6.2.3] @@ -1408,7 +1474,7 @@ def login(user, password) raise LoginDisabledError end send_command("LOGIN", user, password) - .tap { @capabilities = capabilities_from_resp_code _1 } + .tap do state_authenticated! _1 end end # Sends a {SELECT command [IMAP4rev1 §6.3.1]}[https://www.rfc-editor.org/rfc/rfc3501#section-6.3.1] @@ -1448,8 +1514,10 @@ def select(mailbox, condstore: false) args = ["SELECT", mailbox] args << ["CONDSTORE"] if condstore synchronize do + state_unselected! # implicitly closes current mailbox @responses.clear send_command(*args) + .tap do state_selected! end end end @@ -1466,8 +1534,10 @@ def examine(mailbox, condstore: false) args = ["EXAMINE", mailbox] args << ["CONDSTORE"] if condstore synchronize do + state_unselected! # implicitly closes current mailbox @responses.clear send_command(*args) + .tap do state_selected! end end end @@ -1906,6 +1976,7 @@ def check # Related: #unselect def close send_command("CLOSE") + .tap do state_authenticated! end end # Sends an {UNSELECT command [RFC3691 §2]}[https://www.rfc-editor.org/rfc/rfc3691#section-3] @@ -1922,6 +1993,7 @@ def close # [RFC3691[https://www.rfc-editor.org/rfc/rfc3691]]. def unselect send_command("UNSELECT") + .tap do state_authenticated! end end # call-seq: @@ -3180,6 +3252,7 @@ def start_imap_connection @capabilities = capabilities_from_resp_code @greeting @receiver_thread = start_receiver_thread rescue Exception + state_logout! @sock.close raise end @@ -3188,7 +3261,10 @@ def get_server_greeting greeting = get_response raise Error, "No server greeting - connection closed" unless greeting record_untagged_response_code greeting - raise ByeResponseError, greeting if greeting.name == "BYE" + case greeting.name + when "PREAUTH" then state_authenticated! + when "BYE" then state_logout!; raise ByeResponseError, greeting + end greeting end @@ -3198,6 +3274,8 @@ def start_receiver_thread rescue Exception => ex @receiver_thread_exception = ex # don't exit the thread with an exception + ensure + state_logout! end end @@ -3220,6 +3298,7 @@ def receive_responses resp = get_response rescue Exception => e synchronize do + state_logout! @sock.close @exception = e end @@ -3239,6 +3318,7 @@ def receive_responses @tagged_response_arrival.broadcast case resp.tag when @logout_command_tag + state_logout! return when @continued_command_tag @continuation_request_exception = @@ -3248,6 +3328,7 @@ def receive_responses when UntaggedResponse record_untagged_response(resp) if resp.name == "BYE" && @logout_command_tag.nil? + state_logout! @sock.close @exception = ByeResponseError.new(resp) connection_closed = true @@ -3255,6 +3336,7 @@ def receive_responses when ContinuationRequest @continuation_request_arrival.signal end + state_unselected! if resp in {data: {code: {name: "CLOSED"}}} @response_handlers.each do |handler| handler.call(resp) end @@ -3635,6 +3717,29 @@ def start_tls_session end end + def state_authenticated!(resp = nil) + synchronize do + @capabilities = capabilities_from_resp_code resp if resp + @connection_state = ConnectionState::Authenticated.new + end + end + + def state_selected! + synchronize do + @connection_state = ConnectionState::Selected.new + end + end + + def state_unselected! + state_authenticated! if connection_state.to_sym == :selected + end + + def state_logout! + synchronize do + @connection_state = ConnectionState::Logout.new + end + end + def sasl_adapter SASLAdapter.new(self, &method(:send_command_with_continuations)) end diff --git a/lib/net/imap/connection_state.rb b/lib/net/imap/connection_state.rb new file mode 100644 index 000000000..906e99b54 --- /dev/null +++ b/lib/net/imap/connection_state.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Net + class IMAP + class ConnectionState < Net::IMAP::Data # :nodoc: + def self.define(symbol, *attrs) + symbol => Symbol + state = super(*attrs) + state.const_set :NAME, symbol + state + end + + def symbol; self.class::NAME end + def name; self.class::NAME.name end + alias to_sym symbol + + def deconstruct; [symbol, *super] end + + def deconstruct_keys(names) + hash = super + hash[:symbol] = symbol if names.nil? || names.include?(:symbol) + hash[:name] = name if names.nil? || names.include?(:name) + hash + end + + def to_h(&block) + hash = deconstruct_keys(nil) + block ? hash.to_h(&block) : hash + end + + def not_authenticated?; to_sym == :not_authenticated end + def authenticated?; to_sym == :authenticated end + def selected?; to_sym == :selected end + def logout?; to_sym == :logout end + + NotAuthenticated = define(:not_authenticated) + Authenticated = define(:authenticated) + Selected = define(:selected) + Logout = define(:logout) + + class << self + undef :define + end + freeze + end + + end +end diff --git a/test/net/imap/test_imap_connection_state.rb b/test/net/imap/test_imap_connection_state.rb new file mode 100644 index 000000000..41f4f5bfb --- /dev/null +++ b/test/net/imap/test_imap_connection_state.rb @@ -0,0 +1,226 @@ +# frozen_string_literal: true + +require "net/imap" +require "test/unit" +require_relative "fake_server" + +class ConnectionStateTest < Test::Unit::TestCase + NotAuthenticated = Net::IMAP::ConnectionState::NotAuthenticated + Authenticated = Net::IMAP::ConnectionState::Authenticated + Selected = Net::IMAP::ConnectionState::Selected + Logout = Net::IMAP::ConnectionState::Logout + + test "#name" do + assert_equal "not_authenticated", NotAuthenticated[].name + assert_equal "authenticated", Authenticated[] .name + assert_equal "selected", Selected[] .name + assert_equal "logout", Logout[] .name + end + + + test "#to_sym" do + assert_equal :not_authenticated, NotAuthenticated[].to_sym + assert_equal :authenticated, Authenticated[] .to_sym + assert_equal :selected, Selected[] .to_sym + assert_equal :logout, Logout[] .to_sym + end + + test "#deconstruct" do + assert_equal [:not_authenticated], NotAuthenticated[].deconstruct + assert_equal [:authenticated], Authenticated[] .deconstruct + assert_equal [:selected], Selected[] .deconstruct + assert_equal [:logout], Logout[] .deconstruct + end + + test "#deconstruct_keys" do + assert_equal({symbol: :not_authenticated}, NotAuthenticated[].deconstruct_keys([:symbol])) + assert_equal({symbol: :authenticated}, Authenticated[] .deconstruct_keys([:symbol])) + assert_equal({symbol: :selected}, Selected[] .deconstruct_keys([:symbol])) + assert_equal({symbol: :logout}, Logout[] .deconstruct_keys([:symbol])) + assert_equal({name: "not_authenticated"}, NotAuthenticated[].deconstruct_keys([:name])) + assert_equal({name: "authenticated"}, Authenticated[] .deconstruct_keys([:name])) + assert_equal({name: "selected"}, Selected[] .deconstruct_keys([:name])) + assert_equal({name: "logout"}, Logout[] .deconstruct_keys([:name])) + end + + test "#not_authenticated?" do + assert_equal true, NotAuthenticated[].not_authenticated? + assert_equal false, Authenticated[] .not_authenticated? + assert_equal false, Selected[] .not_authenticated? + assert_equal false, Logout[] .not_authenticated? + end + + test "#authenticated?" do + assert_equal false, NotAuthenticated[].authenticated? + assert_equal true, Authenticated[] .authenticated? + assert_equal false, Selected[] .authenticated? + assert_equal false, Logout[] .authenticated? + end + + test "#selected?" do + assert_equal false, NotAuthenticated[].selected? + assert_equal false, Authenticated[] .selected? + assert_equal true, Selected[] .selected? + assert_equal false, Logout[] .selected? + end + + test "#logout?" do + assert_equal false, NotAuthenticated[].logout? + assert_equal false, Authenticated[] .logout? + assert_equal false, Selected[] .logout? + assert_equal true, Logout[] .logout? + end + +end + +class IMAPConnectionStateTest < Test::Unit::TestCase + include Net::IMAP::FakeServer::TestHelper + + def setup + Net::IMAP.config.reset + @do_not_reverse_lookup = Socket.do_not_reverse_lookup + Socket.do_not_reverse_lookup = true + @threads = [] + end + + def teardown + if !@threads.empty? + assert_join_threads(@threads) + end + ensure + Socket.do_not_reverse_lookup = @do_not_reverse_lookup + end + + test "#connection_state after AUTHENTICATE, SELECT, CLOSE successes" do + with_fake_server(preauth: false) do |server, imap| + # AUTHENTICATE, SELECT, CLOSE + assert_equal :not_authenticated, imap.connection_state.to_sym + imap.authenticate :plain, "test_user", "test-password" + assert_equal :authenticated, imap.connection_state.to_sym + imap.select "INBOX" + assert_equal :selected, imap.connection_state.to_sym + imap.close + assert_equal :authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after LOGIN, EXAMINE, UNSELECT successes" do + with_fake_server(preauth: false, cleartext_login: true) do |server, imap| + assert_equal :not_authenticated, imap.connection_state.to_sym + imap.login "test_user", "test-password" + assert_equal :authenticated, imap.connection_state.to_sym + imap.examine "INBOX" + assert_equal :selected, imap.connection_state.to_sym + imap.unselect + assert_equal :authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after PREAUTH" do + with_fake_server(preauth: true) do |server, imap| + assert_equal :authenticated, imap.connection_state.to_sym + imap.select "INBOX" + assert_equal :selected, imap.connection_state.to_sym + imap.unselect + assert_equal :authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after [CLOSED] response code" do + with_fake_server(select: "INBOX") do |server, imap| + # NOOP doesn't _normally_ change the connection_state + assert_equal :selected, imap.connection_state.to_sym + server.on("NOOP", &:done_ok) + imap.noop + assert_equal :selected, imap.connection_state.to_sym + + # using NOOP to trigger the response code + server.on("NOOP") do |resp| + resp.untagged "OK", "[CLOSED] server maintenance" + resp.done_ok + end + imap.noop + assert_equal :authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after failed LOGIN or AUTHENTICATE" do + with_fake_server(preauth: false, cleartext_login: false) do |server, imap| + assert_raise(Net::IMAP::LoginDisabledError) do imap.login "foo", "bar" end + assert_equal :not_authenticated, imap.connection_state.to_sym + + imap.config.enforce_logindisabled = false + server.on "LOGIN" do |cmd| cmd.fail_no "nope" end + server.on "AUTHENTICATE" do |cmd| cmd.fail_no "nope" end + + assert_raise(Net::IMAP::NoResponseError) do + imap.login "foo", "bar" + end + assert_equal :not_authenticated, imap.connection_state.to_sym + + assert_raise(Net::IMAP::NoResponseError) do + imap.authenticate :plain, "foo", "bar" + end + assert_equal :not_authenticated, imap.connection_state.to_sym + + server.on "LOGIN" do |cmd| cmd.fail_bad "bad!" end + server.on "AUTHENTICATE" do |cmd| cmd.fail_bad "bad!" end + + assert_raise(Net::IMAP::BadResponseError) do + imap.login "foo", "bar" + end + assert_equal :not_authenticated, imap.connection_state.to_sym + + assert_raise(Net::IMAP::BadResponseError) do + imap.authenticate :plain, "foo", "bar" + end + assert_equal :not_authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after failed SELECT or EXAMINE" do + with_fake_server(preauth: true) do |server, imap| + # good SELECT to enter the :selected state + imap.select "INBOX" + assert_equal :selected, imap.connection_state.to_sym + # bad SELECT enters the :authenticated state + assert_raise(Net::IMAP::NoResponseError) do + imap.select "doesn't exist" + end + assert_equal :authenticated, imap.connection_state.to_sym + + # back into the :selected state + imap.examine "INBOX" + assert_equal :selected, imap.connection_state.to_sym + # bad EXAMINE enters the :authenticated state + assert_raise(Net::IMAP::NoResponseError) do + imap.examine "doesn't exist" + end + assert_equal :authenticated, imap.connection_state.to_sym + end + end + + test "#connection_state after #logout" do + with_fake_server do |server, imap| + imap.logout + assert_equal :logout, imap.connection_state.to_sym + imap.disconnect # avoid `logout!` warning and wait for closed socket + end + end + + test "#connection_state after #logout!" do + with_fake_server do |server, imap| + imap.logout! + assert_equal :logout, imap.connection_state.to_sym + end + end + + test "#connection_state after #disconnect" do + with_fake_server(ignore_io_error: true) do + |server, imap| + imap.disconnect + assert_equal :logout, imap.connection_state.to_sym + end + end + +end From 0a0fc18a07f966d19ba7956770f0e0b6fd29dc82 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 13 Mar 2025 16:38:27 -0400 Subject: [PATCH 09/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor=20Config=20?= =?UTF-8?q?attr=20type=20coercion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Switching to a "Types" registry, which holds a proc for each coercion. --- lib/net/imap/config.rb | 6 ++-- lib/net/imap/config/attr_type_coercion.rb | 36 ++++++++--------------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 0e682c8e1..5b56ec0a8 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -245,7 +245,7 @@ def self.[](config) # present. When capabilities are unknown, Net::IMAP will automatically # send a +CAPABILITY+ command first before sending +LOGIN+. # - attr_accessor :enforce_logindisabled, type: [ + attr_accessor :enforce_logindisabled, type: Enum[ false, :when_capabilities_cached, true ] @@ -275,7 +275,7 @@ def self.[](config) # Raise an ArgumentError with the deprecation warning. # # Note: #responses_without_args is an alias for #responses_without_block. - attr_accessor :responses_without_block, type: [ + attr_accessor :responses_without_block, type: Enum[ :silence_deprecation_warning, :warn, :frozen_dup, :raise, ] @@ -320,7 +320,7 @@ def self.[](config) # # [+false+ (planned default for +v0.6+)] # ResponseParser _only_ uses AppendUIDData and CopyUIDData. - attr_accessor :parser_use_deprecated_uidplus_data, type: [ + attr_accessor :parser_use_deprecated_uidplus_data, type: Enum[ true, :up_to_max_size, false ] diff --git a/lib/net/imap/config/attr_type_coercion.rb b/lib/net/imap/config/attr_type_coercion.rb index c15dbb8a6..916e89329 100644 --- a/lib/net/imap/config/attr_type_coercion.rb +++ b/lib/net/imap/config/attr_type_coercion.rb @@ -26,34 +26,24 @@ def self.included(mod) end private_class_method :included - def self.attr_accessor(attr, type: nil) - return unless type - if :boolean == type then boolean attr - elsif Integer == type then integer attr - elsif Array === type then enum attr, type - else raise ArgumentError, "unknown type coercion %p" % [type] - end - end - - def self.boolean(attr) - define_method :"#{attr}=" do |val| super !!val end - define_method :"#{attr}?" do send attr end - end + Types = Hash.new do |h, type| type => Proc | nil; type end + Types[:boolean] = Boolean = -> {!!_1} + Types[Integer] = ->{Integer(_1)} - def self.integer(attr) - define_method :"#{attr}=" do |val| super Integer val end + def self.attr_accessor(attr, type: nil) + type = Types[type] or return + define_method :"#{attr}=" do |val| super type[val] end + define_method :"#{attr}?" do send attr end if type == Boolean end - def self.enum(attr, enum) + Enum = ->(*enum) { enum = enum.dup.freeze expected = -"one of #{enum.map(&:inspect).join(", ")}" - define_method :"#{attr}=" do |val| - unless enum.include?(val) - raise ArgumentError, "expected %s, got %p" % [expected, val] - end - super val - end - end + ->val { + return val if enum.include?(val) + raise ArgumentError, "expected %s, got %p" % [expected, val] + } + } end end From 95796752097108fab21d62d0360a29a004f14c77 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 17 Mar 2025 14:58:09 -0400 Subject: [PATCH 10/36] =?UTF-8?q?=E2=9C=A8=20Fix=20Config::AttrTypeCoercio?= =?UTF-8?q?n=20for=20Ractor=20sharing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/config/attr_type_coercion.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/net/imap/config/attr_type_coercion.rb b/lib/net/imap/config/attr_type_coercion.rb index 916e89329..02632d501 100644 --- a/lib/net/imap/config/attr_type_coercion.rb +++ b/lib/net/imap/config/attr_type_coercion.rb @@ -26,9 +26,12 @@ def self.included(mod) end private_class_method :included - Types = Hash.new do |h, type| type => Proc | nil; type end - Types[:boolean] = Boolean = -> {!!_1} - Types[Integer] = ->{Integer(_1)} + def self.safe(...) = Ractor.make_shareable nil.instance_eval(...).freeze + private_class_method :safe + + Types = Hash.new do |h, type| type => Proc | nil; safe{type} end + Types[:boolean] = Boolean = safe{-> {!!_1}} + Types[Integer] = safe{->{Integer(_1)}} def self.attr_accessor(attr, type: nil) type = Types[type] or return @@ -37,12 +40,12 @@ def self.attr_accessor(attr, type: nil) end Enum = ->(*enum) { - enum = enum.dup.freeze + enum = safe{enum} expected = -"one of #{enum.map(&:inspect).join(", ")}" - ->val { + safe{->val { return val if enum.include?(val) raise ArgumentError, "expected %s, got %p" % [expected, val] - } + }} } end From a6b803b494c96ef80e77378228586ffe3bbeeaff Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 22 Mar 2025 14:31:32 -0400 Subject: [PATCH 11/36] =?UTF-8?q?=F0=9F=93=9A=20Add=20docs=20for=20receive?= =?UTF-8?q?r=20thread=20&=20server=20responses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most importantly, this documents the scenarios that need extra care to avoid memory leaks: * Commands such as #list or #fetch can have an enormous number of responses. * Commands such as #fetch can result in an enormous size per response. * Long-lived connections will gradually accumulate unsolicited server responses, especially +EXISTS+, +FETCH+, and +EXPUNGE+ responses. * A buggy or untrusted server could send inappropriate responses, which could be very numerous, very large, and very rapid. --- lib/net/imap.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 50566d064..181fd4a09 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -207,6 +207,37 @@ module Net # # This script invokes the FETCH command and the SEARCH command concurrently. # + # When running multiple commands, care must be taken to avoid ambiguity. For + # example, SEARCH responses are ambiguous about which command they are + # responding to, so search commands should not run simultaneously, unless the + # server supports +ESEARCH+ {[RFC4731]}[https://rfc-editor.org/rfc/rfc4731] or + # IMAP4rev2[https://www.rfc-editor.org/rfc/rfc9051]. See {RFC9051 + # §5.5}[https://www.rfc-editor.org/rfc/rfc9051.html#section-5.5] for + # other examples of command sequences which should not be pipelined. + # + # == Unbounded memory use + # + # Net::IMAP reads server responses in a separate receiver thread per client. + # Unhandled response data is saved to #responses, and response_handlers run + # inside the receiver thread. See the list of methods for {handling server + # responses}[rdoc-ref:Net::IMAP@Handling+server+responses], below. + # + # Because the receiver thread continuously reads and saves new responses, some + # scenarios must be careful to avoid unbounded memory use: + # + # * Commands such as #list or #fetch can have an enormous number of responses. + # * Commands such as #fetch can result in an enormous size per response. + # * Long-lived connections will gradually accumulate unsolicited server + # responses, especially +EXISTS+, +FETCH+, and +EXPUNGE+ responses. + # * A buggy or untrusted server could send inappropriate responses, which + # could be very numerous, very large, and very rapid. + # + # Use paginated or limited versions of commands whenever possible. + # + # Use #add_response_handler to handle responses after each one is received. + # Use #extract_responses, #clear_responses, or #responses (with a block) to + # prune responses. + # # == Errors # # An \IMAP server can send three different types of responses to indicate From c4a6f05f8c24ecc9effe0116032d501b59b5cc5d Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 22 Mar 2025 15:33:02 -0400 Subject: [PATCH 12/36] =?UTF-8?q?=E2=9C=85=20Add=20tests=20for=20#add=5Fre?= =?UTF-8?q?sponse=5Fhandler?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There weren't any response_handler tests at all, prior to this! --- test/net/imap/test_imap_response_handlers.rb | 53 ++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 test/net/imap/test_imap_response_handlers.rb diff --git a/test/net/imap/test_imap_response_handlers.rb b/test/net/imap/test_imap_response_handlers.rb new file mode 100644 index 000000000..900c2ebca --- /dev/null +++ b/test/net/imap/test_imap_response_handlers.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require "net/imap" +require "test/unit" +require_relative "fake_server" + +class IMAPResponseHandlersTest < Test::Unit::TestCase + include Net::IMAP::FakeServer::TestHelper + + def setup + Net::IMAP.config.reset + @do_not_reverse_lookup = Socket.do_not_reverse_lookup + Socket.do_not_reverse_lookup = true + @threads = [] + end + + def teardown + if !@threads.empty? + assert_join_threads(@threads) + end + ensure + Socket.do_not_reverse_lookup = @do_not_reverse_lookup + end + + test "#add_response_handlers" do + responses = [] + with_fake_server do |server, imap| + server.on("NOOP") do |resp| + 3.times do resp.untagged("#{_1 + 1} EXPUNGE") end + resp.done_ok + end + + assert_equal 0, imap.response_handlers.length + imap.add_response_handler do responses << [:block, _1] end + assert_equal 1, imap.response_handlers.length + imap.add_response_handler(->{ responses << [:proc, _1] }) + assert_equal 2, imap.response_handlers.length + + imap.noop + assert_pattern do + responses => [ + [:block, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 1]], + [:proc, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 1]], + [:block, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 2]], + [:proc, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 2]], + [:block, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 3]], + [:proc, Net::IMAP::UntaggedResponse[name: "EXPUNGE", data: 3]], + ] + end + end + end + +end From 7d67c66648c07abfbc7689160074adbe7c58f055 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 22 Mar 2025 14:53:00 -0400 Subject: [PATCH 13/36] =?UTF-8?q?=E2=9C=A8=20Add=20`response=5Fhandlers`?= =?UTF-8?q?=20kwarg=20to=20Net::IMAP.new?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures every server response is handled, including the greeting. --- lib/net/imap.rb | 19 ++++++++-- test/net/imap/test_imap_response_handlers.rb | 37 ++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 181fd4a09..d33831d20 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -235,8 +235,9 @@ module Net # Use paginated or limited versions of commands whenever possible. # # Use #add_response_handler to handle responses after each one is received. - # Use #extract_responses, #clear_responses, or #responses (with a block) to - # prune responses. + # Use the +response_handlers+ argument to ::new to assign response handlers + # before the receiver thread is started. Use #extract_responses, + # #clear_responses, or #responses (with a block) to prune responses. # # == Errors # @@ -961,6 +962,12 @@ def idle_response_timeout; config.idle_response_timeout end # # See DeprecatedClientOptions.new for deprecated SSL arguments. # + # [response_handlers] + # A list of response handlers to be added before the receiver thread is + # started. This ensures every server response is handled, including the + # #greeting. Note that the greeting is handled in the current thread, but + # all other responses are handled in the receiver thread. + # # [config] # A Net::IMAP::Config object to use as the basis for #config. By default, # the global Net::IMAP.config is used. @@ -1032,7 +1039,7 @@ def idle_response_timeout; config.idle_response_timeout end # [Net::IMAP::ByeResponseError] # Connected to the host successfully, but it immediately said goodbye. # - def initialize(host, port: nil, ssl: nil, + def initialize(host, port: nil, ssl: nil, response_handlers: nil, config: Config.global, **config_options) super() # Config options @@ -1057,6 +1064,7 @@ def initialize(host, port: nil, ssl: nil, @receiver_thread = nil @receiver_thread_exception = nil @receiver_thread_terminating = false + response_handlers&.each do add_response_handler(_1) end # Client Protocol Sender (including state for currently running commands) @tag_prefix = "RUBY" @@ -3255,6 +3263,10 @@ def response_handlers # end # } # + # Response handlers can also be added when the client is created before the + # receiver thread is started, by the +response_handlers+ argument to ::new. + # This ensures every server response is handled, including the #greeting. + # # Related: #remove_response_handler, #response_handlers def add_response_handler(handler = nil, &block) raise ArgumentError, "two Procs are passed" if handler && block @@ -3281,6 +3293,7 @@ def remove_response_handler(handler) def start_imap_connection @greeting = get_server_greeting @capabilities = capabilities_from_resp_code @greeting + @response_handlers.each do |handler| handler.call(@greeting) end @receiver_thread = start_receiver_thread rescue Exception state_logout! diff --git a/test/net/imap/test_imap_response_handlers.rb b/test/net/imap/test_imap_response_handlers.rb index 900c2ebca..f513d867f 100644 --- a/test/net/imap/test_imap_response_handlers.rb +++ b/test/net/imap/test_imap_response_handlers.rb @@ -50,4 +50,41 @@ def teardown end end + test "::new with response_handlers kwarg" do + greeting = nil + expunges = [] + alerts = [] + untagged = 0 + handler0 = ->{ greeting ||= _1 } + handler1 = ->{ alerts << _1.data.text if _1 in {data: {code: {name: "ALERT"}}} } + handler2 = ->{ expunges << _1.data if _1 in {name: "EXPUNGE"} } + handler3 = ->{ untagged += 1 if _1.is_a?(Net::IMAP::UntaggedResponse) } + response_handlers = [handler0, handler1, handler2, handler3] + + run_fake_server_in_thread do |server| + port = server.port + imap = Net::IMAP.new("localhost", port:, response_handlers:) + assert_equal response_handlers, imap.response_handlers + refute_same response_handlers, imap.response_handlers + + # handler0 recieved the greeting and handler3 counted it + assert_equal imap.greeting, greeting + assert_equal 1, untagged + + server.on("NOOP") do |resp| + resp.untagged "1 EXPUNGE" + resp.untagged "1 EXPUNGE" + resp.untagged "OK [ALERT] The first alert." + resp.done_ok "[ALERT] Did you see the alert?" + end + + imap.noop + assert_equal 4, untagged + assert_equal [1, 1], expunges # from handler2 + assert_equal ["The first alert.", "Did you see the alert?"], alerts + ensure + imap&.logout! unless imap&.disconnected? + end + end + end From 779f356f9e65ac8cf7615e4d730634a47b38859f Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 17 Feb 2025 20:40:46 -0500 Subject: [PATCH 14/36] =?UTF-8?q?=F0=9F=93=9A=20Consistently=20use=20"elem?= =?UTF-8?q?ent"=20or=20"entry"=20vs=20"object"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/sequence_set.rb | 54 ++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 02cd150d0..15ff68f49 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -174,7 +174,7 @@ class IMAP # # Set membership: # - #include? (aliased as #member?): - # Returns whether a given object (nz-number, range, or *) is + # Returns whether a given element (nz-number, range, or *) is # contained by the set. # - #include_star?: Returns whether the set contains *. # @@ -258,8 +258,8 @@ class IMAP # # These methods always update #string to be fully sorted and coalesced. # - # - #add (aliased as #<<): Adds a given object to the set; returns +self+. - # - #add?: If the given object is not an element in the set, adds it and + # - #add (aliased as #<<): Adds a given element to the set; returns +self+. + # - #add?: If the given element is not fully included the set, adds it and # returns +self+; otherwise, returns +nil+. # - #merge: Merges multiple elements into the set; returns +self+. # - #complement!: Replaces the contents of the set with its own #complement. @@ -268,7 +268,7 @@ class IMAP # # These methods _may_ cause #string to not be sorted or coalesced. # - # - #append: Adds a given object to the set, appending it to the existing + # - #append: Adds the given entry to the set, appending it to the existing # string, and returns +self+. # - #string=: Assigns a new #string value and replaces #elements to match. # - #replace: Replaces the contents of the set with the contents @@ -279,8 +279,8 @@ class IMAP # sorted and coalesced. # # - #clear: Removes all elements in the set; returns +self+. - # - #delete: Removes a given object from the set; returns +self+. - # - #delete?: If the given object is an element in the set, removes it and + # - #delete: Removes a given element from the set; returns +self+. + # - #delete?: If the given element is included in the set, removes it and # returns it; otherwise, returns +nil+. # - #delete_at: Removes the number at a given offset. # - #slice!: Removes the number or consecutive numbers at a given offset or @@ -690,7 +690,7 @@ def ~; remain_frozen dup.complement! end alias complement :~ # :call-seq: - # add(object) -> self + # add(element) -> self # self << other -> self # # Adds a range or number to the set and returns +self+. @@ -698,8 +698,8 @@ def ~; remain_frozen dup.complement! end # #string will be regenerated. Use #merge to add many elements at once. # # Related: #add?, #merge, #union - def add(object) - tuple_add input_to_tuple object + def add(element) + tuple_add input_to_tuple element normalize! end alias << add @@ -708,9 +708,9 @@ def add(object) # # Unlike #add, #merge, or #union, the new value is appended to #string. # This may result in a #string which has duplicates or is out-of-order. - def append(object) + def append(entry) modifying! - tuple = input_to_tuple object + tuple = input_to_tuple entry entry = tuple_to_str tuple string unless empty? # write @string before tuple_add tuple_add tuple @@ -718,19 +718,19 @@ def append(object) self end - # :call-seq: add?(object) -> self or nil + # :call-seq: add?(element) -> self or nil # # Adds a range or number to the set and returns +self+. Returns +nil+ - # when the object is already included in the set. + # when the element is already included in the set. # # #string will be regenerated. Use #merge to add many elements at once. # # Related: #add, #merge, #union, #include? - def add?(object) - add object unless include? object + def add?(element) + add element unless include? element end - # :call-seq: delete(object) -> self + # :call-seq: delete(element) -> self # # Deletes the given range or number from the set and returns +self+. # @@ -738,8 +738,8 @@ def add?(object) # many elements at once. # # Related: #delete?, #delete_at, #subtract, #difference - def delete(object) - tuple_subtract input_to_tuple object + def delete(element) + tuple_subtract input_to_tuple element normalize! end @@ -775,8 +775,8 @@ def delete(object) # #string will be regenerated after deletion. # # Related: #delete, #delete_at, #subtract, #difference, #disjoint? - def delete?(object) - tuple = input_to_tuple object + def delete?(element) + tuple = input_to_tuple element if tuple.first == tuple.last return unless include_tuple? tuple tuple_subtract tuple @@ -1386,14 +1386,14 @@ def initialize_dup(other) super end - def input_to_tuple(obj) - obj = input_try_convert obj - case obj - when *STARS, Integer then [int = to_tuple_int(obj), int] - when Range then range_to_tuple(obj) - when String then str_to_tuple(obj) + def input_to_tuple(entry) + entry = input_try_convert entry + case entry + when *STARS, Integer then [int = to_tuple_int(entry), int] + when Range then range_to_tuple(entry) + when String then str_to_tuple(entry) else - raise DataFormatError, "expected number or range, got %p" % [obj] + raise DataFormatError, "expected number or range, got %p" % [entry] end end From 1d99e400f2ee47acb6bd2218688d14b87a61139f Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 17 Feb 2025 21:33:44 -0500 Subject: [PATCH 15/36] =?UTF-8?q?=F0=9F=93=9A=20Consistently=20use=20"sets?= =?UTF-8?q?"=20or=20"other"=20vs=20"object"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/sequence_set.rb | 61 ++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 15ff68f49..48ca2738b 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -239,13 +239,13 @@ class IMAP # These methods do not modify +self+. # # - #| (aliased as #union and #+): Returns a new set combining all members - # from +self+ with all members from the other object. + # from +self+ with all members from the other set. # - #& (aliased as #intersection): Returns a new set containing all members - # common to +self+ and the other object. + # common to +self+ and the other set. # - #- (aliased as #difference): Returns a copy of +self+ with all members - # in the other object removed. + # in the other set removed. # - #^ (aliased as #xor): Returns a new set containing all members from - # +self+ and the other object except those common to both. + # +self+ and the other set except those common to both. # - #~ (aliased as #complement): Returns a new set containing all members # that are not in +self+ # - #limit: Returns a copy of +self+ which has replaced * with a @@ -261,7 +261,7 @@ class IMAP # - #add (aliased as #<<): Adds a given element to the set; returns +self+. # - #add?: If the given element is not fully included the set, adds it and # returns +self+; otherwise, returns +nil+. - # - #merge: Merges multiple elements into the set; returns +self+. + # - #merge: Adds all members of the given sets into this set; returns +self+. # - #complement!: Replaces the contents of the set with its own #complement. # # Order preserving: @@ -285,7 +285,8 @@ class IMAP # - #delete_at: Removes the number at a given offset. # - #slice!: Removes the number or consecutive numbers at a given offset or # range of offsets. - # - #subtract: Removes each given object from the set; returns +self+. + # - #subtract: Removes all members of the given sets from this set; returns + # +self+. # - #limit!: Replaces * with a given maximum value and removes all # members over that maximum; returns +self+. # @@ -820,33 +821,31 @@ def slice!(index, length = nil) deleted end - # Merges all of the elements that appear in any of the +inputs+ into the + # Merges all of the elements that appear in any of the +sets+ into the # set, and returns +self+. # - # The +inputs+ may be any objects that would be accepted by ::new: - # non-zero 32 bit unsigned integers, ranges, sequence-set - # formatted strings, other sequence sets, or enumerables containing any of - # these. + # The +sets+ may be any objects that would be accepted by ::new: non-zero + # 32 bit unsigned integers, ranges, sequence-set formatted + # strings, other sequence sets, or enumerables containing any of these. # - # #string will be regenerated after all inputs have been merged. + # #string will be regenerated after all sets have been merged. # # Related: #add, #add?, #union - def merge(*inputs) - tuples_add input_to_tuples inputs + def merge(*sets) + tuples_add input_to_tuples sets normalize! end - # Removes all of the elements that appear in any of the given +objects+ - # from the set, and returns +self+. + # Removes all of the elements that appear in any of the given +sets+ from + # the set, and returns +self+. # - # The +objects+ may be any objects that would be accepted by ::new: - # non-zero 32 bit unsigned integers, ranges, sequence-set - # formatted strings, other sequence sets, or enumerables containing any of - # these. + # The +sets+ may be any objects that would be accepted by ::new: non-zero + # 32 bit unsigned integers, ranges, sequence-set formatted + # strings, other sequence sets, or enumerables containing any of these. # # Related: #difference - def subtract(*objects) - tuples_subtract input_to_tuples objects + def subtract(*sets) + tuples_subtract input_to_tuples sets normalize! end @@ -1397,19 +1396,19 @@ def input_to_tuple(entry) end end - def input_to_tuples(obj) - obj = input_try_convert obj - case obj - when *STARS, Integer, Range then [input_to_tuple(obj)] - when String then str_to_tuples obj - when SequenceSet then obj.tuples - when Set then obj.map { [to_tuple_int(_1)] * 2 } - when Array then obj.flat_map { input_to_tuples _1 } + def input_to_tuples(set) + set = input_try_convert set + case set + when *STARS, Integer, Range then [input_to_tuple(set)] + when String then str_to_tuples set + when SequenceSet then set.tuples + when Set then set.map { [to_tuple_int(_1)] * 2 } + when Array then set.flat_map { input_to_tuples _1 } when nil then [] else raise DataFormatError, "expected nz-number, range, string, or enumerable; " \ - "got %p" % [obj] + "got %p" % [set] end end From 6ce9f1baac99e5add50aa6364fd81629b179a5a8 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 17 Feb 2025 21:45:52 -0500 Subject: [PATCH 16/36] =?UTF-8?q?=F0=9F=93=9A=20Improved=20SequenceSet[*in?= =?UTF-8?q?puts]=20documentation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap/sequence_set.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 48ca2738b..4a8b228a0 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -319,9 +319,12 @@ class SequenceSet class << self # :call-seq: - # SequenceSet[*values] -> valid frozen sequence set + # SequenceSet[*inputs] -> valid frozen sequence set # - # Returns a frozen SequenceSet, constructed from +values+. + # Returns a frozen SequenceSet, constructed from +inputs+. + # + # When only a single valid frozen SequenceSet is given, that same set is + # returned. # # An empty SequenceSet is invalid and will raise a DataFormatError. # From 41467900898ac9ff243edf493db4811a3e6153dd Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 24 Mar 2025 17:35:10 -0400 Subject: [PATCH 17/36] =?UTF-8?q?=F0=9F=8E=A8=20Reformat=20get=5Fresponse?= =?UTF-8?q?=20debug=20trace=20printing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index d33831d20..1a0e3592e 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3445,10 +3445,8 @@ def get_response end end return nil if buff.length == 0 - if config.debug? - $stderr.print(buff.gsub(/^/n, "S: ")) - end - return @parser.parse(buff) + $stderr.print(buff.gsub(/^/n, "S: ")) if config.debug? + @parser.parse(buff) end ############################# From 9c4ef330193219f02a0e3c378a036660bfda5c23 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 13:08:21 -0400 Subject: [PATCH 18/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Anchor=20literal=20r?= =?UTF-8?q?egexp=20to=20the=20end=20of=20the=20buffer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This also allows us to check against the concatenated buffer, rather than the smaller line buffer. That distinction doesn't really matter now, since we always read an entire line at once. But it will matter if we read partial lines. --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 1a0e3592e..214878973 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3437,7 +3437,7 @@ def get_response s = @sock.gets(CRLF) break unless s buff.concat(s) - if /\{(\d+)\}\r\n/n =~ s + if /\{(\d+)\}\r\n\z/n =~ buff s = @sock.read($1.to_i) buff.concat(s) else From 3ea7a29ff013c0b31640008825e5492e99532a65 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 12:45:23 -0400 Subject: [PATCH 19/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20line=20and?= =?UTF-8?q?=20literal=20parts=20of=20get=5Fresponse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMO, this refactoring makes `get_response` much easier to understand. Which will be useful, because I'm about to complicate it. 😉 --- lib/net/imap.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 214878973..1fb1cc4d0 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3434,12 +3434,9 @@ def get_tagged_response(tag, cmd, timeout = nil) def get_response buff = String.new while true - s = @sock.gets(CRLF) - break unless s - buff.concat(s) + get_response_line(buff) or break if /\{(\d+)\}\r\n\z/n =~ buff - s = @sock.read($1.to_i) - buff.concat(s) + get_response_literal(buff, $1.to_i) or break else break end @@ -3449,6 +3446,16 @@ def get_response @parser.parse(buff) end + def get_response_line(buff) + line = @sock.gets(CRLF) or return + buff << line + end + + def get_response_literal(buff, literal_size) + literal = @sock.read(literal_size) or return + buff << literal + end + ############################# # built-in response handlers From 85e5e1dcaededfc88a5b381234926ca7f1b52f65 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 13:44:50 -0400 Subject: [PATCH 20/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Allocate=20string=20?= =?UTF-8?q?literals=20with=20specific=20capacity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We know exactly how much memory we're going to need, so we can allocate this up-front, and save a few malloc/memcpy calls on larger literals. --- lib/net/imap.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 1fb1cc4d0..daefc3da2 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3452,7 +3452,8 @@ def get_response_line(buff) end def get_response_literal(buff, literal_size) - literal = @sock.read(literal_size) or return + literal = String.new(capacity: literal_size) + @sock.read(literal_size, literal) or return buff << literal end From 85cb7c71e506604db69bde979d698a53b05fc291 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 12:46:59 -0400 Subject: [PATCH 21/36] =?UTF-8?q?=F0=9F=8E=A8=20Simplify=20get=5Fresponse?= =?UTF-8?q?=20loop=20further?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index daefc3da2..84bf216df 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3435,11 +3435,8 @@ def get_response buff = String.new while true get_response_line(buff) or break - if /\{(\d+)\}\r\n\z/n =~ buff - get_response_literal(buff, $1.to_i) or break - else - break - end + break unless /\{(\d+)\}\r\n\z/n =~ buff + get_response_literal(buff, $1.to_i) or break end return nil if buff.length == 0 $stderr.print(buff.gsub(/^/n, "S: ")) if config.debug? From 02f1cfa2430c3af03ee656307cdd50ccb721ef2c Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 14:00:40 -0400 Subject: [PATCH 22/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Explicitly=20"throw?= =?UTF-8?q?=20:eof"=20for=20EOF=20in=20get=5Fresponse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This feels a lot more self-documenting than returning nil then breaking when nil is returned. Also, it lets me refactor the return values for the get_response_line/get_response_literal methods, or throw from even deeper in the stack. --- lib/net/imap.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 84bf216df..22a0ed953 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -3433,10 +3433,12 @@ def get_tagged_response(tag, cmd, timeout = nil) def get_response buff = String.new - while true - get_response_line(buff) or break - break unless /\{(\d+)\}\r\n\z/n =~ buff - get_response_literal(buff, $1.to_i) or break + catch :eof do + while true + get_response_line(buff) + break unless /\{(\d+)\}\r\n\z/n =~ buff + get_response_literal(buff, $1.to_i) + end end return nil if buff.length == 0 $stderr.print(buff.gsub(/^/n, "S: ")) if config.debug? @@ -3444,13 +3446,13 @@ def get_response end def get_response_line(buff) - line = @sock.gets(CRLF) or return + line = @sock.gets(CRLF) or throw :eof buff << line end def get_response_literal(buff, literal_size) literal = String.new(capacity: literal_size) - @sock.read(literal_size, literal) or return + @sock.read(literal_size, literal) or throw :eof buff << literal end From a15054e6630779ffe14422f3cd32804b2d9092df Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 25 Mar 2025 12:23:18 -0400 Subject: [PATCH 23/36] =?UTF-8?q?=E2=9C=85=20Ignore=20more=20IO=20errors?= =?UTF-8?q?=20in=20some=20FakeServer=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/net/imap/fake_server/test_helper.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/net/imap/fake_server/test_helper.rb b/test/net/imap/fake_server/test_helper.rb index 3c30fc72d..8a91939e0 100644 --- a/test/net/imap/fake_server/test_helper.rb +++ b/test/net/imap/fake_server/test_helper.rb @@ -4,6 +4,15 @@ module Net::IMAP::FakeServer::TestHelper + IO_ERRORS = [ + IOError, + EOFError, + Errno::ECONNABORTED, + Errno::ECONNRESET, + Errno::EPIPE, + Errno::ETIMEDOUT, + ].freeze + def run_fake_server_in_thread(ignore_io_error: false, report_on_exception: true, timeout: 10, **opts) @@ -13,14 +22,14 @@ def run_fake_server_in_thread(ignore_io_error: false, Thread.current.abort_on_exception = false Thread.current.report_on_exception = report_on_exception server.run - rescue IOError + rescue *IO_ERRORS raise unless ignore_io_error end yield server ensure begin server&.shutdown - rescue IOError + rescue *IO_ERRORS raise unless ignore_io_error end end From 1a600a0b6ab61d8667ba5e226c1d0e2d7e113dc3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Apr 2025 09:34:31 -0400 Subject: [PATCH 24/36] =?UTF-8?q?=E2=AC=86=EF=B8=8F=20Bump=20step-security?= =?UTF-8?q?/harden-runner=20from=202.11.0=20to=202.11.1=20(#423)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bumps [step-security/harden-runner](https://github.com/step-security/harden-runner) from 2.11.0 to 2.11.1. - [Release notes](https://github.com/step-security/harden-runner/releases) - [Commits](https://github.com/step-security/harden-runner/compare/4d991eb9b905ef189e4c376166672c3f2f230481...c6295a65d1254861815972266d5933fd6e532bdf) --- updated-dependencies: - dependency-name: step-security/harden-runner dependency-version: 2.11.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/push_gem.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push_gem.yml b/.github/workflows/push_gem.yml index 68cad07b6..0221b5a49 100644 --- a/.github/workflows/push_gem.yml +++ b/.github/workflows/push_gem.yml @@ -24,7 +24,7 @@ jobs: steps: # Set up - name: Harden Runner - uses: step-security/harden-runner@4d991eb9b905ef189e4c376166672c3f2f230481 # v2.11.0 + uses: step-security/harden-runner@c6295a65d1254861815972266d5933fd6e532bdf # v2.11.1 with: egress-policy: audit From 72e4eb7711467314a14fecb8dfd671a428404ffa Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 28 Mar 2025 21:41:13 -0400 Subject: [PATCH 25/36] =?UTF-8?q?=F0=9F=93=9A=20Update=20rdoc=20for=20meth?= =?UTF-8?q?ods=20delegated=20to=20Config?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 22a0ed953..5029f70dc 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -807,9 +807,11 @@ class IMAP < Protocol def self.config; Config.global end # Returns the global debug mode. + # Delegates to {Net::IMAP.config.debug}[rdoc-ref:Config#debug]. def self.debug; config.debug end # Sets the global debug mode. + # Delegates to {Net::IMAP.config.debug=}[rdoc-ref:Config#debug=]. def self.debug=(val) config.debug = val end @@ -839,13 +841,20 @@ class << self # Net::IMAP.config. attr_reader :config + ## + # :attr_reader: open_timeout # Seconds to wait until a connection is opened. - # If the IMAP object cannot open a connection within this time, - # it raises a Net::OpenTimeout exception. The default value is 30 seconds. - def open_timeout; config.open_timeout end + # Delegates to {config.open_timeout}[rdoc-ref:Config#open_timeout]. + ## + # :attr_reader: idle_response_timeout # Seconds to wait until an IDLE response is received. - def idle_response_timeout; config.idle_response_timeout end + # Delegates to {config.idle_response_timeout}[rdoc-ref:Config#idle_response_timeout]. + + # :stopdoc: + def open_timeout; config.open_timeout end + def idle_response_timeout; config.idle_response_timeout end + # :startdoc: # The hostname this client connected to attr_reader :host From 6b226ca254fa8bfe201288105f35f2957efb4f91 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 15 Apr 2025 21:40:34 -0400 Subject: [PATCH 26/36] =?UTF-8?q?=F0=9F=93=9A=20Document=20that=20open=5Ft?= =?UTF-8?q?imeout=20is=20used=20for=20TLS=20too?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 6 +++++- lib/net/imap/config.rb | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 5029f70dc..e86e0730a 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -843,7 +843,7 @@ class << self ## # :attr_reader: open_timeout - # Seconds to wait until a connection is opened. + # Seconds to wait until a connection is opened. Also used by #starttls. # Delegates to {config.open_timeout}[rdoc-ref:Config#open_timeout]. ## @@ -1341,6 +1341,10 @@ def logout! # both successful. Any error indicates that the connection has not been # secured. # + # After the server agrees to start a TLS connection, this method waits up to + # {config.open_timeout}[rdoc-ref:Config#open_timeout] before raising + # +Net::OpenTimeout+. + # # *Note:* # >>> # Any #response_handlers added before STARTTLS should be aware that the diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 5b56ec0a8..e4173960c 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -193,10 +193,13 @@ def self.[](config) # Seconds to wait until a connection is opened. # + # Applied separately for establishing TCP connection and starting a TLS + # connection. + # # If the IMAP object cannot open a connection within this time, # it raises a Net::OpenTimeout exception. # - # See Net::IMAP.new. + # See Net::IMAP.new and Net::IMAP#starttls. # # The default value is +30+ seconds. attr_accessor :open_timeout, type: Integer From 4c61347759ee7bac02e7c181c931c925693ee135 Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 15 Apr 2025 22:43:06 -0400 Subject: [PATCH 27/36] =?UTF-8?q?=F0=9F=93=9A=20Add=20a=20few=20missing=20?= =?UTF-8?q?words=20to=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index e86e0730a..f636fe1c2 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -832,7 +832,7 @@ class << self alias default_ssl_port default_tls_port end - # Returns the initial greeting the server, an UntaggedResponse. + # Returns the initial greeting sent by the server, an UntaggedResponse. attr_reader :greeting # The client configuration. See Net::IMAP::Config. From 4cc1cb9d2fdde6b9ab0374bfe45a11ac73aea89a Mon Sep 17 00:00:00 2001 From: nick evans Date: Tue, 15 Apr 2025 06:42:58 -0400 Subject: [PATCH 28/36] =?UTF-8?q?=F0=9F=8E=A8=20Reformat=20autoloads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index f636fe1c2..5286653a5 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -792,10 +792,11 @@ class IMAP < Protocol "UTF8=ONLY" => "UTF8=ACCEPT", }.freeze - autoload :ConnectionState, File.expand_path("imap/connection_state", __dir__) - autoload :SASL, File.expand_path("imap/sasl", __dir__) - autoload :SASLAdapter, File.expand_path("imap/sasl_adapter", __dir__) - autoload :StringPrep, File.expand_path("imap/stringprep", __dir__) + dir = File.expand_path("imap", __dir__) + autoload :ConnectionState, "#{dir}/connection_state" + autoload :SASL, "#{dir}/sasl" + autoload :SASLAdapter, "#{dir}/sasl_adapter" + autoload :StringPrep, "#{dir}/stringprep" include MonitorMixin if defined?(OpenSSL::SSL) From 66fdb1c0cf57612f445c92f004fa47b75015bf51 Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 17 Apr 2025 12:42:17 -0400 Subject: [PATCH 29/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Avoid=20Float=20erro?= =?UTF-8?q?rs=20in=20Config.version=5Fdefaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Using Rational internally, with aliases for Float and Integer versions. Fixes #428. --- lib/net/imap/config.rb | 34 ++++++++++++++++++++++------------ test/net/imap/test_config.rb | 2 +- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index e4173960c..70bdd5c42 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -439,44 +439,54 @@ def defaults_hash version_defaults[:default] = Config[default.send(:defaults_hash)] - version_defaults[0] = Config[:default].dup.update( + version_defaults[0r] = Config[:default].dup.update( sasl_ir: false, responses_without_block: :silence_deprecation_warning, enforce_logindisabled: false, parser_use_deprecated_uidplus_data: true, parser_max_deprecated_uidplus_data_size: 10_000, ).freeze - version_defaults[0.0] = Config[0] - version_defaults[0.1] = Config[0] - version_defaults[0.2] = Config[0] - version_defaults[0.3] = Config[0] + version_defaults[0.0r] = Config[0r] + version_defaults[0.1r] = Config[0r] + version_defaults[0.2r] = Config[0r] + version_defaults[0.3r] = Config[0r] - version_defaults[0.4] = Config[0.3].dup.update( + version_defaults[0.4r] = Config[0.3r].dup.update( sasl_ir: true, parser_max_deprecated_uidplus_data_size: 1000, ).freeze - version_defaults[0.5] = Config[0.4].dup.update( + version_defaults[0.5r] = Config[0.4r].dup.update( enforce_logindisabled: true, responses_without_block: :warn, parser_use_deprecated_uidplus_data: :up_to_max_size, parser_max_deprecated_uidplus_data_size: 100, ).freeze - version_defaults[0.6] = Config[0.5].dup.update( + version_defaults[0.6r] = Config[0.5r].dup.update( responses_without_block: :frozen_dup, parser_use_deprecated_uidplus_data: false, parser_max_deprecated_uidplus_data_size: 0, ).freeze - version_defaults[0.7] = Config[0.6].dup.update( + version_defaults[0.7r] = Config[0.6r].dup.update( ).freeze - current = VERSION.to_f + # Safe conversions one way only: + # 0.6r.to_f == 0.6 # => true + # 0.6 .to_r == 0.6r # => false + version_defaults.to_a.each do |k, v| + next unless k in Rational + version_defaults[k.to_f] = v + next unless k.to_i.to_r == k + version_defaults[k.to_i] = v + end + + current = VERSION.to_r version_defaults[:original] = Config[0] version_defaults[:current] = Config[current] - version_defaults[:next] = Config[current + 0.1] - version_defaults[:future] = Config[current + 0.2] + version_defaults[:next] = Config[current + 0.1r] + version_defaults[:future] = Config[current + 0.2r] version_defaults.freeze diff --git a/test/net/imap/test_config.rb b/test/net/imap/test_config.rb index 2f5981699..a4af90ee0 100644 --- a/test/net/imap/test_config.rb +++ b/test/net/imap/test_config.rb @@ -140,7 +140,7 @@ class ConfigTest < Test::Unit::TestCase test ".version_defaults are all frozen, and inherit debug from global" do Config.version_defaults.each do |name, config| - assert [0, Float, Symbol].any? { _1 === name } + assert [0, Float, Rational, Symbol].any? { _1 === name } assert_kind_of Config, config assert config.frozen?, "#{name} isn't frozen" assert config.inherited?(:debug), "#{name} doesn't inherit debug" From c7732e65c55ca3fda6a1115b64390a10e63e9aaf Mon Sep 17 00:00:00 2001 From: nick evans Date: Thu, 17 Apr 2025 13:21:05 -0400 Subject: [PATCH 30/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Add=20default=5Fproc?= =?UTF-8?q?=20to=20Config.version=5Fdefaults?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This enables more flexible lookups. So we can now use something like `Config["0.4.11"]` (I do _not_ intend to store version defaults for `x.y.z` releases). I had to add a special-case for zero, to avoid `Config["missing"] == Config[0r]` --- lib/net/imap/config.rb | 26 +++++++++++++---- test/net/imap/test_config.rb | 56 ++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index 70bdd5c42..a64ec3f2e 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -131,8 +131,25 @@ def self.default; @default end def self.global; @global if defined?(@global) end # A hash of hard-coded configurations, indexed by version number or name. + # Values can be accessed with any object that responds to +to_sym+ or + # +to_r+/+to_f+ with a non-zero number. + # + # Config::[] gets named or numbered versions from this hash. + # + # For example: + # Net::IMAP::Config.version_defaults[0.5] == Net::IMAP::Config[0.5] + # Net::IMAP::Config[0.5] == Net::IMAP::Config[0.5r] # => true + # Net::IMAP::Config["current"] == Net::IMAP::Config[:current] # => true + # Net::IMAP::Config["0.5.6"] == Net::IMAP::Config[0.5r] # => true def self.version_defaults; @version_defaults end - @version_defaults = {} + @version_defaults = Hash.new {|h, k| + # NOTE: String responds to both so the order is significant. + # And ignore non-numeric conversion to zero, because: "wat!?".to_r == 0 + (h.fetch(k.to_r, nil) || h.fetch(k.to_f, nil) if k.is_a?(Numeric)) || + (h.fetch(k.to_sym, nil) if k.respond_to?(:to_sym)) || + (h.fetch(k.to_r, nil) if k.respond_to?(:to_r) && k.to_r != 0r) || + (h.fetch(k.to_f, nil) if k.respond_to?(:to_f) && k.to_f != 0.0) + } # :call-seq: # Net::IMAP::Config[number] -> versioned config @@ -155,18 +172,17 @@ def self.[](config) elsif config.nil? && global.nil? then nil elsif config.respond_to?(:to_hash) then new(global, **config).freeze else - version_defaults.fetch(config) do + version_defaults[config] or case config when Numeric raise RangeError, "unknown config version: %p" % [config] - when Symbol + when String, Symbol raise KeyError, "unknown config name: %p" % [config] else raise TypeError, "no implicit conversion of %s to %s" % [ config.class, Config ] end - end end end @@ -478,8 +494,6 @@ def defaults_hash version_defaults.to_a.each do |k, v| next unless k in Rational version_defaults[k.to_f] = v - next unless k.to_i.to_r == k - version_defaults[k.to_i] = v end current = VERSION.to_r diff --git a/test/net/imap/test_config.rb b/test/net/imap/test_config.rb index a4af90ee0..907f360a5 100644 --- a/test/net/imap/test_config.rb +++ b/test/net/imap/test_config.rb @@ -165,14 +165,18 @@ class ConfigTest < Test::Unit::TestCase end test ".[] for all x.y versions" do - original = Config[0] + original = Config[0r] assert_kind_of Config, original + assert_same original, Config[0] assert_same original, Config[0.0] assert_same original, Config[0.1] assert_same original, Config[0.2] assert_same original, Config[0.3] ((0.4r..FUTURE_VERSION.to_r) % 0.1r).each do |version| - assert_kind_of Config, Config[version.to_f] + config = Config[version] + assert_kind_of Config, config + assert_same config, Config[version.to_f] + assert_same config, Config[version.to_f.to_r] end end @@ -186,6 +190,8 @@ class ConfigTest < Test::Unit::TestCase test ".[] key errors" do assert_raise(KeyError) do Config[:nonexistent] end + assert_raise(KeyError) do Config["nonexistent"] end + assert_raise(KeyError) do Config["0.01"] end end test ".[] with symbol names" do @@ -195,6 +201,52 @@ class ConfigTest < Test::Unit::TestCase assert_same Config[FUTURE_VERSION], Config[:future] end + test ".[] with string names" do + assert_same Config[:original], Config["original"] + assert_same Config[:current], Config["current"] + assert_same Config[0.4r], Config["0.4.11"] + assert_same Config[0.5r], Config["0.5.6"] + assert_same Config[:current], Config[Net::IMAP::VERSION] + end + + test ".[] with object responding to to_sym, to_r, or to_f" do + # responds to none of the methods + duck = Object.new + assert_raise TypeError do Config[duck] end + + # to_sym + duck = Object.new + def duck.to_sym = :current + assert_same Config[:current], Config[duck] + + # to_r + duck = Object.new + def duck.to_r = 0.6r + assert_same Config[0.6r], Config[duck] + + # to_f + duck = Object.new + def duck.to_f = 0.4 + assert_same Config[0.4], Config[duck] + + # prefer to_r over to_f + def duck.to_r = 0.5r + assert_same Config[0.5r], Config[duck] + + # prefer to_sym over to_r + def duck.to_sym = :original + assert_same Config[:original], Config[duck] + + # keeps trying if to_sym finds nothing + duck = Object.new + def duck.to_sym = :nope + def duck.to_f = 0.5 + assert_same Config[0.5], Config[duck] + # keeps trying if to_sym and to_r both find nothing + def duck.to_r = 1/11111 + assert_same Config[0.5], Config[duck] + end + test ".[] with a hash" do config = Config[{responses_without_block: :raise, sasl_ir: false}] assert config.frozen? From b1413c65b585bac8f21ba3c2eb9459fe065348b3 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 13 Sep 2024 09:12:25 -0400 Subject: [PATCH 31/36] =?UTF-8?q?=E2=9C=A8=20Customize=20SequenceSet=20YAM?= =?UTF-8?q?L=20serialization?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can perfectly reconstruct the SequenceSet from the string. The string is the canonical IMAP representation. And, even if the tuples representation uses less memory, the string representation will always be more compact than the naive YAML serialization of the tuples. So only serialize the string. --- lib/net/imap/sequence_set.rb | 12 +++ .../response_parser/esearch_responses.yml | 75 ------------------- .../rfc7162_condstore_qresync_responses.yml | 37 +-------- .../response_parser/rfc9394_partial.yml | 8 -- .../response_parser/status_responses.yml | 5 -- 5 files changed, 13 insertions(+), 124 deletions(-) diff --git a/lib/net/imap/sequence_set.rb b/lib/net/imap/sequence_set.rb index 4a8b228a0..d67c77540 100644 --- a/lib/net/imap/sequence_set.rb +++ b/lib/net/imap/sequence_set.rb @@ -1369,6 +1369,18 @@ def send_data(imap, tag) # :nodoc: imap.__send__(:put_string, valid_string) end + # For YAML serialization + def encode_with(coder) # :nodoc: + # we can perfectly reconstruct from the string + coder['string'] = to_s + end + + # For YAML deserialization + def init_with(coder) # :nodoc: + @tuples = [] + self.string = coder['string'] + end + protected attr_reader :tuples # :nodoc: diff --git a/test/net/imap/fixtures/response_parser/esearch_responses.yml b/test/net/imap/fixtures/response_parser/esearch_responses.yml index d070d4337..b76e0da12 100644 --- a/test/net/imap/fixtures/response_parser/esearch_responses.yml +++ b/test/net/imap/fixtures/response_parser/esearch_responses.yml @@ -25,11 +25,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 2,10:11 - tuples: - - - 2 - - 2 - - - 10 - - 11 raw_data: "* ESEARCH (TAG \"A283\") ALL 2,10:11\r\n" rfc9051_6.4.4_ESEARCH_example_3: @@ -53,9 +48,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: '43' - tuples: - - - 43 - - 43 raw_data: "* ESEARCH (TAG \"A285\") ALL 43\r\n" rfc9051_6.4.4_ESEARCH_example_5: @@ -107,11 +99,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: '17,900,901' - tuples: - - - 17 - - 17 - - - 900 - - 901 raw_data: "* ESEARCH (TAG \"A301\") UID ALL 17,900,901\r\n" rfc9051_6.4.4.4_ESEARCH_example_2: @@ -125,15 +112,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 882,1102,3003,3005:3006 - tuples: - - - 882 - - 882 - - - 1102 - - 1102 - - - 3003 - - 3003 - - - 3005 - - 3006 raw_data: "* ESEARCH (TAG \"P283\") ALL 882,1102,3003,3005:3006\r\n" rfc9051_6.4.4.4_ESEARCH_example_3: @@ -147,13 +125,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 3:15,27,29:103 - tuples: - - - 3 - - 15 - - - 27 - - 27 - - - 29 - - 103 raw_data: "* ESEARCH (TAG \"G283\") ALL 3:15,27,29:103\r\n" rfc9051_6.4.4.4_ESEARCH_example_4: @@ -167,13 +138,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 2,10:15,21 - tuples: - - - 2 - - 2 - - - 10 - - 15 - - - 21 - - 21 raw_data: "* ESEARCH (TAG \"C283\") ALL 2,10:15,21\r\n" rfc9051_6.4.4.4_ESEARCH_example_5: @@ -231,13 +195,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 2,10:15,21 - tuples: - - - 2 - - 2 - - - 10 - - 15 - - - 21 - - 21 raw_data: "* ESEARCH (TAG \"C286\") MIN 2 ALL 2,10:15,21\r\n" rfc9051_7.1_ESEARCH_example_1: @@ -251,19 +208,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 1:3,5,8,13,21,42 - tuples: - - - 1 - - 3 - - - 5 - - 5 - - - 8 - - 8 - - - 13 - - 13 - - - 21 - - 21 - - - 42 - - 42 raw_data: "* ESEARCH (TAG \"h\") ALL 1:3,5,8,13,21,42\r\n" rfc9051_7.3.4_ESEARCH_example_1: @@ -279,13 +223,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 4:18,21,28 - tuples: - - - 4 - - 18 - - - 21 - - 21 - - - 28 - - 28 raw_data: "* ESEARCH UID COUNT 17 ALL 4:18,21,28\r\n" rfc9051_7.3.4_ESEARCH_example_2: @@ -301,13 +238,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 4:18,21,28 - tuples: - - - 4 - - 18 - - - 21 - - 21 - - - 28 - - 28 raw_data: "* ESEARCH (TAG \"a567\") UID COUNT 17 ALL 4:18,21,28\r\n" rfc9051_7.3.4_ESEARCH_example_3: @@ -323,9 +253,4 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 1:17,21 - tuples: - - - 1 - - 17 - - - 21 - - 21 raw_data: "* ESEARCH COUNT 18 ALL 1:17,21\r\n" diff --git a/test/net/imap/fixtures/response_parser/rfc7162_condstore_qresync_responses.yml b/test/net/imap/fixtures/response_parser/rfc7162_condstore_qresync_responses.yml index 19fcd1780..9ad950159 100644 --- a/test/net/imap/fixtures/response_parser/rfc7162_condstore_qresync_responses.yml +++ b/test/net/imap/fixtures/response_parser/rfc7162_condstore_qresync_responses.yml @@ -55,12 +55,7 @@ code: !ruby/struct:Net::IMAP::ResponseCode name: MODIFIED data: !ruby/object:Net::IMAP::SequenceSet - str: '7,9' - tuples: - - - 7 - - 7 - - - 9 - - 9 + string: '7,9' text: Conditional STORE failed raw_data: "d105 OK [MODIFIED 7,9] Conditional STORE failed\r\n" @@ -109,11 +104,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: 1:3,5 - tuples: - - - 1 - - 3 - - - 5 - - 5 - - MODSEQ - 1236 raw_data: "* ESEARCH (TAG \"a\") ALL 1:3,5 MODSEQ 1236\r\n" @@ -129,11 +119,6 @@ - - ALL - !ruby/object:Net::IMAP::SequenceSet string: '5,3,2,1' - tuples: - - - 1 - - 3 - - - 5 - - 5 - - MODSEQ - 1236 raw_data: "* ESEARCH (TAG \"a\") ALL 5,3,2,1 MODSEQ 1236\r\n" @@ -145,17 +130,6 @@ data: !ruby/object:Net::IMAP::VanishedData uids: !ruby/object:Net::IMAP::SequenceSet string: 41,43:116,118,120:211,214:540 - tuples: - - - 41 - - 41 - - - 43 - - 116 - - - 118 - - 118 - - - 120 - - 211 - - - 214 - - 540 earlier: true raw_data: "* VANISHED (EARLIER) 41,43:116,118,120:211,214:540\r\n" @@ -166,14 +140,5 @@ data: !ruby/object:Net::IMAP::VanishedData uids: !ruby/object:Net::IMAP::SequenceSet string: '405,407,410,425' - tuples: - - - 405 - - 405 - - - 407 - - 407 - - - 410 - - 410 - - - 425 - - 425 earlier: false raw_data: "* VANISHED 405,407,410,425\r\n" diff --git a/test/net/imap/fixtures/response_parser/rfc9394_partial.yml b/test/net/imap/fixtures/response_parser/rfc9394_partial.yml index 233a9096e..19498e3e1 100644 --- a/test/net/imap/fixtures/response_parser/rfc9394_partial.yml +++ b/test/net/imap/fixtures/response_parser/rfc9394_partial.yml @@ -20,11 +20,6 @@ excl: false results: !ruby/object:Net::IMAP::SequenceSet string: 200:250,252:300 - tuples: - - - 200 - - 250 - - - 252 - - 300 raw_data: "* ESEARCH (TAG \"A01\") UID PARTIAL (-1:-100 200:250,252:300)\r\n" "RFC9394 PARTIAL 3.1. example 2": @@ -43,9 +38,6 @@ excl: false results: !ruby/object:Net::IMAP::SequenceSet string: 55500:56000 - tuples: - - - 55500 - - 56000 raw_data: "* ESEARCH (TAG \"A02\") UID PARTIAL (23500:24000 55500:56000)\r\n" "RFC9394 PARTIAL 3.1. example 3": diff --git a/test/net/imap/fixtures/response_parser/status_responses.yml b/test/net/imap/fixtures/response_parser/status_responses.yml index 074b56ad6..85581360c 100644 --- a/test/net/imap/fixtures/response_parser/status_responses.yml +++ b/test/net/imap/fixtures/response_parser/status_responses.yml @@ -52,11 +52,6 @@ SEQ: !ruby/struct:Net::IMAP::ExtensionData data: !ruby/object:Net::IMAP::SequenceSet string: 1234:5,*:789654 - tuples: - - - 5 - - 1234 - - - 789654 - - 4294967296 COMP-EMPTY: !ruby/struct:Net::IMAP::ExtensionData data: [] COMP-QUOTED: !ruby/struct:Net::IMAP::ExtensionData From 18bc62150df697596510c6f47a765155e4d6c0f9 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 2 Apr 2025 20:50:15 -0400 Subject: [PATCH 32/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Extract=20ResponseRe?= =?UTF-8?q?ader=20from=20get=5Fresponse?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's nice to extract a little bit of the complexity from the core `Net::IMAP` class. But my primary motivation was so that I could directly test this code quickly and in isolation from needing to simulate a full IMAP connection. --- lib/net/imap.rb | 23 +++---------- lib/net/imap/response_reader.rb | 38 +++++++++++++++++++++ test/net/imap/test_response_reader.rb | 49 +++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 19 deletions(-) create mode 100644 lib/net/imap/response_reader.rb create mode 100644 test/net/imap/test_response_reader.rb diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 5286653a5..3dd82ef2f 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -794,6 +794,7 @@ class IMAP < Protocol dir = File.expand_path("imap", __dir__) autoload :ConnectionState, "#{dir}/connection_state" + autoload :ResponseReader, "#{dir}/response_reader" autoload :SASL, "#{dir}/sasl" autoload :SASLAdapter, "#{dir}/sasl_adapter" autoload :StringPrep, "#{dir}/stringprep" @@ -1089,6 +1090,7 @@ def initialize(host, port: nil, ssl: nil, response_handlers: nil, # Connection @sock = tcp_socket(@host, @port) + @reader = ResponseReader.new(self, @sock) start_tls_session if ssl_ctx start_imap_connection end @@ -3446,30 +3448,12 @@ def get_tagged_response(tag, cmd, timeout = nil) end def get_response - buff = String.new - catch :eof do - while true - get_response_line(buff) - break unless /\{(\d+)\}\r\n\z/n =~ buff - get_response_literal(buff, $1.to_i) - end - end + buff = @reader.read_response_buffer return nil if buff.length == 0 $stderr.print(buff.gsub(/^/n, "S: ")) if config.debug? @parser.parse(buff) end - def get_response_line(buff) - line = @sock.gets(CRLF) or throw :eof - buff << line - end - - def get_response_literal(buff, literal_size) - literal = String.new(capacity: literal_size) - @sock.read(literal_size, literal) or throw :eof - buff << literal - end - ############################# # built-in response handlers @@ -3771,6 +3755,7 @@ def start_tls_session raise "already using SSL" if @sock.kind_of?(OpenSSL::SSL::SSLSocket) raise "cannot start TLS without SSLContext" unless ssl_ctx @sock = SSLSocket.new(@sock, ssl_ctx) + @reader = ResponseReader.new(self, @sock) @sock.sync_close = true @sock.hostname = @host if @sock.respond_to? :hostname= ssl_socket_connect(@sock, open_timeout) diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb new file mode 100644 index 000000000..57770e3b8 --- /dev/null +++ b/lib/net/imap/response_reader.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Net + class IMAP + # See https://www.rfc-editor.org/rfc/rfc9051#section-2.2.2 + class ResponseReader # :nodoc: + attr_reader :client + + def initialize(client, sock) + @client, @sock = client, sock + end + + def read_response_buffer + buff = String.new + catch :eof do + while true + read_line(buff) + break unless /\{(\d+)\}\r\n\z/n =~ buff + read_literal(buff, $1.to_i) + end + end + buff + end + + private + + def read_line(buff) + buff << (@sock.gets(CRLF) or throw :eof) + end + + def read_literal(buff, literal_size) + literal = String.new(capacity: literal_size) + buff << (@sock.read(literal_size, literal) or throw :eof) + end + + end + end +end diff --git a/test/net/imap/test_response_reader.rb b/test/net/imap/test_response_reader.rb new file mode 100644 index 000000000..c24f1269d --- /dev/null +++ b/test/net/imap/test_response_reader.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require "net/imap" +require "stringio" +require "test/unit" + +class ResponseReaderTest < Test::Unit::TestCase + def setup + Net::IMAP.config.reset + end + + class FakeClient + def config = @config ||= Net::IMAP.config.new + end + + def literal(str) = "{#{str.bytesize}}\r\n#{str}" + + test "#read_response_buffer" do + client = FakeClient.new + aaaaaaaaa = "a" * (20 << 10) + many_crs = "\r" * 1000 + many_crlfs = "\r\n" * 500 + simple = "* OK greeting\r\n" + long_line = "tag ok #{aaaaaaaaa} #{aaaaaaaaa}\r\n" + literal_aaaa = "* fake #{literal aaaaaaaaa}\r\n" + literal_crlf = "tag ok #{literal many_crlfs} #{literal many_crlfs}\r\n" + illegal_crs = "tag ok #{many_crs} #{many_crs}\r\n" + illegal_lfs = "tag ok #{literal "\r"}\n#{literal "\r"}\n\r\n" + io = StringIO.new([ + simple, + long_line, + literal_aaaa, + literal_crlf, + illegal_crs, + illegal_lfs, + simple, + ].join) + rcvr = Net::IMAP::ResponseReader.new(client, io) + assert_equal simple, rcvr.read_response_buffer.to_str + assert_equal long_line, rcvr.read_response_buffer.to_str + assert_equal literal_aaaa, rcvr.read_response_buffer.to_str + assert_equal literal_crlf, rcvr.read_response_buffer.to_str + assert_equal illegal_crs, rcvr.read_response_buffer.to_str + assert_equal illegal_lfs, rcvr.read_response_buffer.to_str + assert_equal simple, rcvr.read_response_buffer.to_str + assert_equal "", rcvr.read_response_buffer.to_str + end + +end From b32b6751af061b4f0d50c818512b5b41d8fbe951 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 14 Apr 2025 09:23:58 -0400 Subject: [PATCH 33/36] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Save=20ResponseReade?= =?UTF-8?q?r=20ivars:=20@buff=20&=20@literal=5Fsize?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids the need to pass these to every method that uses them. That's not a big deal now, but it simplifies the next few changes. Also added a missing test for empty literals: "{0}\r\n" --- lib/net/imap/response_reader.rb | 20 ++++++++++++++------ test/net/imap/test_response_reader.rb | 3 +++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb index 57770e3b8..7d3a53685 100644 --- a/lib/net/imap/response_reader.rb +++ b/lib/net/imap/response_reader.rb @@ -11,26 +11,34 @@ def initialize(client, sock) end def read_response_buffer - buff = String.new + @buff = String.new catch :eof do while true - read_line(buff) - break unless /\{(\d+)\}\r\n\z/n =~ buff - read_literal(buff, $1.to_i) + read_line + break unless (@literal_size = get_literal_size) + read_literal end end buff + ensure + @buff = nil end private - def read_line(buff) + attr_reader :buff, :literal_size + + def get_literal_size = /\{(\d+)\}\r\n\z/n =~ buff && $1.to_i + + def read_line buff << (@sock.gets(CRLF) or throw :eof) end - def read_literal(buff, literal_size) + def read_literal literal = String.new(capacity: literal_size) buff << (@sock.read(literal_size, literal) or throw :eof) + ensure + @literal_size = nil end end diff --git a/test/net/imap/test_response_reader.rb b/test/net/imap/test_response_reader.rb index c24f1269d..159002c3d 100644 --- a/test/net/imap/test_response_reader.rb +++ b/test/net/imap/test_response_reader.rb @@ -24,6 +24,7 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" long_line = "tag ok #{aaaaaaaaa} #{aaaaaaaaa}\r\n" literal_aaaa = "* fake #{literal aaaaaaaaa}\r\n" literal_crlf = "tag ok #{literal many_crlfs} #{literal many_crlfs}\r\n" + zero_literal = "tag ok #{literal ""} #{literal ""}\r\n" illegal_crs = "tag ok #{many_crs} #{many_crs}\r\n" illegal_lfs = "tag ok #{literal "\r"}\n#{literal "\r"}\n\r\n" io = StringIO.new([ @@ -31,6 +32,7 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" long_line, literal_aaaa, literal_crlf, + zero_literal, illegal_crs, illegal_lfs, simple, @@ -40,6 +42,7 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" assert_equal long_line, rcvr.read_response_buffer.to_str assert_equal literal_aaaa, rcvr.read_response_buffer.to_str assert_equal literal_crlf, rcvr.read_response_buffer.to_str + assert_equal zero_literal, rcvr.read_response_buffer.to_str assert_equal illegal_crs, rcvr.read_response_buffer.to_str assert_equal illegal_lfs, rcvr.read_response_buffer.to_str assert_equal simple, rcvr.read_response_buffer.to_str From 0ae8576c1a90bcd9573f81bdad4b4b824642d105 Mon Sep 17 00:00:00 2001 From: nick evans Date: Sat, 19 Apr 2025 22:21:59 -0400 Subject: [PATCH 34/36] =?UTF-8?q?=E2=9C=A8=20Limit=20max=20response=20size?= =?UTF-8?q?=20to=20512MiB=20(hard-coded)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _Please note:_ this only limits the size per response. It does _not_ limit how many unhandled responses may be stored on the responses hash. --- lib/net/imap/errors.rb | 33 ++++++++++++++++++++++ lib/net/imap/response_reader.rb | 31 +++++++++++++++++++-- test/net/imap/test_errors.rb | 40 +++++++++++++++++++++++++++ test/net/imap/test_response_reader.rb | 23 +++++++++++++++ 4 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 test/net/imap/test_errors.rb diff --git a/lib/net/imap/errors.rb b/lib/net/imap/errors.rb index d329a513d..bab4bbcf5 100644 --- a/lib/net/imap/errors.rb +++ b/lib/net/imap/errors.rb @@ -17,6 +17,39 @@ def initialize(msg = "Remote server has disabled the LOGIN command", ...) class DataFormatError < Error end + # Error raised when the socket cannot be read, due to a Config limit. + class ResponseReadError < Error + end + + # Error raised when a response is larger than IMAP#max_response_size. + class ResponseTooLargeError < ResponseReadError + attr_reader :bytes_read, :literal_size + attr_reader :max_response_size + + def initialize(msg = nil, *args, + bytes_read: nil, + literal_size: nil, + max_response_size: nil, + **kwargs) + @bytes_read = bytes_read + @literal_size = literal_size + @max_response_size = max_response_size + msg ||= [ + "Response size", response_size_msg, "exceeds max_response_size", + max_response_size && "(#{max_response_size}B)", + ].compact.join(" ") + super(msg, *args, **kwargs) + end + + private + + def response_size_msg + if bytes_read && literal_size + "(#{bytes_read}B read + #{literal_size}B literal)" + end + end + end + # Error raised when a response from the server is non-parsable. class ResponseParseError < Error end diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb index 7d3a53685..084bc5b9e 100644 --- a/lib/net/imap/response_reader.rb +++ b/lib/net/imap/response_reader.rb @@ -28,19 +28,46 @@ def read_response_buffer attr_reader :buff, :literal_size + def bytes_read = buff.bytesize + def empty? = buff.empty? + def done? = line_done? && !get_literal_size + def line_done? = buff.end_with?(CRLF) def get_literal_size = /\{(\d+)\}\r\n\z/n =~ buff && $1.to_i def read_line - buff << (@sock.gets(CRLF) or throw :eof) + buff << (@sock.gets(CRLF, read_limit) or throw :eof) + max_response_remaining! unless line_done? end def read_literal + # check before allocating memory for literal + max_response_remaining! literal = String.new(capacity: literal_size) - buff << (@sock.read(literal_size, literal) or throw :eof) + buff << (@sock.read(read_limit(literal_size), literal) or throw :eof) ensure @literal_size = nil end + def read_limit(limit = nil) + [limit, max_response_remaining!].compact.min + end + + def max_response_size = 512 << 20 # TODO: Config#max_response_size + def max_response_remaining = max_response_size &.- bytes_read + def response_too_large? = max_response_size &.< min_response_size + def min_response_size = bytes_read + min_response_remaining + + def min_response_remaining + empty? ? 3 : done? ? 0 : (literal_size || 0) + 2 + end + + def max_response_remaining! + return max_response_remaining unless response_too_large? + raise ResponseTooLargeError.new( + max_response_size:, bytes_read:, literal_size:, + ) + end + end end end diff --git a/test/net/imap/test_errors.rb b/test/net/imap/test_errors.rb new file mode 100644 index 000000000..a6a7cb0f4 --- /dev/null +++ b/test/net/imap/test_errors.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require "net/imap" +require "test/unit" + +class IMAPErrorsTest < Test::Unit::TestCase + + test "ResponseTooLargeError" do + err = Net::IMAP::ResponseTooLargeError.new + assert_nil err.bytes_read + assert_nil err.literal_size + assert_nil err.max_response_size + + err = Net::IMAP::ResponseTooLargeError.new("manually set message") + assert_equal "manually set message", err.message + assert_nil err.bytes_read + assert_nil err.literal_size + assert_nil err.max_response_size + + err = Net::IMAP::ResponseTooLargeError.new(max_response_size: 1024) + assert_equal "Response size exceeds max_response_size (1024B)", err.message + assert_nil err.bytes_read + assert_nil err.literal_size + assert_equal 1024, err.max_response_size + + err = Net::IMAP::ResponseTooLargeError.new(bytes_read: 1200, + max_response_size: 1200) + assert_equal 1200, err.bytes_read + assert_equal "Response size exceeds max_response_size (1200B)", err.message + + err = Net::IMAP::ResponseTooLargeError.new(bytes_read: 800, + literal_size: 1000, + max_response_size: 1200) + assert_equal 800, err.bytes_read + assert_equal 1000, err.literal_size + assert_equal("Response size (800B read + 1000B literal) " \ + "exceeds max_response_size (1200B)", err.message) + end + +end diff --git a/test/net/imap/test_response_reader.rb b/test/net/imap/test_response_reader.rb index 159002c3d..154aff6a1 100644 --- a/test/net/imap/test_response_reader.rb +++ b/test/net/imap/test_response_reader.rb @@ -49,4 +49,27 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" assert_equal "", rcvr.read_response_buffer.to_str end + class LimitedResponseReader < Net::IMAP::ResponseReader + attr_reader :max_response_size + def initialize(*args, max_response_size:) + super(*args) + @max_response_size = max_response_size + end + end + + test "#read_response_buffer with max_response_size" do + client = FakeClient.new + max_response_size = 10 + under = "+ 3456\r\n" + exact = "+ 345678\r\n" + over = "+ 3456789\r\n" + io = StringIO.new([under, exact, over].join) + rcvr = LimitedResponseReader.new(client, io, max_response_size:) + assert_equal under, rcvr.read_response_buffer.to_str + assert_equal exact, rcvr.read_response_buffer.to_str + assert_raise Net::IMAP::ResponseTooLargeError do + rcvr.read_response_buffer + end + end + end From b6bdee27a5cd38dd386042f43fec160840fe7562 Mon Sep 17 00:00:00 2001 From: nick evans Date: Wed, 9 Apr 2025 09:54:51 -0400 Subject: [PATCH 35/36] =?UTF-8?q?=E2=9C=A8=20Make=20max=5Fresponse=5Fsize?= =?UTF-8?q?=20configurable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Though it would be useful to also have limits based on response type and what commands are currently running, that's out of scope for now. _Please note:_ this only limits the size per response. It does _not_ limit how many unhandled responses may be stored on the responses hash. --- lib/net/imap.rb | 12 ++++ lib/net/imap/config.rb | 37 +++++++++++ lib/net/imap/config/attr_type_coercion.rb | 4 ++ lib/net/imap/response_reader.rb | 2 +- test/net/imap/test_config.rb | 13 ++++ test/net/imap/test_imap_max_response_size.rb | 67 ++++++++++++++++++++ test/net/imap/test_response_reader.rb | 13 +--- 7 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 test/net/imap/test_imap_max_response_size.rb diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 3dd82ef2f..d7e304540 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -234,6 +234,10 @@ module Net # # Use paginated or limited versions of commands whenever possible. # + # Use Config#max_response_size to impose a limit on incoming server responses + # as they are being read. This is especially important for untrusted + # servers. + # # Use #add_response_handler to handle responses after each one is received. # Use the +response_handlers+ argument to ::new to assign response handlers # before the receiver thread is started. Use #extract_responses, @@ -853,9 +857,17 @@ class << self # Seconds to wait until an IDLE response is received. # Delegates to {config.idle_response_timeout}[rdoc-ref:Config#idle_response_timeout]. + ## + # :attr_accessor: max_response_size + # + # The maximum allowed server response size, in bytes. + # Delegates to {config.max_response_size}[rdoc-ref:Config#max_response_size]. + # :stopdoc: def open_timeout; config.open_timeout end def idle_response_timeout; config.idle_response_timeout end + def max_response_size; config.max_response_size end + def max_response_size=(val) config.max_response_size = val end # :startdoc: # The hostname this client connected to diff --git a/lib/net/imap/config.rb b/lib/net/imap/config.rb index a64ec3f2e..013cccccb 100644 --- a/lib/net/imap/config.rb +++ b/lib/net/imap/config.rb @@ -268,6 +268,40 @@ def self.[](config) false, :when_capabilities_cached, true ] + # The maximum allowed server response size. When +nil+, there is no limit + # on response size. + # + # The default value (512 MiB, since +v0.5.7+) is very high and + # unlikely to be reached. A _much_ lower value should be used with + # untrusted servers (for example, when connecting to a user-provided + # hostname). When using a lower limit, message bodies should be fetched + # in chunks rather than all at once. + # + # Please Note: this only limits the size per response. It does + # not prevent a flood of individual responses and it does not limit how + # many unhandled responses may be stored on the responses hash. See + # Net::IMAP@Unbounded+memory+use. + # + # Socket reads are limited to the maximum remaining bytes for the current + # response: max_response_size minus the bytes that have already been read. + # When the limit is reached, or reading a +literal+ _would_ go over the + # limit, ResponseTooLargeError is raised and the connection is closed. + # + # Note that changes will not take effect immediately, because the receiver + # thread may already be waiting for the next response using the previous + # value. Net::IMAP#noop can force a response and enforce the new setting + # immediately. + # + # ==== Versioned Defaults + # + # Net::IMAP#max_response_size was added in +v0.2.5+ and +v0.3.9+ as an + # attr_accessor, and in +v0.4.20+ and +v0.5.7+ as a delegator to this + # config attribute. + # + # * original: +nil+ (no limit) + # * +0.5+: 512 MiB + attr_accessor :max_response_size, type: Integer? + # Controls the behavior of Net::IMAP#responses when called without any # arguments (+type+ or +block+). # @@ -446,6 +480,7 @@ def defaults_hash idle_response_timeout: 5, sasl_ir: true, enforce_logindisabled: true, + max_response_size: 512 << 20, # 512 MiB responses_without_block: :warn, parser_use_deprecated_uidplus_data: :up_to_max_size, parser_max_deprecated_uidplus_data_size: 100, @@ -459,6 +494,7 @@ def defaults_hash sasl_ir: false, responses_without_block: :silence_deprecation_warning, enforce_logindisabled: false, + max_response_size: nil, parser_use_deprecated_uidplus_data: true, parser_max_deprecated_uidplus_data_size: 10_000, ).freeze @@ -474,6 +510,7 @@ def defaults_hash version_defaults[0.5r] = Config[0.4r].dup.update( enforce_logindisabled: true, + max_response_size: 512 << 20, # 512 MiB responses_without_block: :warn, parser_use_deprecated_uidplus_data: :up_to_max_size, parser_max_deprecated_uidplus_data_size: 100, diff --git a/lib/net/imap/config/attr_type_coercion.rb b/lib/net/imap/config/attr_type_coercion.rb index 02632d501..264bc63a8 100644 --- a/lib/net/imap/config/attr_type_coercion.rb +++ b/lib/net/imap/config/attr_type_coercion.rb @@ -18,6 +18,8 @@ def attr_accessor(attr, type: nil) super(attr) AttrTypeCoercion.attr_accessor(attr, type: type) end + + module_function def Integer? = NilOrInteger end private_constant :Macros @@ -39,6 +41,8 @@ def self.attr_accessor(attr, type: nil) define_method :"#{attr}?" do send attr end if type == Boolean end + NilOrInteger = safe{->val { Integer val unless val.nil? }} + Enum = ->(*enum) { enum = safe{enum} expected = -"one of #{enum.map(&:inspect).join(", ")}" diff --git a/lib/net/imap/response_reader.rb b/lib/net/imap/response_reader.rb index 084bc5b9e..d3e819c00 100644 --- a/lib/net/imap/response_reader.rb +++ b/lib/net/imap/response_reader.rb @@ -52,7 +52,7 @@ def read_limit(limit = nil) [limit, max_response_remaining!].compact.min end - def max_response_size = 512 << 20 # TODO: Config#max_response_size + def max_response_size = client.max_response_size def max_response_remaining = max_response_size &.- bytes_read def response_too_large? = max_response_size &.< min_response_size def min_response_size = bytes_read + min_response_remaining diff --git a/test/net/imap/test_config.rb b/test/net/imap/test_config.rb index 907f360a5..aa9ed5e7f 100644 --- a/test/net/imap/test_config.rb +++ b/test/net/imap/test_config.rb @@ -427,4 +427,17 @@ def duck.to_r = 1/11111 assert_same grandchild, greatgrandchild.parent end + test "#max_response_size=(Integer | nil)" do + config = Config.new + + config.max_response_size = 10_000 + assert_equal 10_000, config.max_response_size + + config.max_response_size = nil + assert_nil config.max_response_size + + assert_raise(ArgumentError) do config.max_response_size = "invalid" end + assert_raise(TypeError) do config.max_response_size = :invalid end + end + end diff --git a/test/net/imap/test_imap_max_response_size.rb b/test/net/imap/test_imap_max_response_size.rb new file mode 100644 index 000000000..3751d0bc8 --- /dev/null +++ b/test/net/imap/test_imap_max_response_size.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "net/imap" +require "test/unit" +require_relative "fake_server" + +class IMAPMaxResponseSizeTest < Test::Unit::TestCase + include Net::IMAP::FakeServer::TestHelper + + def setup + Net::IMAP.config.reset + @do_not_reverse_lookup = Socket.do_not_reverse_lookup + Socket.do_not_reverse_lookup = true + @threads = [] + end + + def teardown + if !@threads.empty? + assert_join_threads(@threads) + end + ensure + Socket.do_not_reverse_lookup = @do_not_reverse_lookup + end + + test "#max_response_size reading literals" do + with_fake_server(preauth: true) do |server, imap| + imap.max_response_size = 12_345 + 30 + server.on("NOOP") do |resp| + resp.untagged("1 FETCH (BODY[] {12345}\r\n" + "a" * 12_345 + ")") + resp.done_ok + end + imap.noop + assert_equal "a" * 12_345, imap.responses("FETCH").first.message + end + end + + test "#max_response_size closes connection for too long line" do + Net::IMAP.config.max_response_size = 10 + run_fake_server_in_thread(preauth: false, ignore_io_error: true) do |server| + assert_raise_with_message( + Net::IMAP::ResponseTooLargeError, /exceeds max_response_size .*\b10B\b/ + ) do + with_client("localhost", port: server.port) do + fail "should not get here (greeting longer than max_response_size)" + end + end + end + end + + test "#max_response_size closes connection for too long literal" do + Net::IMAP.config.max_response_size = 1<<20 + with_fake_server(preauth: false, ignore_io_error: true) do |server, client| + client.max_response_size = 50 + server.on("NOOP") do |resp| + resp.untagged("1 FETCH (BODY[] {1000}\r\n" + "a" * 1000 + ")") + end + assert_raise_with_message( + Net::IMAP::ResponseTooLargeError, + /\d+B read \+ 1000B literal.* exceeds max_response_size .*\b50B\b/ + ) do + client.noop + fail "should not get here (FETCH literal longer than max_response_size)" + end + end + end + +end diff --git a/test/net/imap/test_response_reader.rb b/test/net/imap/test_response_reader.rb index 154aff6a1..5d64c07a0 100644 --- a/test/net/imap/test_response_reader.rb +++ b/test/net/imap/test_response_reader.rb @@ -11,6 +11,7 @@ def setup class FakeClient def config = @config ||= Net::IMAP.config.new + def max_response_size = config.max_response_size end def literal(str) = "{#{str.bytesize}}\r\n#{str}" @@ -49,22 +50,14 @@ def literal(str) = "{#{str.bytesize}}\r\n#{str}" assert_equal "", rcvr.read_response_buffer.to_str end - class LimitedResponseReader < Net::IMAP::ResponseReader - attr_reader :max_response_size - def initialize(*args, max_response_size:) - super(*args) - @max_response_size = max_response_size - end - end - test "#read_response_buffer with max_response_size" do client = FakeClient.new - max_response_size = 10 + client.config.max_response_size = 10 under = "+ 3456\r\n" exact = "+ 345678\r\n" over = "+ 3456789\r\n" io = StringIO.new([under, exact, over].join) - rcvr = LimitedResponseReader.new(client, io, max_response_size:) + rcvr = Net::IMAP::ResponseReader.new(client, io) assert_equal under, rcvr.read_response_buffer.to_str assert_equal exact, rcvr.read_response_buffer.to_str assert_raise Net::IMAP::ResponseTooLargeError do From 364869b4e674e6c2dd240835b0f3b1e5436fff82 Mon Sep 17 00:00:00 2001 From: nick evans Date: Mon, 21 Apr 2025 22:37:53 -0400 Subject: [PATCH 36/36] =?UTF-8?q?=F0=9F=94=96=20Bump=20version=20to=20v0.5?= =?UTF-8?q?.7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/net/imap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/net/imap.rb b/lib/net/imap.rb index d7e304540..30f91fc68 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -788,7 +788,7 @@ module Net # * {IMAP URLAUTH Authorization Mechanism Registry}[https://www.iana.org/assignments/urlauth-authorization-mechanism-registry/urlauth-authorization-mechanism-registry.xhtml] # class IMAP < Protocol - VERSION = "0.5.6" + VERSION = "0.5.7" # Aliases for supported capabilities, to be used with the #enable command. ENABLE_ALIASES = {