Bugfix/Wrong domain punycode extraction (#165)

* Fixed wrong domain punycode extraction in DNS validation layer * Restored Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, tests * Updated gem codebase/tests * Updated gem version
truemail-rb · Jul 10, 2021 · 8f787bb · 8f787bb
1 parent f63b1f9
commit 8f787bb
Show file tree

Hide file tree

Showing 13 changed files with 120 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,17 @@
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [2.4.6] - 2021.07.10
+
+### Fixed
+
+- Wrong domain punycode extraction in DNS validation layer
+
+### Updated
+
+- Updated gem codebase, restored `Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL`
+- Updated tests
+
 ## [2.4.5] - 2021.07.09
 
 ### Removed

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    truemail (2.4.5)
+    truemail (2.4.6)
       simpleidn (~> 0.2.1)
 
 GEM

diff --git a/lib/truemail/core.rb b/lib/truemail/core.rb
@@ -22,6 +22,7 @@ module RegexConstant
     REGEX_DOMAIN = /[\p{L}0-9]+([\-.]{1}[\p{L}0-9]+)*\.\p{L}{2,63}/i.freeze
     REGEX_EMAIL_PATTERN = /(?=\A.{6,255}\z)(\A([\p{L}0-9]+[\W\w]*)@(#{REGEX_DOMAIN})\z)/.freeze
     REGEX_DOMAIN_PATTERN = /(?=\A.{4,255}\z)(\A#{REGEX_DOMAIN}\z)/.freeze
+    REGEX_DOMAIN_FROM_EMAIL = /\A.+@(.+)\z/.freeze
     REGEX_SMTP_ERROR_BODY_PATTERN = /(?=.*550)(?=.*(user|account|customer|mailbox)).*/i.freeze
     REGEX_IP_ADDRESS = /((1\d|[1-9]|2[0-4])?\d|25[0-5])(\.\g<1>){3}/.freeze
     REGEX_IP_ADDRESS_PATTERN = /\A#{REGEX_IP_ADDRESS}\z/.freeze

diff --git a/lib/truemail/validate/domain_list_match.rb b/lib/truemail/validate/domain_list_match.rb
@@ -15,7 +15,7 @@ def run
       private
 
       def email_domain
-        @email_domain ||= result.email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3]
+        @email_domain ||= result.email[Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, 1]
       end
 
       def whitelisted_domain?

diff --git a/lib/truemail/validate/mx.rb b/lib/truemail/validate/mx.rb
@@ -53,8 +53,8 @@ def mail_servers_found?
 
       def domain
         @domain ||= begin
-          result.domain = result.email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3]
-          result.punycode_email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3]
+          result.domain = result.email[Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, 1]
+          result.punycode_email[Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, 1]
         end
       end
 

diff --git a/lib/truemail/validate/smtp/request.rb b/lib/truemail/validate/smtp/request.rb
@@ -50,7 +50,7 @@ class Configuration
           REQUEST_PARAMS = %i[connection_timeout response_timeout verifier_domain verifier_email].freeze
 
           def initialize(configuration)
-            REQUEST_PARAMS.each do |attribute|
+            Truemail::Validate::Smtp::Request::Configuration::REQUEST_PARAMS.each do |attribute|
               self.class.class_eval { attr_reader attribute }
               instance_variable_set(:"@#{attribute}", configuration.public_send(attribute))
             end

diff --git a/lib/truemail/version.rb b/lib/truemail/version.rb
@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Truemail
-  VERSION = '2.4.5'
+  VERSION = '2.4.6'
 end
diff --git a/spec/support/helpers/context_helper.rb b/spec/support/helpers/context_helper.rb
@@ -2,10 +2,16 @@
 
 module Truemail
   module ContextHelper
+    ASCII_WORDS = %w[mañana ĉapelo dấu παράδειγμα 買@屋企].freeze
+
     def random_email
       faker.email
     end
 
+    def random_internationalized_email
+      "#{faker.username}@#{Truemail::ContextHelper::ASCII_WORDS.sample}.#{faker.domain_suffix}"
+    end
+
     def random_ip_address
       faker.ip_v4_address
     end
@@ -22,6 +28,14 @@ def rdns_lookup_host_address(host_address)
       host_address.gsub(/(\d+).(\d+).(\d+).(\d+)/, '\4.\3.\2.\1.in-addr.arpa')
     end
 
+    def domain_from_email(email)
+      email[Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, 1]
+    end
+
+    def email_punycode_domain(email)
+      Truemail::Dns::PunycodeRepresenter.call(email)[Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL, 1]
+    end
+
     private
 
     def faker

diff --git a/spec/support/helpers/context_helper_spec.rb b/spec/support/helpers/context_helper_spec.rb
@@ -1,6 +1,10 @@
 # frozen_string_literal: true
 
 RSpec.describe Truemail::ContextHelper, type: :helper do # rubocop:disable RSpec/FilePath
+  describe 'defined constants' do
+    specify { expect(described_class).to be_const_defined(:ASCII_WORDS) }
+  end
+
   describe '#random_email' do
     specify do
       expect(Faker::Internet).to receive(:email).and_call_original
@@ -36,4 +40,34 @@
       expect(rdns_lookup_host_address('10.20.30.40')).to eq('40.30.20.10.in-addr.arpa')
     end
   end
+
+  describe '#domain_from_email' do
+    let(:domain) { 'domain' }
+    let(:email) { "user@#{domain}" }
+
+    specify { expect(domain_from_email(email)).to eq(domain) }
+  end
+
+  describe '#email_punycode_domain' do
+    let(:domain) { 'mañana.cøm' }
+    let(:email) { "user@#{domain}" }
+
+    specify do
+      expect(Truemail::Dns::PunycodeRepresenter).to receive(:call).with(email).and_call_original
+      expect(email_punycode_domain(email)).to eq('xn--maana-pta.xn--cm-lka')
+    end
+  end
+
+  describe '#random_internationalized_email' do
+    let(:user) { 'user' }
+    let(:domain_zone) { 'com' }
+    let(:ascii_word) { 'mañana' }
+
+    specify do
+      stub_const("#{described_class}::ASCII_WORDS", [ascii_word])
+      expect(Faker::Internet).to receive(:username).and_return(user)
+      expect(Faker::Internet).to receive(:domain_suffix).and_return(domain_zone)
+      expect(random_internationalized_email).to eq("#{user}@#{ascii_word}.#{domain_zone}")
+    end
+  end
 end
diff --git a/spec/truemail/core_spec.rb b/spec/truemail/core_spec.rb
@@ -17,6 +17,7 @@
     specify { expect(described_class).to be_const_defined(:REGEX_DOMAIN) }
     specify { expect(described_class).to be_const_defined(:REGEX_EMAIL_PATTERN) }
     specify { expect(described_class).to be_const_defined(:REGEX_DOMAIN_PATTERN) }
+    specify { expect(described_class).to be_const_defined(:REGEX_DOMAIN_FROM_EMAIL) }
     specify { expect(described_class).to be_const_defined(:REGEX_SMTP_ERROR_BODY_PATTERN) }
     specify { expect(described_class).to be_const_defined(:REGEX_IP_ADDRESS) }
     specify { expect(described_class).to be_const_defined(:REGEX_IP_ADDRESS_PATTERN) }
@@ -139,6 +140,15 @@
     end
   end
 
+  describe 'Truemail::RegexConstant::REGEX_DOMAIN_FROM_EMAIL' do
+    subject(:regex_pattern) { described_class::REGEX_DOMAIN_FROM_EMAIL }
+
+    let(:email) { 'i@domain' }
+
+    specify { expect(regex_pattern.match?(email)).to be(true) }
+    specify { expect(email[regex_pattern, 1]).to eq('domain') }
+  end
+
   describe 'Truemail::RegexConstant::REGEX_SMTP_ERROR_BODY_PATTERN' do
     subject(:regex_pattern) { described_class::REGEX_SMTP_ERROR_BODY_PATTERN }
 

diff --git a/spec/truemail/log/serializer/validator_text_spec.rb b/spec/truemail/log/serializer/validator_text_spec.rb
@@ -49,7 +49,7 @@
 
             CONFIGURATION SETTINGS:
             whitelist validation: false
-            whitelisted domains: #{email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3]}
+            whitelisted domains: #{domain_from_email(email)}
             not rfc mx lookup flow: false
             smtp fail fast: false
             smtp safe check: false
@@ -110,7 +110,7 @@
 
             CONFIGURATION SETTINGS:
             whitelist validation: false
-            blacklisted domains: #{email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3]}
+            blacklisted domains: #{domain_from_email(email)}
             not rfc mx lookup flow: false
             smtp fail fast: false
             smtp safe check: false

diff --git a/spec/truemail/validate/domain_list_match_spec.rb b/spec/truemail/validate/domain_list_match_spec.rb
@@ -13,7 +13,7 @@
     subject(:domain_list_match_validator) { described_class.check(result_instance) }
 
     let(:email) { random_email }
-    let(:domain) { email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3] }
+    let(:domain) { domain_from_email(email) }
     let(:configuration_instance) { create_configuration }
     let(:result_instance) { Truemail::Validator::Result.new(email: email, configuration: configuration_instance) }
 

