When selecting an "Email to" address, the instructions clearly note: "Form submissions will be e-mailed to this address. Any email, select, or hidden form element may be selected as the recipient address. Multiple e-mail addresses may be separated by commas." [emphasis added]

However, when the hidden form element contains multiple email addresses separated by commas, the sending fails entirely (because the $address is returned with a comma, still, and that fails to pass through the valid_email_address() test). Classifying this as a bug report, since it doesn't work as intended.

Attached is a patch to fix it. I've checked it thoroughly with hidden fields, and it works great. I've also given it a cursory check on checkboxes/radio buttons/select boxes, and it doesn't seem to break either. But I'd recommend a bit more testing on that latter portion...

CommentFileSizeAuthor
webform.patch1.11 KBsbandyopadhyay
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sbandyopadhyay’s picture

Wow, I documented those changes very poorly. Let me try again:

1) The trim is necessary because some people will probably put in "email1@example.com, email2@example.com" with a space in between the comma and "email2". Without the trim, the valid_email_address would fail on " email2@example.com" [NOTE the space in that right at the beginning].

2) The second change should be pretty self-explanatory: rather than simply equate $address with the hidden component value, it explodes it and merges the result in to the previously established $address array. This plays nice with select components, and I even believe it would now allow select boxes to have multiple emails in the key as well. (Though I'm not sure if PHP would allow a comma in the key of an array of options.)

quicksketch’s picture

Category: bug » task
Priority: Critical » Normal

The addition of trim() is nice and should prevent a lot of accidental problems. We already fix this problem for end-users that enter their own e-mails as part of #650434: Remove white space from email fields. Arguably administrators should be smart enough to enter valid e-mail addresses without spaces, but I'm sure most people would say this is not the case.

Regarding explode() on the hidden field values, usually this is not necessary since formatting multiple e-mail addresses separated by commas is directly supported by e-mail standards. If you comma-separate multiple e-mail addresses, they already get sent to all the recipients. Since there's no need to send individual e-mails (since they're all coming from the same component they'll all have the same template), I'm not sure what benefit there is in this change.

quicksketch’s picture

Status: Needs review » Fixed

Ah, okay I see what you've said now (should have read more clearly). From your original report:

Any email, select, or hidden form element may be selected as the recipient address. Multiple e-mail addresses may be separated by commas.

I was worried about making components send multiple e-mails separated by commas because I thought it would allow users to send multiple e-mails by entering commas inside of "email" components. However our form validation prevents users from doing this, meaning the only way to send multiple e-mails is through admin-specified components like radios or hidden fields.

So this all looks good to me. Committed. Thanks!

sbandyopadhyay’s picture

Thanks! :)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.