Sendmail argument change breaks Rails usage of Sendmail

[RaymondFallon]

I see from the CHANGELOG for v2.8.0 that the following change was made intentionally:

Sendmail and exim delivery :arguments option must be an array of string args

It was even listed as a "breaking change," so this issue is more of a question than a bug report. But I wanted to note that this change breaks how Rails' ActionMailer implements Sendmail:

add_delivery_method :sendmail, Mail::Sendmail,
  location:  "/usr/sbin/sendmail",
  arguments: "-i"

I don't know whether or not Rails applications represent a large portion of apps that use the mail gem, but in case they do, I wanted to point out this potentially high-impact breakage and confirm that it was deemed acceptable before attempting to fix things within Rails.

Thanks!

---

Additional info on breakage:

When attempting to deliver mail via the :sendmail method with rails 7.0.4 and mail 2.8.0, I get a runtime error like the following:

ArgumentError (:arguments expected to be an Array of individual string args):
  
/gems/mail/lib/mail/network/delivery_methods/sendmail.rb:53:in `initialize'

If the Sendmail argument signature is to stay as it is, I believe action_mailer will need to change its instantiation of :sendmail to:

add_delivery_method :sendmail, Mail::Sendmail,
  location:  "/usr/sbin/sendmail",
  arguments: "[-i]"

[eval]

Seems serious. @mikel how about a fix that doesn't raise here

mail/lib/mail/network/delivery_methods/sendmail.rb

Line 53 in fbed43b

raise ArgumentError, ":arguments expected to be an Array of individual string args" if settings[:arguments].is_a?(String)

but instead splits the string (and emits a warning)?

[mikel]

@eval yes, that makes sense, we should make that a PR and get that deployed as 2.8.1 right away. If we get a string, split it and put it back in.

[sebbASF]

Agree with restoring support for string.

However if valid arguments can contain blanks, it makes correctly splitting the string tricky.
An alternative would be to append the original string when building the command.

Whilst this would not be as safe against shell mis-interpretation, it is very unlikely that the arguments provided by callers are derived directly from user input.

As well as emitting a warning, it would be useful to document why a string is deprecated.

[kokogakayui]

Hi.

Just for the sake of feedback : mail 2.8.0 is indeed breaking my rails application in production when trying to deploy on AWS Beanstalk.

I am running ruby 3.0.5 and rails 6.1.7. Reverting back to 2.7.1 fixes it back.
Did not really find out what the error was, but something related to an uninitialized constant Mail::SendMail.

Hope this will help.

[eval]

However if valid arguments can contain blanks, it makes correctly splitting the string tricky.
An alternative would be to append the original string when building the command.

Good point @sebbASF. But adding an argument with spaces doesn't work either right? E.g. IO.popen(["ls", "-l -h"]) fails.
So it seems to me that when passed a string, the command should be a string.

[sebbASF]

Good point, agreed

[eval]

Current draft: #1547

[sebbASF]

#1547 includes changes to test.yml allow the PR to run as a GH action.

The unadorned draft is #1545

[pierre-alain-b]

Same here, this broke multiple production Rails apps. Reverting to 2.7.1.

[eval]

Fixed via #1545