From 0c8aa89aead3a7b60d81063960a697d14018c374 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraj=20Juri=C4=8Di=C4=87?= Date: Wed, 19 Feb 2025 18:44:21 +0100 Subject: [PATCH 1/6] Add benchmark spec for disposable_mx_server? --- spec/benchmark_spec.rb | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/spec/benchmark_spec.rb b/spec/benchmark_spec.rb index 414756da..5e6f5613 100644 --- a/spec/benchmark_spec.rb +++ b/spec/benchmark_spec.rb @@ -5,7 +5,7 @@ describe "Performance testing" do let(:disposable_domain) { ValidEmail2.disposable_emails.first } - it "has acceptable lookup performance" do + it "disposable_domain? has acceptable lookup performance" do address = ValidEmail2::Address.new("test@example.com") # preload list and check size @@ -13,6 +13,17 @@ expect(ValidEmail2.disposable_emails.count).to be > 30000 # check lookup timing - expect { address.disposable_domain? }.to perform_under(0.0001).sample(10).times + expect { address.disposable_domain? }.to perform_under(0.0001).sec.sample(10).times + end + + it "disposable_mx_server? has acceptable lookup performance" do + address = ValidEmail2::Address.new("test@gmail.com") + + # preload list and check size + expect(ValidEmail2.disposable_emails).to be_a(Set) + expect(ValidEmail2.disposable_emails.count).to be > 30000 + + # check lookup timing + expect { address.disposable_mx_server? }.to perform_under(0.0001).sec.warmup(1).times.sample(10).times end end From 8f26a0156cf2c46de0e1d10aab8ab29e5bfcd0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraj=20Juri=C4=8Di=C4=87?= Date: Wed, 19 Feb 2025 18:46:39 +0100 Subject: [PATCH 2/6] Add unit tests for disposable_mx_server? --- spec/address_spec.rb | 45 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/spec/address_spec.rb b/spec/address_spec.rb index 644a5239..3fa53f8b 100644 --- a/spec/address_spec.rb +++ b/spec/address_spec.rb @@ -111,7 +111,7 @@ let(:email_instance) { described_class.new(email_address, dns_instance) } let(:ttl) { 1_000 } let(:mock_resolv_dns) { instance_double(Resolv::DNS) } - let(:mock_mx_records) { [double("MX", exchange: "mx.ymail.com", preference: 10, ttl: ttl)] } + let(:mock_mx_records) { [double("MX", exchange: "mx.ymail.com", preference: 10, ttl:)] } before do allow(email_instance).to receive(:null_mx?).and_return(false) @@ -119,6 +119,47 @@ allow(mock_resolv_dns).to receive(:timeouts=) end + describe "#disposable_mx_server?" do + let(:disposable_email_address) { "example@10minutemail.com" } + let(:disposable_mx_server) { ValidEmail2.disposable_emails.select { |domain| domain.count(".") == 1 }.sample } + let(:disposable_email_instance) { described_class.new(disposable_email_address, dns_instance) } + let(:mock_disposable_mx_records) { [double("MX", exchange: "mx.#{disposable_mx_server}", preference: 10, ttl:)] } + + before do + allow(mock_resolv_dns).to receive(:getresources) + .with(disposable_email_instance.address.domain, Resolv::DNS::Resource::IN::MX) + .and_return(mock_disposable_mx_records) + + allow(mock_resolv_dns).to receive(:getresources) + .with(email_instance.address.domain, Resolv::DNS::Resource::IN::MX) + .and_return(mock_mx_records) + end + + it "is false if the MX server is not in the disposable_emails list" do + expect(email_instance).not_to be_disposable_mx_server + end + + it "is true if the MX server is in the disposable_emails list" do + expect(disposable_email_instance).to be_disposable_mx_server + end + + it "is false and then true when the MX record changes from non-disposable to disposable" do + allow(mock_resolv_dns).to receive(:getresources) + .with(disposable_email_instance.address.domain, Resolv::DNS::Resource::IN::MX) + .and_return(mock_mx_records) # non-disposable MX records + + expect(disposable_email_instance).not_to be_disposable_mx_server + + ValidEmail2::Dns.clear_cache + + allow(mock_resolv_dns).to receive(:getresources) + .with(disposable_email_instance.address.domain, Resolv::DNS::Resource::IN::MX) + .and_return(mock_disposable_mx_records) # disposable MX records + + expect(disposable_email_instance).to be_disposable_mx_server + end + end + describe "#valid_strict_mx?" do let(:cached_at) { Time.now } let(:mock_cache_data) { { [email_instance.address.domain, Resolv::DNS::Resource::IN::MX] => ValidEmail2::Dns::CacheEntry.new(mock_mx_records, cached_at, ttl) } } @@ -251,7 +292,7 @@ describe "#valid_mx?" do let(:cached_at) { Time.now } let(:mock_cache_data) { { [email_instance.address.domain, Resolv::DNS::Resource::IN::MX] => ValidEmail2::Dns::CacheEntry.new(mock_a_records, cached_at, ttl) } } - let(:mock_a_records) { [double("A", address: "192.168.1.1", ttl: ttl)] } + let(:mock_a_records) { [double("A", address: "192.168.1.1", ttl:)] } before do allow(email_instance).to receive(:mx_servers).and_return(mock_mx_records) From 652740f6638cac6308fab221a27ebee28c92cbf2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juraj=20Juri=C4=8Di=C4=87?= Date: Wed, 19 Feb 2025 18:47:42 +0100 Subject: [PATCH 3/6] Cache results of mx_server_is_in? to speed up disposable_mx_server? lookup for the same domain --- lib/valid_email2/address.rb | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index ca1710db..6d1b649f 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -3,6 +3,7 @@ require "valid_email2" require "mail" require "valid_email2/dns" +require "digest" module ValidEmail2 class Address @@ -11,6 +12,7 @@ class Address PROHIBITED_DOMAIN_CHARACTERS_REGEX = /[+!_\/\s'#`]/ DEFAULT_RECIPIENT_DELIMITER = '+' DOT_DELIMITER = '.' + @cache_mx_server_is_in = {} def self.prohibited_domain_characters_regex @prohibited_domain_characters_regex ||= PROHIBITED_DOMAIN_CHARACTERS_REGEX @@ -130,15 +132,24 @@ def domain_is_in?(domain_list) end def mx_server_is_in?(domain_list) - @dns.mx_servers(address.domain).any? { |mx_server| + mx_servers = @dns.mx_servers(address.domain) + + cache_key = generate_mx_cache_key(domain_list, mx_servers) + + return self.class.cache_mx_server_is_in[cache_key] if !cache_key.nil? && self.class.cache_mx_server_is_in.key?(cache_key) + + result = mx_servers.any? do |mx_server| return false unless mx_server.respond_to?(:exchange) mx_server = mx_server.exchange.to_s - domain_list.any? { |domain| + domain_list.any? do |domain| mx_server.end_with?(domain) && mx_server =~ /\A(?:.+\.)*?#{domain}\z/ - } - } + end + end + + self.class.cache_mx_server_is_in[cache_key] = result unless cache_key.nil? + result end def address_contain_multibyte_characters? @@ -153,5 +164,18 @@ def null_mx? mx_servers = @dns.mx_servers(address.domain) mx_servers.length == 1 && mx_servers.first.preference == 0 && mx_servers.first.exchange.length == 0 end + + def generate_mx_cache_key(domain_list, mx_servers) + return nil if mx_servers.length == 0 || domain_list.length == 0 + + mx_servers_str = mx_servers.map(&:exchange).map(&:to_s).sort.join + return address.domain if mx_servers_str == "" + + "#{domain_list.object_id}_#{domain_list.length}_#{mx_servers_str.downcase}" + end + + def self.cache_mx_server_is_in + @cache_mx_server_is_in + end end end From 42984d054425d1cdf5c80c1d66e296172b11089c Mon Sep 17 00:00:00 2001 From: Ivan Takarlikov Date: Fri, 21 Feb 2025 12:41:47 -0300 Subject: [PATCH 4/6] Refactor to put all cache into Dns class --- lib/valid_email2/address.rb | 32 +------------------------------- lib/valid_email2/dns.rb | 31 ++++++++++++++++++++++++++++++- 2 files changed, 31 insertions(+), 32 deletions(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index 6d1b649f..bcc5c101 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -132,24 +132,7 @@ def domain_is_in?(domain_list) end def mx_server_is_in?(domain_list) - mx_servers = @dns.mx_servers(address.domain) - - cache_key = generate_mx_cache_key(domain_list, mx_servers) - - return self.class.cache_mx_server_is_in[cache_key] if !cache_key.nil? && self.class.cache_mx_server_is_in.key?(cache_key) - - result = mx_servers.any? do |mx_server| - return false unless mx_server.respond_to?(:exchange) - - mx_server = mx_server.exchange.to_s - - domain_list.any? do |domain| - mx_server.end_with?(domain) && mx_server =~ /\A(?:.+\.)*?#{domain}\z/ - end - end - - self.class.cache_mx_server_is_in[cache_key] = result unless cache_key.nil? - result + @dns.mx_servers_disposable?(address.domain, domain_list) end def address_contain_multibyte_characters? @@ -164,18 +147,5 @@ def null_mx? mx_servers = @dns.mx_servers(address.domain) mx_servers.length == 1 && mx_servers.first.preference == 0 && mx_servers.first.exchange.length == 0 end - - def generate_mx_cache_key(domain_list, mx_servers) - return nil if mx_servers.length == 0 || domain_list.length == 0 - - mx_servers_str = mx_servers.map(&:exchange).map(&:to_s).sort.join - return address.domain if mx_servers_str == "" - - "#{domain_list.object_id}_#{domain_list.length}_#{mx_servers_str.downcase}" - end - - def self.cache_mx_server_is_in - @cache_mx_server_is_in - end end end diff --git a/lib/valid_email2/dns.rb b/lib/valid_email2/dns.rb index 196e7c24..9502e443 100644 --- a/lib/valid_email2/dns.rb +++ b/lib/valid_email2/dns.rb @@ -4,7 +4,7 @@ module ValidEmail2 class Dns MAX_CACHE_SIZE = 1_000 CACHE = {} - + MX_SERVERS_CACHE = {} CacheEntry = Struct.new(:records, :cached_at, :ttl) def self.prune_cache @@ -26,6 +26,26 @@ def mx_servers(domain) fetch(domain, Resolv::DNS::Resource::IN::MX) end + def mx_servers_disposable?(domain, domain_list) + servers = mx_servers(domain) + cache_key = generate_mx_cache_key(domain, domain_list, servers) + return MX_SERVERS_CACHE[cache_key] if !cache_key.nil? && MX_SERVERS_CACHE.key?(cache_key) + + result = servers.any? do |mx_server| + return false unless mx_server.respond_to?(:exchange) + + mx_server = mx_server.exchange.to_s + + domain_list.any? do |disposable_domain| + mx_server.end_with?(disposable_domain) && mx_server =~ /\A(?:.+\.)*?#{disposable_domain}\z/ + end + end + + MX_SERVERS_CACHE[cache_key] = result unless cache_key.nil? + + result + end + def a_servers(domain) fetch(domain, Resolv::DNS::Resource::IN::A) end @@ -67,5 +87,14 @@ def resolv_config config[:nameserver] = @dns_nameserver if @dns_nameserver config end + + def generate_mx_cache_key(domain, domain_list, mx_servers) + return nil if mx_servers.empty? || domain_list.empty? + + mx_servers_str = mx_servers.map(&:exchange).map(&:to_s).sort.join + return domain if mx_servers_str == "" + + "#{domain_list.object_id}_#{domain_list.length}_#{mx_servers_str.downcase}" + end end end From 13749756b2b5549316a02690dc69b4978ac45f64 Mon Sep 17 00:00:00 2001 From: Ivan Takarlikov Date: Fri, 21 Feb 2025 12:42:43 -0300 Subject: [PATCH 5/6] Cleanup --- lib/valid_email2/address.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index bcc5c101..6fe59d7d 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -3,7 +3,6 @@ require "valid_email2" require "mail" require "valid_email2/dns" -require "digest" module ValidEmail2 class Address @@ -12,7 +11,6 @@ class Address PROHIBITED_DOMAIN_CHARACTERS_REGEX = /[+!_\/\s'#`]/ DEFAULT_RECIPIENT_DELIMITER = '+' DOT_DELIMITER = '.' - @cache_mx_server_is_in = {} def self.prohibited_domain_characters_regex @prohibited_domain_characters_regex ||= PROHIBITED_DOMAIN_CHARACTERS_REGEX From 74017a0a411f19eb2dd61a7761c7887731d9557c Mon Sep 17 00:00:00 2001 From: JJ Date: Fri, 21 Feb 2025 19:31:33 +0100 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Sasha Yelkhovenka --- lib/valid_email2/dns.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/valid_email2/dns.rb b/lib/valid_email2/dns.rb index 9502e443..40399e62 100644 --- a/lib/valid_email2/dns.rb +++ b/lib/valid_email2/dns.rb @@ -37,7 +37,7 @@ def mx_servers_disposable?(domain, domain_list) mx_server = mx_server.exchange.to_s domain_list.any? do |disposable_domain| - mx_server.end_with?(disposable_domain) && mx_server =~ /\A(?:.+\.)*?#{disposable_domain}\z/ + mx_server.end_with?(disposable_domain) && mx_server.match?(/\A(?:.+\.)*?#{disposable_domain}\z/) end end @@ -89,10 +89,10 @@ def resolv_config end def generate_mx_cache_key(domain, domain_list, mx_servers) - return nil if mx_servers.empty? || domain_list.empty? + return if mx_servers.empty? || domain_list.empty? mx_servers_str = mx_servers.map(&:exchange).map(&:to_s).sort.join - return domain if mx_servers_str == "" + return domain if mx_servers_str.empty? "#{domain_list.object_id}_#{domain_list.length}_#{mx_servers_str.downcase}" end