Received Header according to RFC822, if possible 'should' contain proper indentation and new lines after each subfield. Problem is that if you add new received header in inmail_module in function inmail_mail , it triggers \Drupal\iinmail\MIME\Header::mimeHeaderEncode() and converts Received Header into unexpected string. Furthermore, lines in the mentioned function are wrapped after 78 characters.
To be more clear, function mimeHeaderEncode() makes encoding for any char between 20(hex value) and 7E, and there is no 'mistakes/bugs' at all in that logic. Since we are adding new header (Received), such a filter can not be applied in this case since it involves "\t" and "\n". So, current API should be extended in order to support new line and tab characters without risk to break the header encoding.


Proposed Solution

Create new Received Header in inmail.module:

$host = \Drupal::request()->getHost();
$ip = \Drupal::request()->getClientIp();
$header_to = preg_replace('/.*<(.*)>/', '\1', $original->getTo());
$time = date_create()->format('r');
$original->getHeader()->addField('Received', 'by ' . $host . ', ' . $ip . ' for <' . $header_to . '>; ' . $time, TRUE);

Possibly override function Header::mimeHeaderEncode() to skip encoding for header Received.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ModernMantra created an issue. See original summary.

miro_dietiker’s picture

Priority: Normal » Critical

Then this is pretty critical in our architecture. :-)

ModernMantra’s picture

Title: Function for Encoding triggered, but it should not » Encoding Function should accept wider range of chars
Issue summary: View changes
ModernMantra’s picture

Issue summary: View changes
ModernMantra’s picture

Assigned: Unassigned » ModernMantra
Status: Active » Needs review
FileSize
1.78 KB

The logic behind the uploaded patch is that Header Received is simply isolated from processes of encoding and preg replace. preg_replace is skipped because of case that space after comma is inserted, preg_replace function adds new line which is not desired (and it is against of some RFC822 Received rules). Hope the patch is in good direction :-)

Bambell’s picture

Status: Needs review » Needs work
+++ b/src/MIME/Header.php
@@ -159,16 +159,25 @@ class Header {
+      $encoded = '';
+      $field_string = '';

Looks good to me, but MIME encoding is entirely skipped for that field.. You also don't need those two lines.

miro_dietiker’s picture

The Received header follows the header encodings and definitions as much as every other header. The standard doesn't define a special case for the Received header.

So if this is a bug, we need to modify our processing in a way that it works for all headers. We can not skip proper encoding for a random header as it would lead to other fun problems.

The problem looks to me like you are passing in some special characters into the Received header, forcing it to do the encoding - and that's perfectly right.
See:
if (preg_match('/[^\x20-\x7E]/', $string)) {
mimeHeaderEncode (called by ) only encodes special characters.

So please investigate the reason and fix it, instead of adding a specific workaround. :-)

ModernMantra’s picture

Status: Needs work » Needs review
FileSize
661 bytes

After some investigation and testings, new line char "\n" is only one who is required in Received Header but not accepted in if (preg_match('/[^\x20-\x7E]/', $string)) (reason is that new line is 0A in hex, which is out of range 20...7E, thus it triggered encoding). In the patch fixes the issue explained previously.
No interdiff since previous patch was not appropriate

miro_dietiker’s picture

Status: Needs review » Needs work

As discussed, allowing a newline and not encoding it leads to header field injection capabilities in header values. Our API simply doesn't allow multiline headers and we first need to define how we can support this without risk to break the header encoding.

First, let's just fix the Received header we save to avoid it is encoded and better readable. I guess we simply need to remove \n and \t. The line will be wrapped at 78 chars by the header output as long as the hostname (and from) doesn't contain any special chars.

As a follow-up we can discuss how we can allow headers that are broken in multiple lines by intention.

Please also update the summary first.

ModernMantra’s picture

Priority: Critical » Minor
Issue summary: View changes
mbovan’s picture

Priority: Minor » Normal

#2385481: Add Received header when forwarding is postponed until we fix this one.

Last status change seemed pretty extreme, so discussed with @ModernMantra to raise the priority to Normal.

I guess the next step in this issue would be to address the feedback from #9.2?

Also, we should create a followup to discuss multiple lines headers issue (#9.3).

ModernMantra’s picture

Status: Needs work » Needs review
Related issues: +#2798305: Allow headers that are broken in multiple lines by intention

Created follow up as suggested in #9.3 https://www.drupal.org/node/2798305, and fixed issues stated in #9.2. Changed status to needs review so that someone can decide for next steps about this.

miro_dietiker’s picture

Status: Needs review » Postponed (maintainer needs more info)

To be clear, the newline is not something we want to accept as "wider range of chars".

There might be other characters that make sense in the field body without need for encoding.
But we need to check the standard and identify them.

Can someone please investigate the clean definition of RFC 2822 and the EBNF definitions for the characters?