From 3be9449d555e6f66c90ac8cf81c305a6c06b1ed0 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Mon, 22 Jan 2024 11:40:25 +0000 Subject: [PATCH 1/9] Add specs for DNS timeout configuration --- spec/valid_email2_spec.rb | 54 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/spec/valid_email2_spec.rb b/spec/valid_email2_spec.rb index 501448f..8f5ceda 100644 --- a/spec/valid_email2_spec.rb +++ b/spec/valid_email2_spec.rb @@ -23,6 +23,15 @@ class TestUserStrictMX < TestModel validates :email, 'valid_email_2/email': { strict_mx: true } end +class TestUserMXDnsTimeout < TestModel + validates :email, 'valid_email_2/email': { mx: true, dns_timeout: 10 } +end + +class TestUserMXDnsFailingTimeout < TestModel + validates :email, 'valid_email_2/email': { mx: true, dns_timeout: 0.001 } +end + + class TestUserDisallowDisposable < TestModel validates :email, 'valid_email_2/email': { disposable: true } end @@ -292,6 +301,51 @@ def set_whitelist end end + describe "with mx validation and dns not hitting timeout" do + it "is valid if mx records are found" do + user = TestUserMXDnsTimeout.new(email: "foo@gmail.com") + expect(user.valid?).to be_truthy + end + + it "is valid if A records are found" do + user = TestUserMXDnsTimeout.new(email: "foo@ghs.google.com") + expect(user.valid?).to be_truthy + end + + it "is invalid if no mx records are found" do + user = TestUserMXDnsTimeout.new(email: "foo@subdomain.gmail.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if a null mx is found" do + user = TestUserMXDnsTimeout.new(email: "foo@gmail.de") + expect(user.valid?).to be_falsey + end + end + + describe "with mx validation and dns hitting timeout" do + it "is never valid even if mx records exist" do + user = TestUserMXDnsFailingTimeout.new(email: "foo@gmail.com") + expect(user.valid?).to be_falsey + end + + it "is never valid even A records exist" do + user = TestUserMXDnsFailingTimeout.new(email: "foo@ghs.google.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if no mx records exist" do + user = TestUserMXDnsFailingTimeout.new(email: "foo@subdomain.gmail.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if a null mx exists" do + user = TestUserMXDnsFailingTimeout.new(email: "foo@gmail.de") + expect(user.valid?).to be_falsey + end + end + + describe "with dotted validation" do it "is valid when address does not contain dots" do user = TestUserDotted.new(email: "johndoe@gmail.com") From 265d4f07fe013f5726de48ffc2484f08a565a337 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Mon, 22 Jan 2024 11:42:02 +0000 Subject: [PATCH 2/9] Add option for overriding `Resolv::DNS` nameservers --- README.md | 14 +++++--- lib/valid_email2/address.rb | 11 +++++-- lib/valid_email2/email_validator.rb | 4 +-- spec/valid_email2_spec.rb | 50 +++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index bae5390..639f6e8 100644 --- a/README.md +++ b/README.md @@ -51,12 +51,18 @@ To validate strictly that the domain has an MX record: ```ruby validates :email, 'valid_email_2/email': { strict_mx: true } ``` -`strict_mx` and `mx` both default to a 5 second timeout for DNS lookups. To +`strict_mx` and `mx` both default to a 5 second timeout for DNS lookups. To override this timeout, specify a `dns_timeout` option: ```ruby validates :email, 'valid_email_2/email': { strict_mx: true, dns_timeout: 10 } ``` +Any checks that require DNS resolution will use the default `Resolv::DNS` nameservers for DNS lookups. To +override these, specify a `dns_nameserver` option: +```ruby +validates :email, 'valid_email_2/email': { mx: true, dns_nameserver: ['8.8.8.8', '8.8.4.4'] } +``` + To validate that the domain is not a disposable email (checks domain and MX server): ```ruby validates :email, 'valid_email_2/email': { disposable: true } @@ -125,7 +131,7 @@ address.subaddressed? => false ### Test environment If you are validating `mx` then your specs will fail without an internet connection. -It is a good idea to stub out that validation in your test environment. +It is a good idea to stub out that validation in your test environment. Do so by adding this in your `spec_helper`: ```ruby config.before(:each) do @@ -146,7 +152,7 @@ This gem is tested against currently supported Ruby and Rails versions. For an u In version v3.0.0 I decided to move __and__ rename the config files from the vendor directory to the config directory. That means: -`vendor/blacklist.yml` -> `config/blacklisted_email_domains.yml` +`vendor/blacklist.yml` -> `config/blacklisted_email_domains.yml` `vendor/whitelist.yml` -> `config/whitelisted_email_domains.yml` The `disposable` validation has been improved with a `mx` check. Apply the @@ -155,7 +161,7 @@ down or if they do not work without an internet connection. ## Upgrading to v2.0.0 -In version 1.0 of valid_email2 we only defined the `email` validator. +In version 1.0 of valid_email2 we only defined the `email` validator. But since other gems also define a `email` validator this can cause some unintended behaviours and emails that shouldn't be valid are regarded valid because the wrong validator is used by rails. diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index ef692a1..f87ec77 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -20,11 +20,16 @@ def self.prohibited_domain_characters_regex=(val) @prohibited_domain_characters_regex = val end - def initialize(address, dns_timeout = 5) + def initialize(address, dns_timeout = 5, dns_nameserver = nil) @parse_error = false @raw_address = address @dns_timeout = dns_timeout + if dns_nameserver + @dns_config_info = Resolv::DNS::Config.default_config_hash + @dns_config_info[:nameserver] = dns_nameserver + end + begin @address = Mail::Address.new(address) rescue Mail::Field::ParseError @@ -134,7 +139,7 @@ def address_contain_emoticons?(email) end def mx_servers - @mx_servers ||= Resolv::DNS.open do |dns| + @mx_servers ||= Resolv::DNS.open(@dns_config_info) do |dns| dns.timeouts = @dns_timeout dns.getresources(address.domain, Resolv::DNS::Resource::IN::MX) end @@ -145,7 +150,7 @@ def null_mx? end def mx_or_a_servers - @mx_or_a_servers ||= Resolv::DNS.open do |dns| + @mx_or_a_servers ||= Resolv::DNS.open(@dns_config_info) do |dns| dns.timeouts = @dns_timeout (mx_servers.any? && mx_servers) || dns.getresources(address.domain, Resolv::DNS::Resource::IN::A) diff --git a/lib/valid_email2/email_validator.rb b/lib/valid_email2/email_validator.rb index 799d3cb..0016c71 100644 --- a/lib/valid_email2/email_validator.rb +++ b/lib/valid_email2/email_validator.rb @@ -5,14 +5,14 @@ module ValidEmail2 class EmailValidator < ActiveModel::EachValidator def default_options - { disposable: false, mx: false, strict_mx: false, disallow_subaddressing: false, multiple: false, dns_timeout: 5 } + { disposable: false, mx: false, strict_mx: false, disallow_subaddressing: false, multiple: false, dns_timeout: 5, dns_nameserver: nil } end def validate_each(record, attribute, value) return unless value.present? options = default_options.merge(self.options) - addresses = sanitized_values(value).map { |v| ValidEmail2::Address.new(v, options[:dns_timeout]) } + addresses = sanitized_values(value).map { |v| ValidEmail2::Address.new(v, options[:dns_timeout], options[:dns_nameserver]) } error(record, attribute) && return unless addresses.all?(&:valid?) diff --git a/spec/valid_email2_spec.rb b/spec/valid_email2_spec.rb index 8f5ceda..995f3d2 100644 --- a/spec/valid_email2_spec.rb +++ b/spec/valid_email2_spec.rb @@ -31,6 +31,13 @@ class TestUserMXDnsFailingTimeout < TestModel validates :email, 'valid_email_2/email': { mx: true, dns_timeout: 0.001 } end +class TestUserMXDnsNameserver < TestModel + validates :email, 'valid_email_2/email': { mx: true, dns_nameserver: ['8.8.8.8', '8.8.4.4'] } +end + +class TestUserMXDnsFailingNameserver < TestModel + validates :email, 'valid_email_2/email': { mx: true, dns_timeout: 0.1, dns_nameserver: '1.0.0.0' } +end class TestUserDisallowDisposable < TestModel validates :email, 'valid_email_2/email': { disposable: true } @@ -345,6 +352,49 @@ def set_whitelist end end + describe "with mx validation and dns nameserver" do + it "is valid if mx records are found" do + user = TestUserMXDnsNameserver.new(email: "foo@gmail.com") + expect(user.valid?).to be_truthy + end + + it "is valid if A records are found" do + user = TestUserMXDnsNameserver.new(email: "foo@ghs.google.com") + expect(user.valid?).to be_truthy + end + + it "is invalid if no mx records are found" do + user = TestUserMXDnsNameserver.new(email: "foo@subdomain.gmail.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if a null mx is found" do + user = TestUserMXDnsNameserver.new(email: "foo@gmail.de") + expect(user.valid?).to be_falsey + end + end + + describe "with mx validation and failing dns nameserver" do + it "is never valid even if mx records exist" do + user = TestUserMXDnsFailingNameserver.new(email: "foo@gmail.com") + expect(user.valid?).to be_falsey + end + + it "is never valid even A records exist" do + user = TestUserMXDnsFailingNameserver.new(email: "foo@ghs.google.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if no mx records exist" do + user = TestUserMXDnsFailingNameserver.new(email: "foo@subdomain.gmail.com") + expect(user.valid?).to be_falsey + end + + it "is invalid if a null mx exists" do + user = TestUserMXDnsFailingNameserver.new(email: "foo@gmail.de") + expect(user.valid?).to be_falsey + end + end describe "with dotted validation" do it "is valid when address does not contain dots" do From 61a99274dc0832ce8a6599f985f21f44699687f3 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:42:20 +0000 Subject: [PATCH 3/9] Update lib/valid_email2/address.rb Co-authored-by: Micke Lisinge --- lib/valid_email2/address.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index f87ec77..672ce26 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -25,10 +25,8 @@ def initialize(address, dns_timeout = 5, dns_nameserver = nil) @raw_address = address @dns_timeout = dns_timeout - if dns_nameserver - @dns_config_info = Resolv::DNS::Config.default_config_hash - @dns_config_info[:nameserver] = dns_nameserver - end + @resolv_config = Resolv::DNS::Config.default_config_hash + @resolv_config[:nameserver] = dns_nameserver if dns_nameserver begin @address = Mail::Address.new(address) From 47f17b6476adeedc8eee14e2c9c8eb1d0461def4 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:42:25 +0000 Subject: [PATCH 4/9] Update lib/valid_email2/address.rb Co-authored-by: Micke Lisinge --- lib/valid_email2/address.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index 672ce26..1396fb2 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -137,7 +137,7 @@ def address_contain_emoticons?(email) end def mx_servers - @mx_servers ||= Resolv::DNS.open(@dns_config_info) do |dns| + @mx_servers ||= Resolv::DNS.open(@resolv_config) do |dns| dns.timeouts = @dns_timeout dns.getresources(address.domain, Resolv::DNS::Resource::IN::MX) end From c410e556d8dac2f1ecd3f5e7ed2c57d9016b4e8a Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:46:03 +0000 Subject: [PATCH 5/9] Add markdown linebreaks in README --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 639f6e8..9ff7a16 100644 --- a/README.md +++ b/README.md @@ -51,14 +51,14 @@ To validate strictly that the domain has an MX record: ```ruby validates :email, 'valid_email_2/email': { strict_mx: true } ``` -`strict_mx` and `mx` both default to a 5 second timeout for DNS lookups. To -override this timeout, specify a `dns_timeout` option: +`strict_mx` and `mx` both default to a 5 second timeout for DNS lookups. +To override this timeout, specify a `dns_timeout` option: ```ruby validates :email, 'valid_email_2/email': { strict_mx: true, dns_timeout: 10 } ``` -Any checks that require DNS resolution will use the default `Resolv::DNS` nameservers for DNS lookups. To -override these, specify a `dns_nameserver` option: +Any checks that require DNS resolution will use the default `Resolv::DNS` nameservers for DNS lookups. +To override these, specify a `dns_nameserver` option: ```ruby validates :email, 'valid_email_2/email': { mx: true, dns_nameserver: ['8.8.8.8', '8.8.4.4'] } ``` From e575519c72d6e3b331f773edb041c78d77f69a90 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:47:41 +0000 Subject: [PATCH 6/9] Replace another markdown linebreak --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9ff7a16..325a1b7 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ address.subaddressed? => false ### Test environment If you are validating `mx` then your specs will fail without an internet connection. -It is a good idea to stub out that validation in your test environment. +It is a good idea to stub out that validation in your test environment. Do so by adding this in your `spec_helper`: ```ruby config.before(:each) do From 8cedb6754cbd69f49a14900c6c3ed827c4558be5 Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:48:08 +0000 Subject: [PATCH 7/9] Update lib/valid_email2/address.rb Co-authored-by: Micke Lisinge --- lib/valid_email2/address.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/valid_email2/address.rb b/lib/valid_email2/address.rb index 1396fb2..31f58e1 100644 --- a/lib/valid_email2/address.rb +++ b/lib/valid_email2/address.rb @@ -148,7 +148,7 @@ def null_mx? end def mx_or_a_servers - @mx_or_a_servers ||= Resolv::DNS.open(@dns_config_info) do |dns| + @mx_or_a_servers ||= Resolv::DNS.open(@resolv_config) do |dns| dns.timeouts = @dns_timeout (mx_servers.any? && mx_servers) || dns.getresources(address.domain, Resolv::DNS::Resource::IN::A) From 8177b4e1351e1b1f0e6f58b67982bda49307decd Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Wed, 31 Jan 2024 22:49:39 +0000 Subject: [PATCH 8/9] Replace other linebreaks which were accidentally removed --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 325a1b7..42a62b7 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ This gem is tested against currently supported Ruby and Rails versions. For an u In version v3.0.0 I decided to move __and__ rename the config files from the vendor directory to the config directory. That means: -`vendor/blacklist.yml` -> `config/blacklisted_email_domains.yml` +`vendor/blacklist.yml` -> `config/blacklisted_email_domains.yml` `vendor/whitelist.yml` -> `config/whitelisted_email_domains.yml` The `disposable` validation has been improved with a `mx` check. Apply the @@ -161,7 +161,7 @@ down or if they do not work without an internet connection. ## Upgrading to v2.0.0 -In version 1.0 of valid_email2 we only defined the `email` validator. +In version 1.0 of valid_email2 we only defined the `email` validator. But since other gems also define a `email` validator this can cause some unintended behaviours and emails that shouldn't be valid are regarded valid because the wrong validator is used by rails. From 69e2a4f7e581a16ded29070be2929ae0c8bffade Mon Sep 17 00:00:00 2001 From: Tim Krins Date: Thu, 1 Feb 2024 12:32:23 +0000 Subject: [PATCH 9/9] Set DNS timeout to Float::MIN for failing DNS timeout test --- spec/valid_email2_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/valid_email2_spec.rb b/spec/valid_email2_spec.rb index 995f3d2..97eb659 100644 --- a/spec/valid_email2_spec.rb +++ b/spec/valid_email2_spec.rb @@ -28,7 +28,7 @@ class TestUserMXDnsTimeout < TestModel end class TestUserMXDnsFailingTimeout < TestModel - validates :email, 'valid_email_2/email': { mx: true, dns_timeout: 0.001 } + validates :email, 'valid_email_2/email': { mx: true, dns_timeout: Float::MIN } end class TestUserMXDnsNameserver < TestModel