Skip to content

Commit 26a0f55

Browse files
committed
Autoload IPAddr instead of requiring it eagerly
openssl/ssl.rb requires ipaddr at the top of the file. The only use is in OpenSSL::SSL.verify_certificate_identity, and only for certificates carrying an iPAddress SAN: return true if san.value == IPAddr.new(hostname).hton rescue IPAddr::InvalidAddressError Autoload matches the shape of the change net/http is making for the same constant and keeps IPAddr resolvable, so anything downstream that relied on require "openssl" defining it is unaffected. Note this does not save the socket require on the next line, which is unconditional and unrelated. The saving is ipaddr.rb itself, 857 lines. Measured with socket already loaded, which is openssl's actual situation, on Ruby 4.0.6 (arm64-darwin), 15 runs: marginal cost of require "ipaddr" min 2.47 ms / median 2.95 ms, 1 file End-to-end require "openssl" moves from a min of 26.42 ms to 24.88 ms, but that figure carries a lot of variance from loading openssl.so, so the marginal number above is the honest one. Small in absolute terms, but openssl is loaded in a large share of Ruby processes, and the change is one line with no behavior difference. The rescue clause still resolves correctly: IPAddr.new triggers the autoload before it can raise, so IPAddr::InvalidAddressError is defined by the time the rescue is evaluated. Test suite: 630 tests, 0 failures, unchanged. The existing coverage already exercises the IPAddr path directly, asserting verify_certificate_identity against a cert with an IP:127.0.0.1 SAN. The three added tests cover that requiring openssl does not load ipaddr, that IPAddr still resolves afterwards, and that referencing it pulls the library in. The first fails against the previous code. Signed-off-by: Tim Smith <tsmith84@proton.me>
1 parent baa28f5 commit 26a0f55

2 files changed

Lines changed: 29 additions & 1 deletion

File tree

lib/openssl/ssl.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515
if defined?(OpenSSL::SSL)
1616

1717
require "io/nonblock"
18-
require "ipaddr"
18+
# Only OpenSSL::SSL.verify_certificate_identity uses IPAddr, and only for
19+
# certificates carrying an iPAddress SAN. Autoloading keeps ipaddr (and the
20+
# socket library it pulls in) off the require path for everyone else, while
21+
# leaving the IPAddr constant resolvable as before.
22+
autoload :IPAddr, "ipaddr"
1923
require "socket"
2024

2125
module OpenSSL

test/openssl/test_require.rb

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# frozen_string_literal: true
2+
require_relative "utils"
3+
4+
class OpenSSL::TestRequire < OpenSSL::TestCase
5+
IPADDR_LOADED = '$LOADED_FEATURES.any? { |f| File.basename(f) == "ipaddr.rb" }'
6+
7+
def subprocess(script)
8+
lib = File.expand_path("../../lib", __dir__)
9+
IO.popen([RbConfig.ruby, "-I", lib, "-e", script], &:read)
10+
end
11+
12+
def test_requiring_openssl_does_not_load_ipaddr
13+
assert_equal "false", subprocess("require 'openssl'; print #{IPADDR_LOADED}")
14+
end
15+
16+
def test_ipaddr_is_still_reachable_after_requiring_openssl
17+
assert_equal "127.0.0.1", subprocess("require 'openssl'; print IPAddr.new('127.0.0.1').to_s")
18+
end
19+
20+
def test_referencing_ipaddr_loads_it
21+
script = "require 'openssl'; IPAddr.new('127.0.0.1'); print #{IPADDR_LOADED}"
22+
assert_equal "true", subprocess(script)
23+
end
24+
end

0 commit comments

Comments
 (0)