Skip to content

Commit e2e1426

Browse files
nevansnevans
committed
🔒🗑️ Deprecate old Net::IMAP.new parameters
Converts `Net::IMAP#initialize` to use keyword parameters for all options. Creates `Net::IMAP::DeprecatedClientOptions`, which is prepended to `Net::IMAP` to override `#initialize`. Moved all of the original argument handling into it. Obsolete, but not yet deprecated: * Using an `options` hash instead of keyword parameters. * Sending `port` as the second parameter. Deprecated (will raise a warning): * Setting `ssl` params via `usessl`, `certs`, or `verify` arguments.
1 parent cfb8caf commit e2e1426

File tree

4 files changed

+246
-41
lines changed

4 files changed

+246
-41
lines changed

‎lib/net/imap.rb

Lines changed: 10 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ class << self
760760
# [idle_response_timeout]
761761
# Seconds to wait until an IDLE response is received
762762
#
763-
# See DeprecatedClientOptions for obsolete backwards compatible arguments.
763+
# See DeprecatedClientOptions.new for deprecated arguments.
764764
#
765765
# ==== Examples
766766
#
@@ -810,16 +810,15 @@ class << self
810810
# [Net::IMAP::ByeResponseError]
811811
# Connected to the host successfully, but it immediately said goodbye.
812812
#
813-
def initialize(host, options = {}, *deprecated)
813+
def initialize(host, port: nil, ssl: nil,
814+
open_timeout: 30, idle_response_timeout: 5)
814815
super()
815-
options = convert_deprecated_options(options, *deprecated)
816-
817816
# Config options
818817
@host = host
819-
@port = options[:port] || (options[:ssl] ? SSL_PORT : PORT)
820-
@open_timeout = options[:open_timeout] || 30
821-
@idle_response_timeout = options[:idle_response_timeout] || 5
822-
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(options[:ssl])
818+
@port = port || (ssl ? SSL_PORT : PORT)
819+
@open_timeout = Integer(open_timeout)
820+
@idle_response_timeout = Integer(idle_response_timeout)
821+
@ssl_ctx_params, @ssl_ctx = build_ssl_ctx(ssl)
823822

824823
# Basic Client State
825824
@utf8_strings = false
@@ -2354,20 +2353,6 @@ def remove_response_handler(handler)
23542353

23552354
@@debug = false
23562355

2357-
def convert_deprecated_options(
2358-
port_or_options = {}, usessl = false, certs = nil, verify = true
2359-
)
2360-
port_or_options.to_hash
2361-
rescue NoMethodError
2362-
# for backward compatibility
2363-
options = {}
2364-
options[:port] = port_or_options
2365-
if usessl
2366-
options[:ssl] = create_ssl_params(certs, verify)
2367-
end
2368-
options
2369-
end
2370-
23712356
def start_imap_connection
23722357
@greeting = get_server_greeting
23732358
@capabilities = capabilities_from_resp_code @greeting
@@ -2695,23 +2680,6 @@ def build_ssl_ctx(ssl)
26952680
end
26962681
end
26972682

2698-
def create_ssl_params(certs = nil, verify = true)
2699-
params = {}
2700-
if certs
2701-
if File.file?(certs)
2702-
params[:ca_file] = certs
2703-
elsif File.directory?(certs)
2704-
params[:ca_path] = certs
2705-
end
2706-
end
2707-
if verify
2708-
params[:verify_mode] = VERIFY_PEER
2709-
else
2710-
params[:verify_mode] = VERIFY_NONE
2711-
end
2712-
return params
2713-
end
2714-
27152683
def start_tls_session
27162684
raise "SSL extension not installed" unless defined?(OpenSSL::SSL)
27172685
raise "already using SSL" if @sock.kind_of?(OpenSSL::SSL::SSLSocket)
@@ -2746,3 +2714,6 @@ def self.saslprep(string, **opts)
27462714
require_relative "imap/response_data"
27472715
require_relative "imap/response_parser"
27482716
require_relative "imap/authenticators"
2717+
2718+
require_relative "imap/deprecated_client_options"
2719+
Net::IMAP.prepend Net::IMAP::DeprecatedClientOptions

