Skip to content

Commit 782cea8

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 e588adb commit 782cea8

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
@@ -800,8 +800,21 @@ def idle_response_timeout; config.idle_response_timeout end
800800
# Returns +false+ for a plaintext connection.
801801
attr_reader :ssl_ctx_params
802802

803-
# Creates a new Net::IMAP object and connects it to the specified
804-
# +host+.
803+
# Creates a new Net::IMAP object and connects it to the specified +host+.
804+
#
805+
# ==== Default port and SSL
806+
#
807+
# When both both +port+ and +ssl+ are unspecified or +nil+,
808+
# +ssl+ is determined by {config.default_ssl}[rdoc-ref:Config#default_ssl]
809+
# and +port+ is based on that implicit value for +ssl+.
810+
#
811+
# When only one of the two is specified:
812+
# * When +ssl+ is truthy, +port+ defaults to +993+.
813+
# * When +ssl+ is +false+, +port+ defaults to +143+.
814+
# * When +port+ is +993+, +ssl+ defaults to +true+.
815+
# * When +port+ is +143+, +ssl+ defaults to +false+.
816+
# * When +port+ is nonstandard, the default for +ssl+ is determined
817+
# by {config.default_ssl}[rdoc-ref:Config#default_ssl].
805818
#
806819
# ==== Options
807820
#
@@ -829,7 +842,9 @@ def idle_response_timeout; config.idle_response_timeout end
829842
# SSL session verification mode. Valid modes include
830843
# +OpenSSL::SSL::VERIFY_PEER+ and +OpenSSL::SSL::VERIFY_NONE+.
831844
#
832-
# See {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html] for other valid SSL context params.
845+
# See
846+
# {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html]
847+
# for other valid SSL context params.
833848
#
834849
# See DeprecatedClientOptions.new for deprecated SSL arguments.
835850
#
@@ -910,7 +925,7 @@ def initialize(host, port: nil, ssl: nil,
910925
# Config options
911926
@host = host
912927
@config = Config.new(config, **config_options)
913-
@port = port || (ssl ? SSL_PORT : PORT)
928+
ssl, @port = default_ssl_and_port(ssl, port)
914929
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl)
915930

916931
# Basic Client State
@@ -2633,6 +2648,27 @@ def remove_response_handler(handler)
26332648
PORT = 143 # :nodoc:
26342649
SSL_PORT = 993 # :nodoc:
26352650

2651+
def default_ssl_and_port(tls, port)
2652+
if tls.nil? && port
2653+
tls = true if port == SSL_PORT || /\Aimaps\z/i === port
2654+
tls = false if port == PORT
2655+
elsif port.nil? && !tls.nil?
2656+
port = tls ? SSL_PORT : PORT
2657+
end
2658+
if tls.nil? && port.nil?
2659+
tls = config.default_tls.dup.freeze
2660+
port = tls ? SSL_PORT : PORT
2661+
if tls.nil?
2662+
warn "A future version of Net::IMAP::Config#default_tls " \
2663+
"will default to 'true', for secure connections by default. " \
2664+
"Use 'Net::IMAP.new(host, ssl: false)' or " \
2665+
"Net::IMAP.config.default_tls = false' to silence this warning."
2666+
end
2667+
end
2668+
tls &&= tls.respond_to?(:to_hash) ? tls.to_hash : {}
2669+
[tls, port]
2670+
end
2671+
26362672
def start_imap_connection
26372673
@greeting = get_server_greeting
26382674
@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
@@ -305,6 +328,7 @@ def defaults_hash
305328
debug: false,
306329
open_timeout: 30,
307330
idle_response_timeout: 5,
331+
default_ssl: nil,
308332
sasl_ir: true,
309333
responses_without_block: :silence_deprecation_warning,
310334
).freeze

0 commit comments

Comments
 (0)