Problem/Motivation

User accounts that don't have an email address can be created by administrators. _user_mail_notify() assumes that when it's passed a user account that it has an address. When that happens errors occur. It happens for any user action that sends an email. It can happen via the admin UI or Drush. This issue has resulted in at least three different issues that I could find, but there may be more:

These issues have proposed patches, but they all try to stop the email from being sent at the source, e.g. the User entity or the password reset form. None of them try to prevent _user_mail_notify() from sending an email to a NULL address, which means the inherent bug will persist. This issue will take that approach.

Afterward, some of the related issues may simply be closed. Some may be repurposed as follow-ups. For instance, the password reset form may want to print an extra message if an email can't be sent.

Steps to reproduce

  1. Log into the site as an administrator.
  2. Create a new user. The user should not have an email address.
  3. Block the user.

Expected result

The user should be blocked with no attempt to send an email.

Actual result

[error]  TypeError: Drupal\Core\Mail\Plugin\Mail\PhpMail::doMail(): Argument #1 ($to) must be of type string, null given, called in /var/www/html/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php on line 123 in Drupal\Core\Mail\Plugin\Mail\PhpMail->doMail() (line 168 of /var/www/html/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php) #0 /var/www/html/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php(123): Drupal\Core\Mail\Plugin\Mail\PhpMail->doMail()
#1 /var/www/html/core/lib/Drupal/Core/Mail/MailManager.php(308): Drupal\Core\Mail\Plugin\Mail\PhpMail->mail()
#2 /var/www/html/core/lib/Drupal/Core/Mail/MailManager.php(181): Drupal\Core\Mail\MailManager->doMail()
#3 /var/www/html/core/lib/Drupal/Core/Render/Renderer.php(600): Drupal\Core\Mail\MailManager->Drupal\Core\Mail\{closure}()
#4 /var/www/html/core/lib/Drupal/Core/Mail/MailManager.php(180): Drupal\Core\Render\Renderer->executeInRenderContext()
#5 /var/www/html/core/modules/user/user.module(659): Drupal\Core\Mail\MailManager->mail()
#6 /var/www/html/core/modules/user/src/Entity/User.php(162): _user_mail_notify()
#7 /var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php(564): Drupal\user\Entity\User->postSave()
#8 /var/www/html/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php(796): Drupal\Core\Entity\EntityStorageBase->doPostSave()
#9 /var/www/html/core/lib/Drupal/Core/Entity/EntityStorageBase.php(490): Drupal\Core\Entity\ContentEntityStorageBase->doPostSave()
#10 /var/www/html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php(806): Drupal\Core\Entity\EntityStorageBase->save()
#11 /var/www/html/core/lib/Drupal/Core/Entity/EntityBase.php(370): Drupal\Core\Entity\Sql\SqlContentEntityStorage->save()
#12 /var/www/html/vendor/drush/drush/src/Commands/core/UserCommands.php(154): Drupal\Core\Entity\EntityBase->save()
#13 [internal function]: Drush\Commands\core\UserCommands->unblock()
#14 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(276): call_user_func_array()
#15 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback()
#16 /var/www/html/vendor/consolidation/annotated-command/src/CommandProcessor.php(175): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter()
#17 /var/www/html/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(387): Consolidation\AnnotatedCommand\CommandProcessor->process()
#18 /var/www/html/vendor/symfony/console/Command/Command.php(279): Consolidation\AnnotatedCommand\AnnotatedCommand->execute()
#19 /var/www/html/vendor/symfony/console/Application.php(1094): Symfony\Component\Console\Command\Command->run()
#20 /var/www/html/vendor/symfony/console/Application.php(342): Symfony\Component\Console\Application->doRunCommand()
#21 /var/www/html/vendor/symfony/console/Application.php(193): Symfony\Component\Console\Application->doRun()
#22 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(110): Symfony\Component\Console\Application->run()
#23 /var/www/html/vendor/drush/drush/src/Runtime/Runtime.php(40): Drush\Runtime\Runtime->doRun()
#24 /var/www/html/vendor/drush/drush/drush.php(140): Drush\Runtime\Runtime->run()
#25 /var/www/html/vendor/bin/drush.php(119): include('...')
#26 {main}.

Proposed resolution

Add a check to _user_mail_notify() to verify that the account has an email address. Log info if it doesn't have one.

Remaining tasks

  • Agree on the message to be displayed
  • Review
  • Commit
  • Evaluate related issues to see if some can be closed

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 after.png223.42 KBshivam_tiwari
#7 before.jpg309.55 KBshivam_tiwari

Issue fork drupal-3518058

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:

Comments

dcam created an issue. See original summary.

dcam’s picture

Issue summary: View changes
dcam’s picture

Status: Active » Needs review
dcam’s picture

Issue summary: View changes
shivam_tiwari’s picture

Assigned: Unassigned » shivam_tiwari
shivam_tiwari’s picture

Assigned: shivam_tiwari » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new309.55 KB
new223.42 KB

I checked this issue and MR. I got same problem, if user don't have email address then I am getting 500 error page with same error message as given here. I applied MR as a patch and it is working fine. It gives an information also to the admin user that updated user don't have email address so email could not be sent.
I am adding some screenshots also related to it and moving this to RTBC

poker10’s picture

Status: Reviewed & tested by the community » Needs work

Thanks for working on this. I think that it is a good idea to check the existence of the email address in the _user_mail_notify(), instead of in all calling functions (see the related issues in the IS).

I reviewed this and added some comments to the MR. Moving it to NW.

dcam’s picture

Issue summary: View changes
Status: Needs work » Needs review

Applied all feedback. Updated the proposed resolution for info logging.

smustgrave’s picture

Status: Needs review » Needs work

Resolved the threads but appears to be 1 open one still.

dcam’s picture

Status: Needs work » Needs review

I updated the @return statement.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Feedback appears to be addressed.

dcam changed the visibility of the branch 11.x to hidden.

catch’s picture

Status: Reviewed & tested by the community » Needs work

One question on the MR.

dcam’s picture

Status: Needs work » Needs review

I addressed the feedback.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Feedback appears to be addressed.

  • catch committed 07df55fc on 11.x
    Issue #3518058 by dcam, shivam_tiwari, poker10: _user_mail_notify()...
catch’s picture

Status: Reviewed & tested by the community » Fixed

New log message looks good.

Committed/pushed to 11.x, thanks!

dcam’s picture

I closed or updated the related issues.

Status: Fixed » Closed (fixed)

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