Skip to content

Commit 28888ec

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 62710b9 commit 28888ec

File tree

2 files changed

+79
-5
lines changed

2 files changed

+79
-5
lines changed

lib/net/imap.rb

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -827,8 +827,21 @@ def idle_response_timeout; config.idle_response_timeout end
827827
# Returns +false+ for a plaintext connection.
828828
attr_reader :ssl_ctx_params
829829

830-
# Creates a new Net::IMAP object and connects it to the specified
831-
# +host+.
830+
# Creates a new Net::IMAP object and connects it to the specified +host+.
831+
#
832+
# ==== Default port and SSL
833+
#
834+
# When both both +port+ and +ssl+ are unspecified or +nil+,
835+
# +ssl+ is determined by {config.default_ssl}[rdoc-ref:Config#default_ssl]
836+
# and +port+ is based on that implicit value for +ssl+.
837+
#
838+
# When only one of the two is specified:
839+
# * When +ssl+ is truthy, +port+ defaults to +993+.
840+
# * When +ssl+ is +false+, +port+ defaults to +143+.
841+
# * When +port+ is +993+, +ssl+ defaults to +true+.
842+
# * When +port+ is +143+, +ssl+ defaults to +false+.
843+
# * When +port+ is nonstandard, the default for +ssl+ is determined
844+
# by {config.default_ssl}[rdoc-ref:Config#default_ssl].
832845
#
833846
# ==== Options
834847
#
@@ -856,7 +869,9 @@ def idle_response_timeout; config.idle_response_timeout end
856869
# SSL session verification mode. Valid modes include
857870
# +OpenSSL::SSL::VERIFY_PEER+ and +OpenSSL::SSL::VERIFY_NONE+.
858871
#
859-
# See {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html] for other valid SSL context params.
872+
# See
873+
# {OpenSSL::SSL::SSLContext}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html]
874+
# for other valid SSL context params.
860875
#
861876
# See DeprecatedClientOptions.new for deprecated SSL arguments.
862877
#
@@ -937,7 +952,7 @@ def initialize(host, port: nil, ssl: nil,
937952
# Config options
938953
@host = host
939954
@config = Config.new(config, **config_options)
940-
@port = port || (ssl ? SSL_PORT : PORT)
955+
ssl, @port = default_ssl_and_port(ssl, port)
941956
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl)
942957

943958
# Basic Client State
@@ -3169,6 +3184,27 @@ def remove_response_handler(handler)
31693184
PORT = 143 # :nodoc:
31703185
SSL_PORT = 993 # :nodoc:
31713186

3187+
def default_ssl_and_port(tls, port)
3188+
if tls.nil? && port
3189+
tls = true if port == SSL_PORT || /\Aimaps\z/i === port
3190+
tls = false if port == PORT
3191+
elsif port.nil? && !tls.nil?
3192+
port = tls ? SSL_PORT : PORT
3193+
end
3194+
if tls.nil? && port.nil?
3195+
tls = config.default_tls.dup.freeze
3196+
port = tls ? SSL_PORT : PORT
3197+
if tls.nil?
3198+
warn "A future version of Net::IMAP::Config#default_tls " \
3199+
"will default to 'true', for secure connections by default. " \
3200+
"Use 'Net::IMAP.new(host, ssl: false)' or " \
3201+
"Net::IMAP.config.default_tls = false' to silence this warning."
3202+
end
3203+
end
3204+
tls &&= tls.respond_to?(:to_hash) ? tls.to_hash : {}
3205+
[tls, port]
3206+
end
3207+
31723208
def start_imap_connection
31733209
@greeting = get_server_greeting
31743210
@capabilities = capabilities_from_resp_code @greeting

lib/net/imap/config.rb

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,35 @@ def self.[](config)
209209
# The default value is +5+ seconds.
210210
attr_accessor :idle_response_timeout, type: Integer
211211

212+
# The default value for the +ssl+ option of Net::IMAP.new, when +port+ is
213+
# unspecified or non-standard and +ssl+ is unspecified. default_ssl is
214+
# ignored when Net::IMAP.new is called with any value for +ssl+ besides
215+
# +nil+,
216+
#
217+
# *Note*: A future release of Net::IMAP will set the default to +true+, as
218+
# per RFC7525[https://tools.ietf.org/html/rfc7525],
219+
# RFC7817[https://tools.ietf.org/html/rfc7817], and
220+
# RFC8314[https://tools.ietf.org/html/rfc8314].
221+
#
222+
# <em>(The default_ssl config attribute was added in +v0.5.?+.)</em>
223+
#
224+
# ==== Valid options
225+
#
226+
# [+false+ <em>(original behavior)</em>]
227+
# Plaintext by default, with no warnings.
228+
# [+nil+ <em>(planned default for +v0.6+)</em>]
229+
# Plaintext by default, and prints a warning.
230+
#
231+
# <em>This option will be removed in +v0.7+, when +:warn+ becomes the
232+
# default.</em>
233+
# [+:warn+ <em>(planned default for +v0.7+)</em>]
234+
# A warning will be printed, and behave as if the default were +true+.
235+
# [+true+ <em>(planned future default)</em>]
236+
# Use TLS by default, with the default SSL context params set by calling
237+
# {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params]
238+
# with no params.
239+
attr_accessor :default_ssl, type: [false, nil, :warn, true]
240+
212241
# Whether to use the +SASL-IR+ extension when the server and \SASL
213242
# mechanism both support it. Can be overridden by the +sasl_ir+ keyword
214243
# parameter to Net::IMAP#authenticate.
@@ -425,6 +454,7 @@ def defaults_hash
425454
debug: false,
426455
open_timeout: 30,
427456
idle_response_timeout: 5,
457+
default_ssl: false,
428458
sasl_ir: true,
429459
enforce_logindisabled: true,
430460
responses_without_block: :warn,
@@ -458,11 +488,19 @@ def defaults_hash
458488

459489
version_defaults[0.6] = Config[0.5].dup.update(
460490
responses_without_block: :frozen_dup,
491+
default_ssl: nil,
461492
parser_use_deprecated_uidplus_data: false,
462493
parser_max_deprecated_uidplus_data_size: 0,
463494
).freeze
464495
version_defaults[:next] = Config[0.6]
465-
version_defaults[:future] = Config[:next]
496+
497+
version_defaults[0.7] = Config[0.6].dup.update(
498+
default_ssl: :warn,
499+
).freeze
500+
501+
version_defaults[:future] = Config[0.7].dup.update(
502+
default_ssl: true,
503+
).freeze
466504

467505
version_defaults.freeze
468506
end

0 commit comments

Comments
 (0)