‎lib/net/imap/deprecated_client_options.rb

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
# frozen_string_literal: true
2+
3+
module Net
4+
class IMAP < Protocol
5+
6+
# This module handles deprecated arguments to various Net::IMAP methods.
7+
module DeprecatedClientOptions
8+
9+
# :call-seq:
10+
# Net::IMAP.new(host, **options) # standard keyword options
11+
# Net::IMAP.new(host, options) # obsolete hash options
12+
# Net::IMAP.new(host, port) # obsolete port argument
13+
# Net::IMAP.new(host, port, usessl, certs = nil, verify = true) # deprecated SSL arguments
14+
#
15+
# Translates Net::IMAP.new arguments for backward compatibility.
16+
#
17+
# ==== Obsolete arguments
18+
#
19+
# Using obsolete arguments does not a warning. Obsolete arguments will be
20+
# deprecated by a future release.
21+
#
22+
# If a second positional argument is given and it is a hash (or is
23+
# convertable via +#to_hash+), it is converted to keyword arguments.
24+
#
25+
# # Obsolete:
26+
# Net::IMAP.new("imap.example.com", options_hash)
27+
# # Use instead:
28+
# Net::IMAP.new("imap.example.com", **options_hash)
29+
#
30+
# If a second positional argument is given and it is not a hash, it is
31+
# converted to the +port+ keyword argument.
32+
# # Obsolete:
33+
# Net::IMAP.new("imap.example.com", 114433)
34+
# # Use instead:
35+
# Net::IMAP.new("imap.example.com", port: 114433)
36+
#
37+
# ==== Deprecated arguments
38+
#
39+
# Using deprecated arguments prints a warning. Convert to keyword
40+
# arguments to avoid the warning. Deprecated arguments will be removed in
41+
# a future release.
42+
#
43+
# If +usessl+ is false, +certs+, and +verify+ are ignored. When it true,
44+
# all three arguments are converted to the +ssl+ keyword argument.
45+
# Without +certs+ or +verify+, it is converted to <tt>ssl: true</tt>.
46+
# # DEPRECATED:
47+
# Net::IMAP.new("imap.example.com", nil, true) # => prints a warning
48+
# # Use instead:
49+
# Net::IMAP.new("imap.example.com", ssl: true)
50+
#
51+
# When +certs+ is a path to a directory, it is converted to <tt>ca_path:
52+
# certs</tt>.
53+
# # DEPRECATED:
54+
# Net::IMAP.new("imap.example.com", nil, true, "/path/to/certs") # => prints a warning
55+
# # Use instead:
56+
# Net::IMAP.new("imap.example.com", ssl: {ca_path: "/path/to/certs"})
57+
#
58+
# When +certs+ is a path to a file, it is converted to <tt>ca_file:
59+
# certs</tt>.
60+
# # DEPRECATED:
61+
# Net::IMAP.new("imap.example.com", nil, true, "/path/to/cert.pem") # => prints a warning
62+
# # Use instead:
63+
# Net::IMAP.new("imap.example.com", ssl: {ca_file: "/path/to/cert.pem"})
64+
#
65+
# When +verify+ is +false+, it is converted to <tt>verify_mode:
66+
# OpenSSL::SSL::VERIFY_NONE</tt>.
67+
# # DEPRECATED:
68+
# Net::IMAP.new("imap.example.com", nil, true, nil, false) # => prints a warning
69+
# # Use instead:
70+
# Net::IMAP.new("imap.example.com", ssl: {verify_mode: OpenSSL::SSL::VERIFY_NONE})
71+
#
72+
def initialize(host, port_or_options = nil, *deprecated, **options)
73+
if port_or_options.nil? && deprecated.empty?
74+
super host, **options
75+
elsif options.any?
76+
# Net::IMAP.new(host, *__invalid__, **options)
77+
raise ArgumentError, "Do not combine deprecated and keyword arguments"
78+
elsif port_or_options.respond_to?(:to_hash) and deprecated.any?
79+
# Net::IMAP.new(host, options, *__invalid__)
80+
raise ArgumentError, "Do not use deprecated SSL params with options hash"
81+
elsif port_or_options.respond_to?(:to_hash)
82+
super host, **Hash.try_convert(port_or_options)
83+
elsif deprecated.empty?
84+
super host, port: port_or_options
85+
elsif deprecated.shift
86+
warn "DEPRECATED: Call Net::IMAP.new with keyword options", uplevel: 1
87+
super host, port: port_or_options, ssl: create_ssl_params(*deprecated)
88+
else
89+
warn "DEPRECATED: Call Net::IMAP.new with keyword options", uplevel: 1
90+
super host, port: port_or_options, ssl: false
91+
end
92+
end
93+
94+
private
95+
96+
def create_ssl_params(certs = nil, verify = true)
97+
params = {}
98+
if certs
99+
if File.file?(certs)
100+
params[:ca_file] = certs
101+
elsif File.directory?(certs)
102+
params[:ca_path] = certs
103+
end
104+
end
105+
params[:verify_mode] =
106+
verify ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE
107+
params
108+
end
109+
110+
end
111+
end
112+
end

‎test/net/imap/fake_server/test_helper.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44

55
module Net::IMAP::FakeServer::TestHelper
66

