Problem/Motivation

When not logged in, it seems that trying to send a contact form automatically adds the anonymous user empty email address to the "addresses" array of the email object (in reply-to I think).

Steps to reproduce:

  1. Create a contact form.
  2. Go to "Manage form display" and set "Sender email" to disabled
  3. Log out, and visit the contact form as anonymous
  4. Send the message and see an error "Unable to send email. Contact the site administrator if the problem persists."
  5. A log message is generated "Error sending email: Email "" does not comply with addr-spec of RFC 2822."

Proposed resolution

Prevent getSymfonyEmail() from adding empty addresses ?
It should be fixed at the source - i.e. in this case, in the contact form module, but it could be worth not breaking the sending of any email with multiple addresses due to one empty email.

CommentFileSizeAuthor
#4 screenshot-contact-form-fields-symfony-mailer.png27.38 KBpaulmicha
#2 symfony_mailer-errorSendingEmailDoesNotComplyRFC-3401849-1.patch559 bytespaulmicha

Issue fork symfony_mailer-3401849

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Paulmicha created an issue. See original summary.

paulmicha’s picture

paulmicha commented
StatusFileSize
new symfony_mailer-errorSendingEmailDoesNotComplyRFC-3401849-1.patch559 bytes

Current workaround : cf. patch attached.

adamps’s picture

adamps commented
Status: Active » Postponed (maintainer needs more info)

I can't reproduce this. When sending as anonymous then there is a box to put your email address in. Perhaps you have disabled the field on your site?

paulmicha’s picture

paulmicha commented
StatusFileSize
new screenshot-contact-form-fields-symfony-mailer.png27.38 KB

Indeed, could it be that the switch from Swiftmailer to Symfony Mailer has introduced these fields ?
In our case, it seems preferable not to add those fields to the existing forms.

adamps’s picture

adamps commented
Title: Error sending email: Email "" does not comply with addr-spec of RFC 2822. » Allow null sender for contact form
Category: Support request » Feature request
Status: Postponed (maintainer needs more info) » Active

Indeed, could it be that the switch from Swiftmailer to Symfony Mailer has introduced these fields ?

No, the field I refer to is part of Core.

In our case, it seems preferable not to add those fields to the existing forms.

Interesting I'm surprised that it worked without using this module. I had imagined that this field is mandatory.

I believe that the right fix is in ContactEmailBuilderBase

- if ($email->getSubType() == 'mail') {
+ if ($sender && $email->getSubType() == 'mail') {
mrcdrx’s picture

mrcdrx commented

I can confirm that the patch in #2 works.

adamps’s picture

adamps commented
Status: Active » Needs work
adamps’s picture

adamps commented
Issue tags: +Novice

See #5 for the proposed 1-line fix

koustav_mondal’s picture

koustav_mondal commented
Assigned: Unassigned » koustav_mondal

Working on it.

koustav_mondal opened merge request !137

koustav_mondal’s picture

koustav_mondal commented
Assigned: koustav_mondal » Unassigned
Status: Needs work » Needs review

@adamps I made the changes according to #5 . Please have a look.

adamps’s picture

adamps commented
Status: Needs review » Needs work

Thanks. There is a merge conflict unfortunately.

brandonlira made their first commit to this issue’s fork.

brandonlira opened merge request !142

brandonlira closed merge request !142

brandonlira’s picture

brandonlira commented
Assigned: Unassigned » brandonlira

brandonlira opened merge request !143

brandonlira’s picture

brandonlira commented
Status: Needs work » Needs review

Hi everyone,

I have updated the fix to target the correct version (1.4.0) by creating a new branch based on 1.x. The previous merge request was for 2.x, but after reviewing the issue and its comments, I realized that the correct fix should be applied to 1.4.0.

I have now submitted a new Merge Request (!143) for this version. Please review the changes and let me know if anything needs to be adjusted.

Additionally, I noticed that some PHPStan checks have failed. From my understanding, these errors seem to be pre-existing and unrelated to this MR. Could someone confirm if I should address them, or if they can be handled separately?

Thanks!

brandonlira’s picture

brandonlira commented
Assigned: brandonlira » Unassigned
adamps’s picture

adamps commented
Version: 1.4.0 » 2.x-dev
Status: Needs review » Needs work

All fixes now go in 2.x please. The file is now ContactMailer.php and the code to change is line 100

    if ($email->getTag(2) == 'mail') {
brandonlira’s picture

brandonlira commented
Assigned: Unassigned » brandonlira

brandonlira closed merge request !143

brandonlira opened merge request !144

brandonlira’s picture

brandonlira commented

Hi @adamps,

I have moved the fix to ContactMailer.php (line 100) for 2.x MR (!144) and added a check to ensure $sender is not NULL before calling setReplyTo().

Please review and let me know if any adjustments are needed.

Thanks!

brandonlira’s picture

brandonlira commented
Assigned: brandonlira » Unassigned
Status: Needs work » Needs review
adamps’s picture

adamps commented
Issue summary: View changes

Thanks it looks good.

This needs someone to do a manual test. I added "steps to reproduce" in the issue summary. First run them without this patch and check you see the error. Then apply the patch and confirm the problem is fixed.

adamps’s picture

adamps commented

  • adamps committed fa24d4c6 on 2.x authored by brandonlira 
    Issue #3401849 by brandonlira, adamps, koustav_mondal, paulmicha: Allow...
adamps’s picture

adamps commented
Status: Needs review » Fixed
Issue tags: -Novice

Good job I tested it as it didn't work. In future please test fixes if you want to get credit😃.

Status: Fixed » Closed (fixed)

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