Being all that said, I actually think it deserves Lighthouse issue, please create one.
Pavel On Apr 24, 10:25 pm, Pavel Kunc <[email protected]> wrote: > Hi Tony, > > Being the author of the change I explain my rationale behind it. > > The old Merb code is not correct either because it doesn't work in > Ruby 1.9.1. The behaviour of Array#to_s did change and so the old Merb > code didn't work at all. > > The @mail.to.to_s.split() is also unnecessary if the @mail.to array > would look like ['email', 'email'], than you can pass it directly to > the send_message(). However it seems that code is there to catch cases > like ['email, email', 'email'] to become and array of three elements. > Which actually seems like your case. > > Also the fact that the MailFactory does return only array might be > true but using merb_exceptions I found that the @mail.to can actually > be string in some circumstances. I don't remember, it's some time so > we can revisit this, if that is not true than we could actually make > all work on all Rubies with something like: > > @mail.to.join(',').split(/[,;]/) > > I have no problem resolving the issue to some state which will work on > 1.8.x and 1.9.x and for merb_exceptions :-) > > Pavel > > On Apr 14, 8:10 pm, Tony Mann <[email protected]> wrote: > > > > > > > Currently, merb-mailer 1.1 will not properly support a comma-separated list > > of addresses nor an array of addresses if you are using SMTP. Here is why: > > > MailFactory has a bug that prevents it from supporting an array of > > addresses. If you try to use an array, the addresses get smashed together. > > The problem is in their add_header method: > > > def add_header(header, value) > > value = quoted_printable_with_instruction(value, @charset) if header == > > 'subject' > > value = quote_address_if_necessary(value, @charset) if %w[from to cc bcc > > reply-to].include?(header.downcase) > > @headers << "#{header}: #{value}" > > end > > > The quote_address_if_necessary call handles an array, but then it gets > > converted into a string when added to @headers. Oops. > > > So, MailFactory requires that you pass in a comma-separate list. However, > > later on in merb-mailer, we have this code: > > > smtp.start(config[:domain], config[:user], config[:pass], config[:auth]) { > > |smtp| > > to = @mail.to.is_a?(String) ? @mail.to.split(/[,;]/) : @mail.to > > smtp.send_message(@mail.to_s, @mail.from.first, to) > > > } > > > Unfortunately, this will not work, because @mail.to is never a String! Why? > > Because MailFactory always returns an array with one item in it (to support > > multiple headers with the same tag). As mentioned above, the single item in > > that array is always a String. So the whole check is no good and > > unnecessary. The old merb-mailer code looked like this: > > > smtp.start(config[:domain], config[:user], config[:pass], config[:auth]) { > > |smtp| > > smtp.send_message(@mail.to_s, @mail.from.first, > > @mail.to.to_s.split(/[,;]/)) > > > } > > > This is correct code, and the 1.1 code should be reverted to this. > > > Now, I have not posted a Lighthouse ticket quite yet, because I am afraid I > > am missing something. Per above, merb-mailer 1.1 breaks sending mail via > > SMTP to multiple recipients, and I find it hard to believe no one else has > > encountered this issue yet. Can anyone enlighten me? > > > ..tony.. > > -- > You received this message because you are subscribed to the Google Groups > "merb" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]. > For more options, visit this group > athttp://groups.google.com/group/merb?hl=en. -- You received this message because you are subscribed to the Google Groups "merb" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/merb?hl=en.
