Skip to content

Commit

Permalink
πŸ”’πŸ—‘οΈ Deprecate old starttls args, use keyword args
Browse files Browse the repository at this point in the history
`Net::IMAP#starttls` converts all incoming keyword args in params for
`OpenSSL::SSL::SSLContext#set_params`.
  • Loading branch information
nevans committed Sep 20, 2023
1 parent e2e1426 commit 2066e7f
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 9 deletions.
17 changes: 8 additions & 9 deletions lib/net/imap.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,12 @@ def logout
# Sends a {STARTTLS command [IMAP4rev1 Β§6.2.1]}[https://www.rfc-editor.org/rfc/rfc3501#section-6.2.1]
# to start a TLS session.
#
# Any +options+ are forwarded to OpenSSL::SSL::SSLContext#set_params.
# Any +options+ are forwarded directly to
# {OpenSSL::SSL::SSLContext#set_params}[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html#method-i-set_params];
# the keys are names of attribute assignment methods on
# SSLContext[https://docs.ruby-lang.org/en/master/OpenSSL/SSL/SSLContext.html].
#
# See DeprecatedClientOptions#starttls for deprecated arguments.
#
# This method returns after TLS negotiation and hostname verification are
# both successful. Any error indicates that the connection has not been
Expand All @@ -1097,14 +1102,8 @@ 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)
begin
# for backward compatibility
certs = options.to_str
options = create_ssl_params(certs, verify)
rescue NoMethodError
end
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options || {})
def starttls(**options)
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options)
send_command("STARTTLS") do |resp|
if resp.kind_of?(TaggedResponse) && resp.name == "OK"
clear_cached_capabilities
Expand Down
27 changes: 27 additions & 0 deletions lib/net/imap/deprecated_client_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,33 @@ def initialize(host, port_or_options = nil, *deprecated, **options)
end
end

# :call-seq:
# starttls(**options) # standard
# starttls(options = {}) # obsolete
# starttls(certs = nil, verify = true) # deprecated
#
# Translates Net::IMAP#starttls arguments for backward compatibility.
#
# Support for +certs+ and +verify+ will be dropped in a future release.
#
# See ::new for interpretation of +certs+ and +verify+.
def starttls(*deprecated, **options)
if deprecated.empty?
super(**options)
elsif options.any?
# starttls(*__invalid__, **options)
raise ArgumentError, "Do not combine deprecated and keyword options"
elsif deprecated.first.respond_to?(:to_hash) && deprecated.length > 1
# starttls(*__invalid__, **options)
raise ArgumentError, "Do not use deprecated verify param with options hash"
elsif deprecated.first.respond_to?(:to_hash)
super(**Hash.try_convert(deprecated.first))
else
warn "DEPRECATED: Call Net::IMAP#starttls with keyword options", uplevel: 1
super(**create_ssl_params(*deprecated))
end
end

private

def create_ssl_params(certs = nil, verify = true)
Expand Down
47 changes: 47 additions & 0 deletions test/net/imap/test_deprecated_client_options.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,51 @@ class InitializeTests < DeprecatedClientOptionsTest

end

class StartTLSTests < DeprecatedClientOptionsTest
test "Convert obsolete options hash to keywords" do
with_fake_server(preauth: false) do |server, imap|
imap.starttls(ca_file: server.config.tls[:ca_file], min_version: :TLS1_2)
assert_equal(
{ca_file: server.config.tls[:ca_file], min_version: :TLS1_2},
imap.ssl_ctx_params
)
assert imap.ssl_ctx.verify_hostname
assert_equal(server.config.tls[:ca_file], imap.ssl_ctx.ca_file)
end
end

test "Convert deprecated certs, verify with warning" do
with_fake_server(preauth: false) do |server, imap|
assert_deprecated_warning(/Call Net::IMAP#starttls with keyword/i) do
imap.starttls(server.config.tls[:ca_file], false)
end
assert_equal(
{
ca_file: server.config.tls[:ca_file],
verify_mode: OpenSSL::SSL::VERIFY_NONE,
},
imap.ssl_ctx_params
)
assert_equal server.config.tls[:ca_file], imap.ssl_ctx.ca_file
end
end

test "combined options hash and keyword args raises ArgumentError" do
with_fake_server(preauth: false) do |server, imap|
assert_raise(ArgumentError) do
imap.starttls({min_version: :TLS1_2},
ca_file: server.config.tls[:ca_file])
end
end
end

test "combined options hash and ssl args raises ArgumentError" do
with_fake_server(preauth: false) do |server, imap|
assert_raise(ArgumentError) do
imap.starttls({min_version: :TLS1_2}, false)
end
end
end
end

end

0 comments on commit 2066e7f

Please sign in to comment.