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:
- #3312582: When a user account doesn't have an email, new user notification email attempt shouldn't be made
- #3355151: Exception thrown when unblocking a user via drush
- #3481894: Error when trying to reset password of the account without email address
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
- Log into the site as an administrator.
- Create a new user. The user should not have an email address.
- 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
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | after.png | 223.42 KB | shivam_tiwari |
| #7 | before.jpg | 309.55 KB | shivam_tiwari |
Issue fork drupal-3518058
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
Comment #3
dcam commentedComment #4
dcam commentedComment #5
dcam commentedComment #6
shivam_tiwari commentedComment #7
shivam_tiwari commentedI 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
Comment #8
poker10 commentedThanks 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.
Comment #9
dcam commentedApplied all feedback. Updated the proposed resolution for info logging.
Comment #10
smustgrave commentedResolved the threads but appears to be 1 open one still.
Comment #11
dcam commentedI updated the @return statement.
Comment #12
smustgrave commentedFeedback appears to be addressed.
Comment #14
catchOne question on the MR.
Comment #15
dcam commentedI addressed the feedback.
Comment #16
smustgrave commentedFeedback appears to be addressed.
Comment #18
catchNew log message looks good.
Committed/pushed to 11.x, thanks!
Comment #20
dcam commentedI closed or updated the related issues.