From e479952491e4db96b3efd3cead48ae54912a3a13 Mon Sep 17 00:00:00 2001 From: nick evans Date: Fri, 20 Jan 2023 23:52:57 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=97=91=EF=B8=8F=20Add=20deprecation=20war?= =?UTF-8?q?nings=20to=20.new=20and=20#starttls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preparing for a (backwards-incompatible) secure-by-default configuration, Net::IMAP.default_ssl will be used when no explicit port or tls setting is provided. TODO: should truthy default_ssl be used to config params when port is 993 but ssl is implicit? Another var? Moved all deprecated option handling to DeprecatedClientOptions, which is prepended to Net::IMAP. Additionally, split `initialize` up into small helper methods making it easier to understand at a glance. --- lib/net/imap.rb | 257 ++++++++++++---------- lib/net/imap/deprecated_client_options.rb | 83 +++++++ 2 files changed, 227 insertions(+), 113 deletions(-) create mode 100644 lib/net/imap/deprecated_client_options.rb diff --git a/lib/net/imap.rb b/lib/net/imap.rb index 72fb8b0d..b7d53aaa 100644 --- a/lib/net/imap.rb +++ b/lib/net/imap.rb @@ -716,11 +716,104 @@ class << self alias default_imap_port default_port alias default_imaps_port default_tls_port alias default_ssl_port default_tls_port + + # The default value for the +ssl+ option of ::new, when +port+ is + # unspecified or non-standard. + # + # Defaults to +nil+ for backward compatibility, which prints a warning and + # does _not_ use TLS. + # + # >>> + # *Note*: A future version of Net::IMAP will default to +true+, as per + # RFC7525[https://tools.ietf.org/html/rfc7525], + # RFC7817[https://tools.ietf.org/html/rfc7817], + # and RFC8314[https://tools.ietf.org/html/rfc8314]. + # + # Set to +false+ to *globally* use insecure defaults and silence warnings. + # Send ssl: false to ::new to explicitly silence warnings for a + # single connection. + attr_accessor :default_ssl + end + + # Creates a new Net::IMAP object and connects it to the specified + # +host+. + # + # Accepts the following options: + # + # [port] + # Port number (default value is 143 for imap, or 993 for imaps) + # [ssl] + # When +true+, the connection will use TLS using the defaults chosen by + # {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params]. + # Use a hash to override the defaults---the keys are assignment methods on + # SSLContext[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html]. + # Defaults to +true+ or +false+ to match +port+, or to ::default_ssl when + # +port+ is unspecified or non-standard. + # [open_timeout] + # Seconds to wait until a connection is opened + # [idle_response_timeout] + # Seconds to wait until an IDLE response is received + # + # The most common errors are: + # + # Errno::ECONNREFUSED:: Connection refused by +host+ or an intervening + # firewall. + # Errno::ETIMEDOUT:: Connection timed out (possibly due to packets + # being dropped by an intervening firewall). + # Errno::ENETUNREACH:: There is no route to that network. + # SocketError:: Hostname not known or other socket error. + # Net::IMAP::ByeResponseError:: Connected to the host successfully, but + # it immediately said goodbye. + def initialize(host, + port: nil, + ssl: nil, + open_timeout: 30, + idle_response_timeout: 5) + # Basic configuration + @host = host + @ssl, @port = default_ssl_and_port(ssl, port) + @open_timeout = Integer(open_timeout) + @idle_response_timeout = Integer(idle_response_timeout) + + # Basic Client state + super() # Mutex and condition vars (MonitorMixin#initialize) + @greeting = nil + @capabilities = nil + @utf8_strings = false # TODO: use @enabled instead + @debug_output_bol = true + + # Client Protocol Reciever + @parser = ResponseParser.new + @receiver_thread = nil + @receiver_thread_terminating = false + @exception = nil + + # Client Protocol Sender + @tag_prefix = "RUBY" + @tagno = 0 + + # Response handlers + @continuation_request_arrival = new_cond + @continuation_request_exception = nil + @tagged_response_arrival = new_cond + @tagged_responses = {} + @response_handlers = [] + @responses = Hash.new {|h, k| h[k] = [] } + + # Command execution state + @logout_command_tag = nil + @continued_command_tag = nil + @idle_done_cond = nil + + # create the connection + @sock = nil + start_connection end def client_thread # :nodoc: - warn "Net::IMAP#client_thread is deprecated and will be removed soon." - @client_thread + warn "Net::IMAP#client_thread is deprecated and always returns the " \ + "caller's current thread." + Thread.current end # Disconnects from the server. @@ -966,15 +1059,9 @@ def logout # Server capabilities may change after #starttls, #login, and #authenticate. # Cached #capabilities will be cleared when this method completes. # - def starttls(options = {}, verify = true) + def starttls(options = {}) send_command("STARTTLS") do |resp| if resp.kind_of?(TaggedResponse) && resp.name == "OK" - begin - # for backward compatibility - certs = options.to_str - options = create_ssl_params(certs, verify) - rescue NoMethodError - end clear_cached_capabilities clear_responses start_tls_session(options) @@ -2190,99 +2277,62 @@ def remove_response_handler(handler) @@debug = false - # :call-seq: - # Net::IMAP.new(host, options = {}) - # - # Creates a new Net::IMAP object and connects it to the specified - # +host+. - # - # +options+ is an option hash, each key of which is a symbol. - # - # The available options are: - # - # port:: Port number (default value is 143 for imap, or 993 for imaps) - # ssl:: If +options[:ssl]+ is true, then an attempt will be made - # to use SSL (now TLS) to connect to the server. - # If +options[:ssl]+ is a hash, it's passed to - # OpenSSL::SSL::SSLContext#set_params as parameters. - # open_timeout:: Seconds to wait until a connection is opened - # idle_response_timeout:: Seconds to wait until an IDLE response is received - # - # The most common errors are: - # - # Errno::ECONNREFUSED:: Connection refused by +host+ or an intervening - # firewall. - # Errno::ETIMEDOUT:: Connection timed out (possibly due to packets - # being dropped by an intervening firewall). - # Errno::ENETUNREACH:: There is no route to that network. - # SocketError:: Hostname not known or other socket error. - # Net::IMAP::ByeResponseError:: The connected to the host was successful, but - # it immediately said goodbye. - def initialize(host, port_or_options = {}, - usessl = false, certs = nil, verify = true) - super() - @host = host - begin - options = port_or_options.to_hash - rescue NoMethodError - # for backward compatibility - options = {} - options[:port] = port_or_options - if usessl - options[:ssl] = create_ssl_params(certs, verify) + def default_ssl_and_port(ssl, port) + if ssl.nil? && port + ssl = true if port == SSL_PORT || /\Aimaps\z/i === port + ssl = false if port == PORT + elsif port.nil? && !ssl.nil? + port = ssl ? SSL_PORT : PORT + end + if ssl.nil? && port.nil? + ssl = self.class.default_ssl.dup.freeze + port = ssl ? SSL_PORT : PORT + if ssl.nil? + warn "A future version of Net::IMAP.default_ssl " \ + "will default to 'true', for secure connections by default. " \ + "Use 'Net::IMAP.new(host, ssl: false)' or set " \ + "Net::IMAP.default_ssl = false' to silence this warning." end end - @port = options[:port] || (options[:ssl] ? SSL_PORT : PORT) - @tag_prefix = "RUBY" - @tagno = 0 - @utf8_strings = false - @open_timeout = options[:open_timeout] || 30 - @idle_response_timeout = options[:idle_response_timeout] || 5 - @parser = ResponseParser.new + ssl &&= ssl.respond_to?(:to_hash) ? ssl.to_hash : {} + [ssl, port] + end + + def start_connection @sock = tcp_socket(@host, @port) begin - if options[:ssl] - start_tls_session(options[:ssl]) - @usessl = true - else - @usessl = false - end - @responses = Hash.new {|h, k| h[k] = [] } - @tagged_responses = {} - @response_handlers = [] - @tagged_response_arrival = new_cond - @continued_command_tag = nil - @continuation_request_arrival = new_cond - @continuation_request_exception = nil - @idle_done_cond = nil - @logout_command_tag = nil - @debug_output_bol = true - @exception = nil - + start_tls_session(@ssl) if @ssl @greeting = get_response - if @greeting.nil? - raise Error, "connection closed" - end - record_untagged_response_code @greeting - @capabilities = capabilities_from_resp_code @greeting - if @greeting.name == "BYE" - raise ByeResponseError, @greeting - end - - @client_thread = Thread.current - @receiver_thread = Thread.start { - begin - receive_responses - rescue Exception - end - } - @receiver_thread_terminating = false + handle_server_greeting + @receiver_thread = start_receiver_thread rescue Exception @sock.close raise end end + def handle_server_greeting + if @greeting.nil? + raise Error, "connection closed" + end + record_untagged_response_code(@greeting) + @capabilities = capabilities_from_resp_code @greeting + if @greeting.name == "BYE" + raise ByeResponseError, @greeting + end + end + + def start_receiver_thread + Thread.start do + receive_responses + rescue Exception + # don't exit the thread with an exception + end + rescue Exception + @sock.close + raise + end + def tcp_socket(host, port) s = Socket.tcp(host, port, :connect_timeout => @open_timeout) s.setsockopt(:SOL_SOCKET, :SO_KEEPALIVE, true) @@ -2569,23 +2619,6 @@ def normalize_searching_criteria(keys) end end - def create_ssl_params(certs = nil, verify = true) - params = {} - if certs - if File.file?(certs) - params[:ca_file] = certs - elsif File.directory?(certs) - params[:ca_path] = certs - end - end - if verify - params[:verify_mode] = VERIFY_PEER - else - params[:verify_mode] = VERIFY_NONE - end - return params - end - def start_tls_session(params = {}) unless defined?(OpenSSL::SSL) raise "SSL extension not installed" @@ -2593,11 +2626,6 @@ def start_tls_session(params = {}) if @sock.kind_of?(OpenSSL::SSL::SSLSocket) raise RuntimeError, "already using SSL" end - begin - params = params.to_hash - rescue NoMethodError - params = {} - end context = SSLContext.new context.set_params(params) if defined?(VerifyCallbackProc) @@ -2632,3 +2660,6 @@ def self.saslprep(string, **opts) require_relative "imap/response_data" require_relative "imap/response_parser" require_relative "imap/authenticators" + +require_relative "imap/deprecated_client_options" +Net::IMAP.prepend Net::IMAP::DeprecatedClientOptions diff --git a/lib/net/imap/deprecated_client_options.rb b/lib/net/imap/deprecated_client_options.rb new file mode 100644 index 00000000..50ad5148 --- /dev/null +++ b/lib/net/imap/deprecated_client_options.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Net + class IMAP < Protocol + + # This module handles deprecated arguments to various methods. It will be + # deleted in a future release. + module DeprecatedClientOptions + UNDEFINED = Module.new.freeze + private_constant :UNDEFINED + + # Passing any arguments *both* positionally and as a keyword will raise an + # ArgumentError. + # + # Allows port to be sent as an integer or string without warning or error. + # + # SSL options sent positionally will print a deprecation warning (and, + # eventually, raise an argument error). + def initialize(host, *deprecated, **options) + unless deprecated.empty? + port_or_options, sslopts = deprecated + if !port_or_options.nil? && + (port_or_options.respond_to?(:to_hash) ? + options.empty? : options.key?(:port)) || + !sslopts.nil? && options.key?(:ssl) + raise ArgumentError, "Don't use both positional and keyword options" + end + # handle port_or_options => options + if port_or_options.respond_to?(:to_hash) + warn "DEPRECATED: options should be set by keyword arguments" + options = port_or_options.to_hash + elsif !port_or_options.nil? + warn "DEPRECATED: port should be set by keyword argument" + options[:port] = port_or_options + end + # handle ssl options + unless sslopts.nil? + warn "DEPRECATED: SSL options should be set by keyword argument" + usessl, certs, verify = sslopts + if usessl + options[:ssl] = create_ssl_params(certs, verify) + end + end + end + super(host, **options) + end + + # +call-seq: + # starttls(options = {}) + # starttls(certs, verify = true) + # + # For backward compatibility. A future release will only accept + # OpenSSL::SSL::SSLContext.set_params options. + def starttls(options = {}, verify = UNDEFINED) + if options.respond_to?(:to_str) + warn "DEPRECATED: starttls(certs, verify). Use starttls(ssl_params)" + certs = options.to_str + verify = true if verify == UNDEFINED + options = create_ssl_params(certs, verify) + end + super(options) + end + + private + + def create_ssl_params(certs, verify) + params = {} + if certs + if File.file?(certs) + params[:ca_file] = certs + elsif File.directory?(certs) + params[:ca_path] = certs + end + end + params[:verify_mode] = verify ? VERIFY_PEER : VERIFY_NONE + params + end + + end + + prepend DeprecatedClientOptions + end +end