laminas / laminas-mime

Create and parse MIME messages and parts
https://docs.laminas.dev/laminas-mime/
BSD 3-Clause "New" or "Revised" License
29 stars 23 forks source link

Fold header lines with the correct length #25

Closed danielabyan closed 2 years ago

danielabyan commented 3 years ago
Q A
Documentation no
Bugfix yes
BC Break yes
New Feature no
RFC yes
QA no

Description

According to the RFC 2822 specification (clause 2.1.1), the header length should be no more than 78 characters.

Each line of characters MUST be no more than 998 characters, and SHOULD be no more than 78 characters, excluding the CRLF.

According to the specification, the length of the ENTIRE LINE should not exceed 78 characters. Currently, the laminas-mime library only counts the length of a value of the header.

Reproducing the problem

$message = new Message();
$message->setEncoding('UTF-8');
$message->addFrom('matthew@example.org', 'Matthew Somelli');
$message->addTo('foobar@example.com');
$message->setSubject('xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx');
$message->setBody('This is the message body.');

echo $message->toString();

This code will return the following result.

Date: Thu, 21 Oct 2021 10:37:01 +0000 From: =?UTF-8?Q?Matthew=20Somelli?= matthew@example.org To: foobar@example.com Subject: =?UTF-8?Q?xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20?= =?UTF-8?Q?xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx?=

This is the message body.

As you can see from the example, the header is longer than 78 characters.

New code behavior

This commit has fixed this issue. In this pool request, the code above will return the following result.

Date: Thu, 21 Oct 2021 10:38:56 +0000 From: =?UTF-8?Q?Matthew=20Somelli?= matthew@example.org To: foobar@example.com Subject: =?UTF-8?Q?xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20?= =?UTF-8?Q?xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx?=

This is the message body.

Now, the title does not exceed 78 characters.

Note

I will wait until this pool request is merged to open my pool request on laminas-mail

glensc commented 3 years ago

Perhaps it's just better to add a new api that can take headerline or headerkey,headervalue pair as input?

or maybe that is not needed if the caller just passes headerline instead of headervalue as input? it may break headerkeys that are longer than 998 bytes, but perhaps caller should handle (throw) such edge cases themselves?

danielabyan commented 3 years ago

@glensc I was considering passing the whole string, but there is a problem here. If we pass the entire line, the header name will be encoded as well. But according to the RFC 2822, only the value (body) of the header must be encoded. For example, this code

$mime = Mime\Mime::encodeQuotedPrintableHeader(
    'Subject: xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx xxxxx',
    'UTF-8',
    78
);

var_dump($mime);

Will return a result like this

=?UTF-8?Q?Subject:=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20?=
 =?UTF-8?Q?xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx=20xxxxx?="

As you can see, in this case, the whole line breaks. So, I made the changes to be minimal and not to break past behavior.

danielabyan commented 3 years ago

@Ocramius Do we have any updates? I just need this change to improve my mailing since my emails have long lines than recommended in RFC.

danielabyan commented 2 years ago

@Ocramius Sorry for writing a lot about this, but can we release it if everything is fine with this request?

Ocramius commented 2 years ago

Given my low RFC-fu here, perhaps either @Xerkus or @glensc can give a green light here.

Code-wise, it is fine for me.

Xerkus commented 2 years ago

Makes sense to me. Spec does speak about line length irrespective to the structured content of the line.

danielabyan commented 2 years ago

I have made all the changes in the code to meet the requirements of the code sniffer. But now I have a problem with composer. Please check this problem.
image As you can see the problem lies in the new security policy in Composer 2.2. This problem can be fixed if I add this configuration to the "composer.json" file

   ...
    "config": {
        "sort-packages": true,
        "allow-plugins": {
            "dealerdirect/phpcodesniffer-composer-installer": true
        }
    },
   ...

@Ocramius can I do this in this pull request or should I open a separate pull request?

Thank you.

Ocramius commented 2 years ago

@danielabyan from my point of view, you can do it here :+1:

danielabyan commented 2 years ago

@Ocramius I fixed all the bugs and issues that were in this pool request. Please check my work, and if everything is correct, please create a new release, as some mail services refuse to deliver my emails due to RFC complaints. Thanks a lot.