4 ms·

Since both users mentioned were the last in the list of addresses for the email they received, my money's on a trivial mistake like: getEmailBody(users[:i]

Since both users mentioned were the last in the list of addresses for the email they received, my money's on a trivial mistake like: getEmailBody(users[:i]) instead of getEmailBody(users[i]) I typically prefer a high level of polymorphism in my code/APIs (sensibly handling single inputs vs. arrays) but this is a great counter-example even if not the actual root cause. Every feature is also a liability. Double edged sword. Etc.

justinpombrio 10y ago
Sounds like an argument to write for user in users: getEmailBody(user) instead of for i in range(len(users)): getEmailBody(users[i]) (for the languages that let you do so).
codygman 10y ago
A type system can really help here as well. sendUserEmail :: FromEmail -> ToEmail -> [EmailHeader] -> EmailBody -> IO () However that also requires the discipline of wrapping a lower level function that is probably: sendEmails :: FromEmail -> [ToEmail] -> [EmailHeader] -> EmailBody However even a functional language would shy away from using indexing which could be argued to be the source of this problem.
- ubernostrum 10y ago
  Nothing about your suggested solution requires or is exclusive to a statically-typed language. "Replace a send-to-arbitrary-number-of-addresses function with a send-to-exactly-one-address function" is possible in dynamically-typed languages, too, and as you openly admit even a statically-typed language is likely to model email sending in a way that accepts multiple recipient addresses. So your "type system can really help" is really just an irrelevancy you've come up with to try hide your attempt to shove your preferred programming paradigm onto other people. You should probably stop doing that.
  - Johnny_Brahms 10y ago
    i thought of that as well.Went and looked at my mail list programme in Racket, and the mail procedure takes many recipients, but in the form if rest arguments, like: (define (send-mail msg . recipients) ...) passing it a list makes the recipients arg a list with a list of recipients. To actually send to multiple recipients I explicitly have to use the apply procedure that passes all list elements as arguments. as you said, not a staticly typed language. types would have made an eventual error easier to debug though.
  - codygman 10y ago
    > So your "type system can really help" is really just an irrelevancy you've come up with to try hide your attempt to shove your preferred programming paradigm onto other people. Not exactly, but it is a weaker advantage than I thought when writing it. The advantage is that languages like Haskell encourage specializing functions in that manner which avoid that specific bug. However so does test driven development which is just as possible in dynamic languages. The advantage the statically typed language has over even the dynamically typed language plus tdd is that the program won't compile whereas dynamic language plus tdd relies on programmer discipline.
- andrewstuart2 10y ago
  Well, using the type system, weak or strong, to force correct code either at compile time or runtime would help. I can write a function that uses Java reflection for polymorphism as easily as I can use JavaScript reflection to enforce an API contract. // JavaScript if ( Array.isArray(address) ) { throw new Error("Must only pass a single email address."); } // Java void sendEmail(Object recipientOrRecipients) { ... } There are times when polymorphism is low-risk, and there are times when it's better safe than sorry. Best to know your risk model (and your libraries) and act accordingly.
codedokode 10y ago
People on twitter suggest the reason could be a feature of a Python library: https://twitter.com/TvdW/status/741481798014664704 https://twitter.com/TvdW/status/741481798014664704 Assignment is interpreted as add-to-the-list operator.
- sirn 10y ago
  Relevant code from the `email` module. Python 2.7: https://hg.python.org/releasing/2.7.9/file/tip/Lib/email/message.py#l296 https://hg.python.org/releasing/2.7.9/file/tip/Lib/email/mes... Python 3.4: https://hg.python.org/releasing/3.4/file/tip/Lib/email/message.py#l388 https://hg.python.org/releasing/3.4/file/tip/Lib/email/messa... Ouch.
- Cyph0n 10y ago
  It was confirmed by Josh, co-founder of Let's Encrypt: https://twitter.com/0xjosh/status/741487697059946497 https://twitter.com/0xjosh/status/741487697059946497.
- Gigablah 10y ago
  This looks like it belongs in a "fractal of bad design" article.
