Skip to content

Update mail gem to v2.8.1

Stan Hurequested to merge
sh-update-mail-2.8.1 into master

What does this MR do and why?

  1. This merge request fixes mail sending when TLS is disabled. In Ruby 3.0.5, net-smtp v0.2.1 enabled TLS by default. However, mail v2.7.1 didn't explicitly disable TLS (https://github.com/mikel/mail/issues/1434), so TLS is always enabled with Ruby 3. mail v2.8.1 has since fixed this issue via https://github.com/mikel/mail/pull/1480. https://github.com/mikel/mail/pull/1536 improves this in master.

  2. This merge request also removes one monkey patch introduced in !24153 (diffs) since https://github.com/mikel/mail/pull/1210 was merged for mail v2.8.0.

  3. In addition, we can drop the mail interceptor validator. This is no longer needed in mail 2.8.1 since https://github.com/mikel/mail/commit/7e1196bd29815a0901d7290c82a332c0959b163a has solved the security issue by passing the arguments to Sendmail as an array instead of a string.

  4. Ignore net-protocol warnings for Ruby 2.7.7. Since the mail gem now requires net-imap, which needs net-protocol, there's no use in adding an exception in Gemfile. We now ignore the warnings in Ruby 2.7.

Relates to:

  1. #399241 (closed)
  2. #396252 (closed)

Diff: https://my.diffend.io/gems/mail/2.7.1/2.8.1

Testing

I used the package in https://gitlab.com/gitlab-org/build/omnibus-gitlab-mirror/-/jobs/3993676328 with Gmail SMTP.

Before

This shouldn't have worked, but it did:

irb(main):001:0> ActionMailer::Base.smtp_settings[:enable_starttls_auto] = false
=> false
irb(main):002:0> Notify.test_email('stanhu@gitlab.example.com', 'Hello World', 'This is a test message').deliver_now
Delivered mail 641d43b8e741d_38eed748f8258f@ip-172-30-0-215.mail (1237.8ms)

After

With mail v2.8.1, disabling TLS does the right thing:

irb(main):002:0> Notify.test_email('stanhu@gitlab.example.com', 'Hello World', 'This is a test message').deliver_now
Delivered mail 641d41e8a7be3_154f464934549b@stanhu-gce.mail (943.8ms)
irb(main):003:0> ActionMailer::Base.smtp_settings[:enable_starttls_auto] = false
=> false
irb(main):004:0> Notify.test_email('stanhu@gitlab.example.com', 'Hello World', 'This is a test message').deliver_now
Delivered mail 641d420c6c2_154f46493455f4@stanhu-gce.mail (9.9ms)
/opt/gitlab/embedded/lib/ruby/gems/3.0.0/gems/net-smtp-0.3.3/lib/net/smtp.rb:1094:in `check_auth_continue': 530 5.7.0 Must issue a STARTTLS command first. ck16-20020a0566383f1000b003cfd7393382sm6493936jab.93 - gsmtp (Net::SMTPAuthenticationError)

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Stan Hu

Merge request reports