Skip to content

Commit 0cd91b4

Browse files
committed
🗑️ Add deprecation warnings to .new and #starttls [🚧 WIP]
* `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. * 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?
1 parent cf7e62c commit 0cd91b4

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed

lib/net/imap.rb

+40-4
Original file line numberDiff line numberDiff line change
@@ -802,8 +802,21 @@ def idle_response_timeout; config.idle_response_timeout end
802802
# Returns +false+ for a plaintext connection.
803803
attr_reader :ssl_ctx_params
804804

805-
# Creates a new Net::IMAP object and connects it to the specified
806-
# +host+.
805+
# Creates a new Net::IMAP object and connects it to the specified +host+.
806+
#
807+
# ==== Default port and SSL
808+
#
809+
# When both both +port+ and +ssl+ are unspecified or +nil+,
810+
# +ssl+ is determined by {config.default_ssl}[rdoc-ref:Config#default_ssl]
811+
# and +port+ is based on that implicit value for +ssl+.
812+
#
813+
# When only one of the two is specified:
814+
# * When +ssl+ is truthy, +port+ defaults to +993+.
815+
# * When +ssl+ is +false+, +port+ defaults to +143+.
816+
# * When +port+ is +993+, +ssl+ defaults to +true+.
817+
# * When +port+ is +143+, +ssl+ defaults to +false+.
818+
# * When +port+ is nonstandard, the default for +ssl+ is determined
819+
# by {config.default_ssl}[rdoc-ref:Config#default_ssl].
807820
#
808821
# ==== Options
809822
#
@@ -831,7 +844,9 @@ def idle_response_timeout; config.idle_response_timeout end
831844
# SSL session verification mode. Valid modes include
832845
# +OpenSSL::SSL::VERIFY_PEER+ and +OpenSSL::SSL::VERIFY_NONE+.
833846
#
834-
# See {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html] for other valid SSL context params.
847+
# See
848+
# {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html]
849+
# for other valid SSL context params.
835850
#
836851
# See DeprecatedClientOptions.new for deprecated SSL arguments.
837852
#
@@ -912,7 +927,7 @@ def initialize(host, port: nil, ssl: nil,
912927
# Config options
913928
@host = host
914929
@config = Config.new(config, **config_options)
915-
@port = port || (ssl ? SSL_PORT : PORT)
930+
ssl, @port = default_ssl_and_port(ssl, port)
916931
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl)
917932

918933
# Basic Client State
@@ -2654,6 +2669,27 @@ def remove_response_handler(handler)
26542669
PORT = 143 # :nodoc:
26552670
SSL_PORT = 993 # :nodoc:
26562671

2672+
def default_ssl_and_port(tls, port)
2673+
if tls.nil? && port
2674+
tls = true if port == SSL_PORT || /\Aimaps\z/i === port
2675+
tls = false if port == PORT
2676+
elsif port.nil? && !tls.nil?
2677+
port = tls ? SSL_PORT : PORT
2678+
end
2679+
if tls.nil? && port.nil?
2680+
tls = config.default_tls.dup.freeze
2681+
port = tls ? SSL_PORT : PORT
2682+
if tls.nil?
2683+
warn "A future version of Net::IMAP::Config#default_tls " \
2684+
"will default to 'true', for secure connections by default. " \
2685+
"Use 'Net::IMAP.new(host, ssl: false)' or " \
2686+
"Net::IMAP.config.default_tls = false' to silence this warning."
2687+
end
2688+
end
2689+
tls &&= tls.respond_to?(:to_hash) ? tls.to_hash : {}
2690+
[tls, port]
2691+
end
2692+
26572693
def start_imap_connection
26582694
@greeting = get_server_greeting
26592695
@capabilities = capabilities_from_resp_code @greeting

lib/net/imap/config.rb

+24
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,29 @@ def self.[](config)
200200
# The default value is +5+ seconds.
201201
attr_accessor :idle_response_timeout, type: Integer
202202

203+
# :markup: markdown
204+
#
205+
# The default value for the +ssl+ option of Net::IMAP.new, when +port+ is
206+
# unspecified or non-standard.
207+
#
208+
# *Note*: A future release of Net::IMAP will set the default to +true+, as
209+
# per RFC7525[https://tools.ietf.org/html/rfc7525],
210+
# RFC7817[https://tools.ietf.org/html/rfc7817], and
211+
# RFC8314[https://tools.ietf.org/html/rfc8314].
212+
#
213+
# * Set to +true+ for the secure default without warnings.
214+
# * Set to +:warn+ for the secure default _with_ warnings.
215+
# * Set to +nil+ to use insecure defaults with warnings.
216+
# * Set to +false+ to silence warnings and use insecure defaults.
217+
#
218+
# | Starting with version | The default value is |
219+
# |-------------------------|------------------------|
220+
# | _original_ | +false+ |
221+
# | v0.6 <em>(planned)</em> | +nil+ |
222+
# | v0.7 <em>(planned)</em> | +:warn+ |
223+
# | _eventually_ | +true+ |
224+
attr_accessor :default_ssl
225+
203226
# :markup: markdown
204227
#
205228
# Whether to use the +SASL-IR+ extension when the server and \SASL
@@ -331,6 +354,7 @@ def defaults_hash
331354
debug: false,
332355
open_timeout: 30,
333356
idle_response_timeout: 5,
357+
default_ssl: nil,
334358
sasl_ir: true,
335359
enforce_logindisabled: true,
336360
responses_without_block: :warn,

0 commit comments

Comments
 (0)