diff --git a/spec/truemail/validate/mx_spec.rb b/spec/truemail/validate/mx_spec.rb
@@ -32,10 +32,25 @@
 
     let(:mx_validator_instance) { described_class.new(result_instance) }
 
-    shared_examples 'calls email punycode representer' do
+    shared_examples 'calls email punycode representer, returns memoized result' do
       specify do
         expect(result_instance).to receive(:punycode_email).and_call_original
         mx_validator
+        expect(mx_validator_instance.send(:domain)).to eq(email_punycode_domain(email))
+      end
+    end
+
+    shared_context 'when internationalized email' do
+      context 'when internationalized email' do
+        let(:email) { random_internationalized_email }
+
+        specify do
+          expect { mx_validator }
+            .to change(result_instance, :domain)
+            .from(nil).to(domain_from_email(email))
+        end
+
+        include_examples 'calls email punycode representer, returns memoized result'
       end
     end
 
@@ -52,9 +67,7 @@
         let(:a_records) { ::Array.new(total_records) { [random_ip_address, a_record] } }
         let(:uniq_mail_servers_by_ip) { a_records.flatten.uniq }
         let(:mx_records_dns_mock) { mx_records.zip(a_records).to_h.transform_values { |value| { a: value } } }
-        let(:dns_mock_records) do
-          { email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3] => { mx: mx_records } }.merge(mx_records_dns_mock)
-        end
+        let(:dns_mock_records) { { email_punycode_domain(email) => { mx: mx_records } }.merge(mx_records_dns_mock) }
 
         before { dns_mock_server.assign_mocks(dns_mock_records) }
 
