Problem/Motivation

Plain text email constructed with mail.inc will trigger several SpamAssassin mixed case URI tests. Any MTA that has those filters enabled will flag mail from Drupal as spam (if they are all enabled, it can add up to a very, very high spam score); some of those tests are HTTP_MIXC BODY, URI_TLD_MIXC URI, and URI_PROTO_MIXC URI.

These tests are not enabled by default, and it is possibly a questionable decision to have them enabled, at all, but some relatively large mail servers do (I get bounces every few days due to it, when I forget the patch my mail.inc after a Drupal update; we send out a few thousand notification emails a day).

The offending lines are:

// Fancy headers
        case 'h1':
          $indent[] = '======== ';
          $casing = 'drupal_strtoupper';
          break;
        case 'h2':
          $indent[] = '-------- ';
          $casing = 'drupal_strtoupper';
          break;

Simply removing the "$casing = 'drupal_strtoupper';" lines resolves this problem. It also makes plain text email slightly less ugly (all caps is obnoxious, regardless of its problems triggering spam filters).

Note that this also overrides templates and such, without any configurable options to avoid it, which seems unfriendly.

The problem has been present for most of forever. It is in both our 6.x production site and 7.x development site.

Proposed resolution

  • Remove mb_strtoupper from the h1 and h2 tags in MailFormatHelper.php::htmlToText() method

Remaining tasks

  • Review
  • Commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#20 interdiff-18-20.txt3.32 KBHardik_Patel_12
#20 2414019-20.patch5.13 KBHardik_Patel_12
#18 2414019-18.patch1.65 KBHardik_Patel_12
#4 keep_case_in_plain_text_mail-2414019-4-d8.patch1.72 KBmpp
#1 MailFormatHelper.php-nocaps.patch511 bytesswelljoe
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

swelljoe’s picture

swelljoe CreditAttribution: swelljoe as a volunteer commented
Version: 7.34 » 8.0.0-rc4
FileSize
MailFormatHelper.php-nocaps.patch511 bytes

This problem still exists in Drupal 8.0-rc4. In fact, it's had some attention by someone to keep the misfeature working as always, as it has been rewritten to support the new Drupal Unicode stuff.

          // Fancy headers.
          case 'h1':
            $indent[] = '======== ';
            $casing = '\Drupal\Component\Utility\Unicode::strtoupper';
            break;

          case 'h2':
            $indent[] = '-------- ';
            $casing = '\Drupal\Component\Utility\Unicode::strtoupper';
            break;

Am I wrong for not wanting spam filters to reject mail from my Drupal installation (and to not have capslock imposed on every mail out of a Drupal system)? Can this at least be made configurable or easier to override without patching core?

I've attached a patch to remove the forced caps. I'd be willing to make it configurable, if there were some indication it'd be included, and if given feedback on where such an option should be added.

If I'm filing against the wrong component, please let me know.

swelljoe’s picture

swelljoe CreditAttribution: swelljoe as a volunteer commented
Title: mail.inc use of strtoupper for URLs trigger spam filters » MailFormatHelper.php or mail.inc use of strtoupper for URLs trigger spam filters
mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented
Status: Active » Needs review

Makes sense. Triggering testbot.

mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented
FileSize
keep_case_in_plain_text_mail-2414019-4-d8.patch1.72 KB

Completely removed the $casing variable/concept.

mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented
Version: 8.0.0-rc4 » 8.4.x-dev
mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented
Status: Needs review » Needs work
mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented
Status: Needs work » Needs review
mpp’s picture

mpp CreditAttribution: mpp as a volunteer and at AmeXio commented

The last submitted patch, 1: MailFormatHelper.php-nocaps.patch, failed testing. View results

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented
Issue summary: View changes
Issue tags: +Bug Smash Initiative, +Needs reroll
  1. Title needs changing as mail.inc does not exist anymore.
  2. Applying IS template.
  3. Tagging "Bug Smash Initiative" and "Needs reroll"
jungle’s picture

jungle
He/Him
Location Chongqing, China
CreditAttribution: jungle as a volunteer commented
Status: Needs review » Needs work
Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2414019-18.patch1.65 KB

Re-rolled against 9.1.x , kindly review.

Status: Needs review » Needs work

The last submitted patch, 18: 2414019-18.patch, failed testing. View results

Hardik_Patel_12’s picture

Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 for Drupal India Association commented
Status: Needs work » Needs review
FileSize
2414019-20.patch5.13 KB
interdiff-18-20.txt3.32 KB

Solving failed test case , kindly review.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Novice

The patch still applies to 9.2.x. On a brief look the patch looks fines and there is a test. Just a few items needed to move this along.

Not all the items in #16 have been completed. Added those to the remaining tasks section of the IS. The first two items are novice level tasks, so adding 'novice' tag.

mohit_aghera’s picture

mohit_aghera CreditAttribution: mohit_aghera at QED42 commented
Title: MailFormatHelper.php or mail.inc use of strtoupper for URLs trigger spam filters » MailFormatHelper.php use of strtoupper for URLs trigger spam filters
Issue summary: View changes
Status: Needs work » Needs review

- Remove mail.inc reference from the title.
- Update the proposed solution section of the issue.

Changing status to Needs review for reviewers.

mohit_aghera’s picture

mohit_aghera CreditAttribution: mohit_aghera at QED42 commented
Title: MailFormatHelper.php use of strtoupper for URLs trigger spam filters » Use of strtoupper for URLs in MailFormatHelper.php's htmlToText() method triggers spam filters
quietone’s picture

quietone CreditAttribution: quietone as a volunteer commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Novice

@mohit_aghera, thanks!

Looking at this again, and a bit of converstaion with jibran in #bugsmash, I think this is a good idea. It seems really odd to me that we are changing the case of text constructed by someone. They took care to create the header and we should not be changing that. The addition of the indentation and either the '=' or '-' character is sufficient to notify the reader of the plain text mail that this is 'section' of text.

The patch still applies and I reviewed the code. And we can now removing $casing as it is no longer used. The test changes look fine to me.

  • catch committed 3d23f22 on 9.2.x 
    Issue #2414019 by Hardik_Patel_12, mpp, swelljoe, jungle, quietone,...

  • catch committed 78b2be2 on 9.1.x 
    Issue #2414019 by Hardik_Patel_12, mpp, swelljoe, jungle, quietone,...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Makes sense, I checked to see how long the uppercasing has been in there, and it's more than 14 years.

Committed 3d23f22 and pushed to 9.2.x. Thanks! Also cherry-picked to 9.1.x

Status: Fixed » Closed (fixed)

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