Problem/Motivation

Hello, when sending mail in the KernelEvents::TERMINATE event you will receive an exception from NativeSessionStorage with the message "Failed to start the session because headers have already been sent by ..." this happens after updating to version 1.3 since the method \Drupal\smtp\ Plugin\Mail\SMTPMailSystem::debug() tries to determine whether debug is enabled by checking the value from the session.

Sending mail in the KernelEvents::TERMINATE event is a fairly common solution if the sending status is not important to the user. This allows you to avoid slowing down page loading for the user by sending mail, since the event is executed after the response is sent.

Steps to reproduce

Add a subscriber to the KernelEvents::TERMINATE event and send an email from it.

Proposed resolution

Use any other storage for the global debug indicator for the current request.
It is advisable to use something that is not sent to the client. For example, a static class variable or the request itself.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#9 smtp-headers_sent-3458046-9.patch1.52 KBkerasai
#8 smtp_failed_to_start_session.patch834 bytesholo96

Issue fork smtp-3458046

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

kksandr created an issue. See original summary.

kksandr’s picture

kksandr commented
Issue summary: View changes

kksandr opened merge request !36

kksandr’s picture

kksandr commented

I added a test that simulates sending mail to reproduce the error.

kksandr’s picture

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

kksandr commented

I don’t really understand why the session was used here, but it seems that every mail sending ends with logs being written and cleared, that is, there is no accumulation at the request level, so I’ll replace the log storage with a regular class property instead of a static one. If I'm wrong, the commit can be deleted.

holo96’s picture

holo96 commented
Priority: Normal » Critical

Setting this to critical since core's Automated Cron is using KernelEvents::TERMINATE, and it is surely common to send emails from cron or queues

edit: queues endup in endless looping

holo96’s picture

holo96 commented
StatusFileSize
new smtp_failed_to_start_session.patch834 bytes

This is probably acceptable while while debug is enabled
So checking smtp_debugging before getting smtp_debug from session could also work, patch attached

kerasai’s picture

kerasai commented
StatusFileSize
new smtp-headers_sent-3458046-9.patch1.52 KB

Here's an updated version of the patch.

I moved the conditionals that would prevent debugging from being logged to the beginning of the method, so we can immediately return. I've also added a call to headers_sent() to avoid accessing the session if TRUE (headers have been sent).

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

bluegeek9 changed the visibility of the branch 8.x-1.x to hidden.

brevilo’s picture

brevilo commented

Is this module still maintained? This is a rather major bug with a proposed patch and it's been sitting here for almost 1.5 years (the lastest release is even older)...

Thanks