Skip to content

Commit

Permalink
🗑️ Add deprecation warnings to .new and #starttls [🚧 WIP]
Browse files Browse the repository at this point in the history 
* Moved all deprecated option handling to DeprecatedClientOptions, which
  is prepended to Net::IMAP.  This allows `Net::IMAP` to cleanly
  implement and document the simplified keyword args API, while still
  providing a place to document the old APIs.

  Some weird and potentially confusing edge-cases were converted to
  `ArgumentError`.  The very old forms are documented and will warn
  loudly.  Some cases are obsolete but don't print any warnings... yet.

* `ssl` was renamed to `tls` in most places, with backwards compatible
  aliases.  Using `ssl` does not print any deprecation warnings.  Using
  both `tls` and `ssl` keywords raises an ArgumentError.

* ♻️ Additionally, split `initialize` up into small helper methods making
  it easier to understand at a glance.

* Preparing for a (backwards-incompatible) secure-by-default
  configuration, `Net::IMAP.default_tls` will determine the value for
  `tls` when no explicit port or tls setting is provided.  Using port
  143 will be insecure by default.  Using port 993 will be secure by
  default.  Providing no explicit port will use `Net::IMAP.default_tls`
  with the appropriate port.  And providing any other unknown port will
  use `default_tls` with a warning.

  🚧 TODO: should we use a different config var for default tls params
  when port is 993 and `tls` is unspecified?

  🚧 TODO: should we use a different config var for choosing `tls` when
  `port` is non-standard vs choosing `port` and `tls` when neither are
  specified?

  🚧 TODO: should we use a different var for `default_tls` be used to
  config params when port is 993 but tls is implicit? Another var?
  • Loading branch information
@nevans
nevans committed Aug 28, 2023
1 parent fb9d9ee commit 688fae2
Show file tree
Hide file tree
Showing 2 changed files with 319 additions and 114 deletions.
267 changes: 153 additions & 114 deletions lib/net/imap.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,107 @@ 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 +tls+ option of ::new, when +port+ is
# unspecified or non-standard.
#
# *Note*: A future release of Net::IMAP will set the 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 +true+ for the secure default without warnings. Set to
# +false+ to globally silence warnings and use insecure defaults.
attr_accessor :default_tls
alias default_ssl default_tls
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)
# [tls]
# When +true+, the connection will use TLS with the default params set by
# {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params].
# Assign a hash to override TLS params—the keys are assignment methods on
# SSLContext[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html].
#
# When <tt>port: 993</tt>, +tls+ defaults to +true+.
# When <tt>port: 143</tt>, +tls+ defaults to +false+.
# When port is unspecified or non-standard, +tls+ defaults to
# ::default_tls. When ::default_tls is also +nil+, a warning is printed
# and the connection does _not_ use TLS.
#
# When +nil+ or unassigned a default value is assigned: the default is
# +true+ if <tt>port: 993</tt>, +false+ if <tt>port: 143</tt>, and
# ::default_tls when +port+ is unspecified or non-standard. When
# ::default_tls is +nil+, a back
#
# [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,
tls: nil,
open_timeout: 30,
idle_response_timeout: 5)
# Basic configuration
@host = host
@tls, @port = default_tls_and_port(tls, 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

# DEPRECATED
@client_thread = Thread.current

# create the connection
@sock = nil
start_connection
end

def client_thread # :nodoc:
Expand Down Expand Up @@ -795,7 +896,7 @@ def capabilities
# servers will drop all <tt>AUTH=</tt> mechanisms from #capabilities after
# the connection has authenticated.
#
# imap = Net::IMAP.new(hostname, ssl: false)
# imap = Net::IMAP.new(hostname, tls: false)
# imap.capabilities # => ["IMAP4REV1", "LOGINDISABLED"]
# imap.auth_mechanisms # => []
#
Expand Down Expand Up @@ -970,15 +1071,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)
Expand Down Expand Up @@ -2213,99 +2308,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_tls_and_port(tls, port)
if tls.nil? && port
tls = true if port == SSL_PORT || /\Aimaps\z/i === port
tls = false if port == PORT
elsif port.nil? && !tls.nil?
port = tls ? SSL_PORT : PORT
end
if tls.nil? && port.nil?
tls = self.class.default_tls.dup.freeze
port = tls ? SSL_PORT : PORT
if tls.nil?
warn "A future version of Net::IMAP.default_tls " \
"will default to 'true', for secure connections by default. " \
"Use 'Net::IMAP.new(host, tls: false)' or set " \
"Net::IMAP.default_tls = 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
tls &&= tls.respond_to?(:to_hash) ? tls.to_hash : {}
[tls, 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(@tls) if @tls
@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)
Expand Down Expand Up @@ -2592,34 +2650,12 @@ 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"
raise "OpenSSL extension not installed"
end
if @sock.kind_of?(OpenSSL::SSL::SSLSocket)
raise RuntimeError, "already using SSL"
end
begin
params = params.to_hash
rescue NoMethodError
params = {}
raise RuntimeError, "already using TLS"
end
context = SSLContext.new
context.set_params(params)
Expand Down Expand Up @@ -2655,3 +2691,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
Loading

0 comments on commit 688fae2

Please sign in to comment.