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.
Comment | File | Size | Author |
---|---|---|---|
#8 | encoding_header_received-2790563-8.patch | 661 bytes | ModernMantra |
|
Comments
Comment #2
miro_dietikerThen this is pretty critical in our architecture. :-)
Comment #3
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedComment #4
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedComment #5
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedThe 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 :-)Comment #6
Bambell CreditAttribution: Bambell at MD Systems GmbH commentedLooks good to me, but MIME encoding is entirely skipped for that field.. You also don't need those two lines.
Comment #7
miro_dietikerThe 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. :-)
Comment #8
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedAfter some investigation and testings, new line char
"\n"
is only one who is required in Received Header but not accepted inif (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
Comment #9
miro_dietikerAs 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.
Comment #10
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedComment #11
mbovan CreditAttribution: mbovan at MD Systems GmbH commented#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).
Comment #12
ModernMantra CreditAttribution: ModernMantra at MD Systems GmbH for MD Systems GmbH commentedCreated 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.
Comment #13
miro_dietikerTo 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?