Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Net::SMTP#start signature differs depending on Ruby's version #188

Closed
4 tasks done
evserykh opened this issue Dec 3, 2021 · 3 comments · Fixed by #189
Closed
4 tasks done

[BUG] Net::SMTP#start signature differs depending on Ruby's version #188

evserykh opened this issue Dec 3, 2021 · 3 comments · Fixed by #189
Assignees
Labels
bug Something isn't working

Comments

@evserykh
Copy link

evserykh commented Dec 3, 2021

New bug checklist

Bug description

I'm using the following Ruby version ruby 2.5.5p157 (2019-03-15 revision 67260) [x86_64-linux]

Here is the code I use:

require 'net/smtp'
require 'truemail'

session = Net::SMTP.new('gmail-smtp-in.l.google.com', 25)
session.start('example.com') do |smtp|
  smtp.mailfrom 'test@example.com'
  p smtp.rcptto 'foobar@gmail.com'
end

Truemail.configure do |config|
  config.verifier_email = 'test@example.com'
  config.smtp_fail_fast = true
end

jj JSON.parse(Truemail.validate('foobar@gmail.com').as_json)

and here is the output:

#<Net::SMTP::Response:0x000055af65823998 @status="250", @string="250 2.1.5 OK n1si4121035lji.231 - gsmtp\n">
{
  "date": "2021-12-03 09:56:05 +0000",
  "email": "foobar@gmail.com",
  "validation_type": "smtp",
  "success": false,
  "errors": {
    "smtp": "smtp error"
  },
  "smtp_debug": [
    {
      "mail_host": "64.233.164.26",
      "port_opened": true,
      "connection": false,
      "errors": {
        "connection": "SMTP-AUTH requested but missing secret phrase"
      }
    }
  ],
  "configuration": {
    "validation_type_by_domain": null,
    "whitelist_validation": false,
    "whitelisted_domains": null,
    "blacklisted_domains": null,
    "blacklisted_mx_ip_addresses": null,
    "dns": null,
    "not_rfc_mx_lookup_flow": false,
    "smtp_fail_fast": true,
    "smtp_safe_check": false,
    "email_pattern": "default gem value",
    "smtp_error_body_pattern": "default gem value"
  }
}

After some research I figured out before Ruby 3 the Net::SMTP#start method's definition looks like this:

def start(helo = 'localhost',
          user = nil, secret = nil, authtype = nil)

and starting from Ruby 3 the method's definition has been changed to this:

def start(*args, helo: nil,
          user: nil, secret: nil, password: nil, authtype: nil, tls_verify: true, tls_hostname: nil)

This monkey patch allows to fix my case:

module Truemail
  module Validate
    class Smtp
      class Request
        def start_session_wrapper(&block)
          ruby3 = ::RUBY_VERSION[/\A3\..+\z/]
          if ruby3
            session.start(configuration.verifier_domain, tls_verify: false, &block)
          else
            session.start(configuration.verifier_domain, &block)
          end
        end

        def run
          start_session_wrapper do |smtp_request|
            response.connection = response.helo = true
            smtp_handshakes(smtp_request, response)
          end
        rescue => error
          retry if attempts_exist?
          assign_error(attribute: :connection, message: compose_from(error))
        end
      end
    end
  end
end

So if I don't miss anything I could create a PR using this patch 🤷‍♂️

@evserykh evserykh added the bug Something isn't working label Dec 3, 2021
@bestwebua
Copy link
Member

bestwebua commented Dec 3, 2021

Hi, @evserykh! What your truemail version, show your Truemail::VERSION output please. Which version are you using? As far as I remember this issue has been fixed in latest release: #186. Current master:

session.start(configuration.verifier_domain, **(::RUBY_VERSION[/\A3\..+\z/] ? { tls_verify: false } : {})) do |smtp_request|

@bestwebua
Copy link
Member

bestwebua commented Dec 3, 2021

Oh, my fault. Latest updates have broke Net::SMTP#start Ruby 2.x compatibility. Looks like that I should as soon as possible write truemail integration tests using smtpmock under the hood to get rid bugs like this one in the future 👯‍♀️

@bestwebua bestwebua linked a pull request Dec 3, 2021 that will close this issue
4 tasks
@bestwebua
Copy link
Member

@evserykh, this fix is available in latest (2.5.4) truemail release. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

2 participants