7-
def run_fake_server_in_thread(timeout: 5, **opts)
7+
def run_fake_server_in_thread(ignore_io_error: false, timeout: 5, **opts)
88
Timeout.timeout(timeout) do
99
server = Net::IMAP::FakeServer.new(timeout: timeout, **opts)
10-
@threads << Thread.new do server.run end if @threads
10+
@threads << Thread.new do
11+
server.run
12+
rescue IOError
13+
raise unless ignore_io_error
14+
end
1115
yield server
1216
ensure
1317
server&.shutdown

‎test/net/imap/test_deprecated_client_options.rb

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
# frozen_string_literal: true
2+
3+
require "net/imap"
4+
require "test/unit"
5+
require_relative "fake_server"
6+
7+
class DeprecatedClientOptionsTest < Test::Unit::TestCase
8+
include Net::IMAP::FakeServer::TestHelper
9+
10+
def setup
11+
@do_not_reverse_lookup = Socket.do_not_reverse_lookup
12+
Socket.do_not_reverse_lookup = true
13+
@threads = []
14+
end
15+
16+
def teardown
17+
assert_join_threads(@threads) unless @threads.empty?
18+
ensure
19+
Socket.do_not_reverse_lookup = @do_not_reverse_lookup
20+
end
21+
22+
class InitializeTests < DeprecatedClientOptionsTest
23+
24+
test "Convert obsolete options hash to keywords" do
25+
run_fake_server_in_thread do |server|
26+
with_client(server.host, {port: server.port, ssl: false}) do |client|
27+
assert_equal server.host, client.host
28+
assert_equal server.port, client.port
29+
assert_equal false, client.ssl_ctx_params
30+
end
31+
end
32+
end
33+
34+
test "Convert obsolete port argument to :port keyword" do
35+
run_fake_server_in_thread do |server|
36+
with_client(server.host, server.port) do |client|
37+
assert_equal server.host, client.host
38+
assert_equal server.port, client.port
39+
assert_equal false, client.ssl_ctx_params
40+
end
41+
end
42+
end
43+
44+
test "Convert deprecated usessl (= false) with warning" do
45+
run_fake_server_in_thread do |server|
46+
assert_deprecated_warning(/Call Net::IMAP\.new with keyword/i) do
47+
with_client(server.host, server.port, false, :who, :cares) do |client|
48+
assert_equal server.host, client.host
49+
assert_equal server.port, client.port
50+
assert_equal false, client.ssl_ctx_params
51+
end
52+
end
53+
end
54+
end
55+
56+
test "Convert deprecated usessl (= true) and certs, with warning" do
57+
run_fake_server_in_thread(implicit_tls: true) do |server|
58+
certs = server.config.tls[:ca_file]
59+
assert_deprecated_warning(/Call Net::IMAP\.new with keyword/i) do
60+
with_client("localhost", server.port, true, certs) do |client|
61+
assert_equal "localhost", client.host
62+
assert_equal server.port, client.port
63+
assert_equal(
64+
{ca_file: certs, verify_mode: OpenSSL::SSL::VERIFY_PEER},
65+
client.ssl_ctx_params
66+
)
67+
end
68+
end
69+
end
70+
end
71+
72+
test "Convert deprecated usessl (= true) and verify (= false), with warning" do
73+
run_fake_server_in_thread(implicit_tls: true) do |server|
74+
assert_deprecated_warning(/Call Net::IMAP\.new with keyword/i) do
75+
with_client("localhost", server.port, true, nil, false) do |client|
76+
assert_equal "localhost", client.host
77+
assert_equal server.port, client.port
78+
assert_equal(
79+
{verify_mode: OpenSSL::SSL::VERIFY_NONE},
80+
client.ssl_ctx_params
81+
)
82+
assert_equal OpenSSL::SSL::VERIFY_NONE, client.ssl_ctx.verify_mode
83+
end
84+
end
85+
end
86+
end
87+
88+
test "combined options hash and keyword args raises ArgumentError" do
89+
ex = nil
90+
run_fake_server_in_thread(
91+
ignore_io_error: true, implicit_tls: true
92+
) do |server|
93+
imap = Net::IMAP.new("localhost", {port: 993}, ssl: true)
94+
rescue => ex
95+
nil
96+
ensure
97+
imap&.disconnect
98+
end
99+
assert_equal ArgumentError, ex.class
100+
end
101+
102+
test "combined options hash and ssl args raises ArgumentError" do
103+
ex = nil
104+
run_fake_server_in_thread(
105+
ignore_io_error: true, implicit_tls: true
106+
) do |server|
107+
imap = Net::IMAP.new("localhost", {port: 993}, true)
108+
rescue => ex
109+
nil
110+
ensure
111+
imap&.disconnect
112+
end
113+
assert_equal ArgumentError, ex.class
114+
end
115+
116+
end
117+
118+
end

0 commit comments

Comments
 (0)