chrismorgan 10y ago
I fixed a very similar bug in an ASP.NET site a few weeks back; email generation had a factory class where each time it wished to construct an email it would set From = this, Subject = that, Body = the other, but To was a list, so it wrote To.Add(user.Email), without a To.Clear() call at the start. So the first email went out to one person, the second to two, the third to three… someone else had made the site and on receiving a report of it once I had briefly looked through the code and missed it, then when another client complained of the addresses plus having received sixteen emails I looked again and realised what was going on.

justinpombrio 10y ago

Sounds like an argument to write for user in users: getEmailBody(user) instead of for i in range(len(users)): getEmailBody(users[i]) (for the languages that let you do so).

codygman 10y ago

A type system can really help here as well. sendUserEmail :: FromEmail -> ToEmail -> [EmailHeader] -> EmailBody -> IO () However that also requires the discipline of wrapping a lower level function that is probably: sendEmails :: FromEmail -> [ToEmail] -> [EmailHeader] -> EmailBody However even a functional language would shy away from using indexing which could be argued to be the source of this problem.

ubernostrum 10y ago
Nothing about your suggested solution requires or is exclusive to a statically-typed language. "Replace a send-to-arbitrary-number-of-addresses function with a send-to-exactly-one-address function" is possible in dynamically-typed languages, too, and as you openly admit even a statically-typed language is likely to model email sending in a way that accepts multiple recipient addresses. So your "type system can really help" is really just an irrelevancy you've come up with to try hide your attempt to shove your preferred programming paradigm onto other people. You should probably stop doing that.
- Johnny_Brahms 10y ago
  i thought of that as well.Went and looked at my mail list programme in Racket, and the mail procedure takes many recipients, but in the form if rest arguments, like: (define (send-mail msg . recipients) ...) passing it a list makes the recipients arg a list with a list of recipients. To actually send to multiple recipients I explicitly have to use the apply procedure that passes all list elements as arguments. as you said, not a staticly typed language. types would have made an eventual error easier to debug though.
- codygman 10y ago
  > So your "type system can really help" is really just an irrelevancy you've come up with to try hide your attempt to shove your preferred programming paradigm onto other people. Not exactly, but it is a weaker advantage than I thought when writing it. The advantage is that languages like Haskell encourage specializing functions in that manner which avoid that specific bug. However so does test driven development which is just as possible in dynamic languages. The advantage the statically typed language has over even the dynamically typed language plus tdd is that the program won't compile whereas dynamic language plus tdd relies on programmer discipline.
andrewstuart2 10y ago
Well, using the type system, weak or strong, to force correct code either at compile time or runtime would help. I can write a function that uses Java reflection for polymorphism as easily as I can use JavaScript reflection to enforce an API contract. // JavaScript if ( Array.isArray(address) ) { throw new Error("Must only pass a single email address."); } // Java void sendEmail(Object recipientOrRecipients) { ... } There are times when polymorphism is low-risk, and there are times when it's better safe than sorry. Best to know your risk model (and your libraries) and act accordingly.

codedokode 10y ago

People on twitter suggest the reason could be a feature of a Python library: https://twitter.com/TvdW/status/741481798014664704 https://twitter.com/TvdW/status/741481798014664704 Assignment is interpreted as add-to-the-list operator.

sirn 10y ago
Relevant code from the `email` module. Python 2.7: https://hg.python.org/releasing/2.7.9/file/tip/Lib/email/message.py#l296 https://hg.python.org/releasing/2.7.9/file/tip/Lib/email/mes... Python 3.4: https://hg.python.org/releasing/3.4/file/tip/Lib/email/message.py#l388 https://hg.python.org/releasing/3.4/file/tip/Lib/email/messa... Ouch.
Cyph0n 10y ago
It was confirmed by Josh, co-founder of Let's Encrypt: https://twitter.com/0xjosh/status/741487697059946497 https://twitter.com/0xjosh/status/741487697059946497.
Gigablah 10y ago
This looks like it belongs in a "fractal of bad design" article.

chrismorgan 10y ago

I fixed a very similar bug in an ASP.NET site a few weeks back; email generation had a factory class where each time it wished to construct an email it would set From = this, Subject = that, Body = the other, but To was a list, so it wrote To.Add(user.Email), without a To.Clear() call at the start. So the first email went out to one person, the second to two, the third to three… someone else had made the site and on receiving a report of it once I had briefly looked through the code and missed it, then when another client complained of the addresses plus having received sixteen emails I looked again and realised what was going on.