@@ -68,15 +81,17 @@
 
             expect { mx_validator }
               .to change(result_instance, :domain)
-              .from(nil).to(email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3])
+              .from(nil).to(domain_from_email(email))
               .and change(result_instance, :mail_servers)
               .from([]).to(uniq_mail_servers_by_ip)
               .and not_change(result_instance, :success)
           end
 
-          include_examples 'calls email punycode representer'
+          include_examples 'calls email punycode representer, returns memoized result'
 
           specify { expect(mx_validator).to be(true) }
+
+          include_context 'when internationalized email'
         end
       end
 
@@ -93,7 +108,7 @@
         let(:mx_records_dns_mock) { mx_records.zip(mx_a_records).to_h.transform_values { |value| { a: value } } }
         let(:dns_mock_records) do
           {
-            email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3] => { cname: cname_records.first }
+            email_punycode_domain(email) => { cname: cname_records.first }
           }.merge(cname_records_dns_mock).merge(ptr_records_dns_mock).merge(mx_records_dns_mock)
         end
 
@@ -107,15 +122,17 @@
 
             expect { mx_validator }
               .to change(result_instance, :domain)
-              .from(nil).to(email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3])
+              .from(nil).to(domain_from_email(email))
               .and change(result_instance, :mail_servers)
               .from([]).to(uniq_mail_servers_by_ip)
               .and not_change(result_instance, :success)
           end
 
