Problem/Motivation

Running drush to send mail I see deprecation in logs because SERVER_SOFTWARE environment variable is not set.

PHP Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in docroot/core/lib/Drupal/Core/Mail/Plugin/Mail/PhpMail.php on line 112

the most common case is checking for updates and sending mail

Steps to reproduce

run drush to execute job to send mail and then check logs

Proposed resolution

- Use common across core the check for windows git grep PHP_OS which is substr(PHP_OS, 0, 3) == 'WIN'
- deprecate unused property in #3372667: Deprecate PHPMail::request property
- decide fate of platform in #3358248: [policy, no patch] Drop support for IIS in Drupal 11 as looks the check failing since Drupal 7 - #2983438: Truncated MailFrom address - Incorrect Windows detection

Remaining tasks

- patch
- add test and commit

User interface changes

no

API changes

no

Data model changes

no

Release notes snippet

CommentFileSizeAuthor
#19 3372667-2.patch806 bytesandypost
#8 interdiff_7-8.txt841 bytesdivyansh
#8 3328187-8.patch903 bytesdivyansh
#7 3328187-7.patch905 bytesdivyansh
#6 3328187-6.patch891 bytesdivyansh
#3 3328187-3-9.5.x.patch943 bytesthtas

Issue fork drupal-3328187

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

Rajeshreeputra created an issue. See original summary.

Rajeshreeputra opened merge request !3142

thtas’s picture

thtas commented
StatusFileSize
new 3328187-3-9.5.x.patch943 bytes

Re-rolled against 9.5.x

enchufe’s picture

enchufe
Location Spain
commented

Patch #3 worked for me, thanks!

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Status: Active » Needs work
Issue tags: +Needs reroll
divyansh’s picture

divyansh commented
Status: Needs work » Needs review
StatusFileSize
new 3328187-6.patch891 bytes

I am new to the contribution and below is my first patch for this issue. I have rerolled the patch for 10.0.x.

divyansh’s picture

divyansh commented
StatusFileSize
new 3328187-7.patch905 bytes

I have updated the patch. I have called the "request" variable from protected using "this"

divyansh’s picture

divyansh commented
StatusFileSize
new 3328187-8.patch903 bytes
new interdiff_7-8.txt841 bytes

Sorry for the previous patch, I missed to remove "$". I have fixed in this patch

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented
Issue tags: -Needs reroll

Method `get` supports the second argument, NULL, by default. Passing an empty string will provide the expected behavior.
Rebased the MR and provided suggestions for the current solution.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update, +Needs steps to reproduce, +Needs tests

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

The issue summary should be updated with steps to reproduce this issue.

We should track down what is causing this issue as this may be covering up a larger problem.

After that we need tests.

enchufe’s picture

enchufe
Location Spain
commented

Issue solved in branch 9.5.x, replaced code with 

substr(PHP_OS, 0, 3) != 'WIN'
rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

For all the tags

rajeshreeputra’s picture

rajeshreeputra
he/him
Location Pune
commented

rebased as patch failure with latest release.

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

sakthi_dev’s picture

sakthi_dev commented
Status: Needs work » Needs review

Please review.

borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented
Status: Needs review » Needs work

For all the tags

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3372667: Deprecate PHPMail::request property

I filed duplicate issue #3372667: Deprecate PHPMail::request property

Probably patch could be moved here and removal of request from constructor could be done there

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
Issue tags: -Needs issue summary update, -Needs steps to reproduce
Related issues: +#3358248: [policy, no patch] Drop support for IIS in Drupal 11, +#2983438: Truncated MailFrom address - Incorrect Windows detection
StatusFileSize
new 3372667-2.patch806 bytes

In #3372667-5: Deprecate PHPMail::request property discovered that this check probably not working for a long time, see #2983438: Truncated MailFrom address - Incorrect Windows detection

New patch from #3372667-2: Deprecate PHPMail::request property

Updated IS and steps, leaving NW for tests

Probably it should be Unit test, because other tests overriding this plugin to catch mail

Version: 10.0.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.