-          include_examples 'calls email punycode representer'
+          include_examples 'calls email punycode representer, returns memoized result'
 
           specify { expect(mx_validator).to be(true) }
+
+          include_context 'when internationalized email'
         end
 
         context 'when mx records not found' do
@@ -127,21 +144,23 @@
             expect(mx_validator_instance).not_to receive(:host_from_a_record?)
             expect { mx_validator }
               .to change(result_instance, :domain)
-              .from(nil).to(email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3])
+              .from(nil).to(domain_from_email(email))
               .and change(result_instance, :mail_servers)
               .from([]).to([a_records.first]) # one cname record is equal to one a record
               .and not_change(result_instance, :success)
           end
 
-          include_examples 'calls email punycode representer'
+          include_examples 'calls email punycode representer, returns memoized result'
 
           specify { expect(mx_validator).to be(true) }
+
+          include_context 'when internationalized email'
         end
       end
 
       context 'when a record found' do
         let(:a_record) { random_ip_address }
-        let(:dns_mock_records) { { email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3] => { a: [a_record] } } }
+        let(:dns_mock_records) { { email_punycode_domain(email) => { a: [a_record] } } }
 
         before { dns_mock_server.assign_mocks(dns_mock_records) }
 
@@ -151,15 +170,17 @@
           expect(mx_validator_instance).to receive(:host_from_a_record?).and_call_original
           expect { mx_validator }
             .to change(result_instance, :domain)
-            .from(nil).to(email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3])
+            .from(nil).to(domain_from_email(email))
             .and change(result_instance, :mail_servers)
             .from([]).to([a_record])
             .and not_change(result_instance, :success)
         end
 
-        include_examples 'calls email punycode representer'
+        include_examples 'calls email punycode representer, returns memoized result'
 
         specify { expect(mx_validator).to be(true) }
+
+        include_context 'when internationalized email'
       end
     end
 
@@ -169,18 +190,18 @@
           mx_lookup_chain_expectations
           expect { mx_validator }
             .to change(result_instance, :domain)
-            .from(nil).to(email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3])
+            .from(nil).to(domain_from_email(email))
             .and not_change(result_instance, :mail_servers)
             .and change(result_instance, :success).from(true).to(false)
         end
 
-        include_examples 'calls email punycode representer'
+        include_examples 'calls email punycode representer, returns memoized result'
 
         specify { is_expected.to be(false) }
       end
 
       context 'when mx records found with null mx' do
-        let(:dns_mock_records) { { email[Truemail::RegexConstant::REGEX_EMAIL_PATTERN, 3] => { mx: %w[.:0] } } }
+        let(:dns_mock_records) { { email_punycode_domain(email) => { mx: %w[.:0] } } }
         let(:mx_lookup_chain_expectations) do
           expect(mx_validator_instance).to receive(:hosts_from_mx_records?).and_call_original
           expect(mx_validator_instance).not_to receive(:hosts_from_cname_records?)
@@ -194,6 +215,7 @@
         end
 
         include_examples 'validation fails'
+        include_context 'when internationalized email'
       end
 
       context 'when not RFC MX lookup flow enabled' do
@@ -215,6 +237,7 @@
         end
 
         include_examples 'validation fails'
+        include_context 'when internationalized email'
       end
 
       context 'when any of mx lookup methods fail' do
@@ -230,6 +253,7 @@
         end
 
         include_examples 'validation fails'
+        include_context 'when internationalized email'
       end
 
       context 'when regex fails' do