Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N°4281 - Add RFC-2045-Compliant Way of Handling Filenames in Attachments #16

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

N°4281 - Add RFC-2045-Compliant Way of Handling Filenames in Attachments #16

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
51f688b
N°4281 - "Content Type" and "Content Disposition" RFC 2045-compliant…
itomig-martin Dec 14, 2022
ed5577f
Remove trim() before the final results, as it is not necessary anymore.
mraenker Dec 14, 2022
81bd7fe
Remove redundant part of comment
mraenker Dec 15, 2022
b660fb2
Moved the Regex into a class var + Testcases
itomig-martin Jan 11, 2023
610c2f3
Change Namespace in test/TestRegexAttachmentNames.php
mraenker Jan 12, 2023
f0521eb
Regex is now a const, and its uses are factorized in a dedicated method
Feb 3, 2023
5dc779c
Merge branch 'Combodo:master' into issue/4281
mraenker Feb 6, 2023
541bf57
:art: Fix formatting
Feb 6, 2023
3158cf5
:art: Fix formatting again
Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 59 additions & 51 deletions classes/rawemailmessage.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,19 +67,21 @@ class RawEmailMessage
protected $iNextId;

/**
* @var string This Regex complies with RFC 2045 regarding the Grammar of
* @var string This Regex complies with RFC 2045 regarding the Grammar of
* Content Type Headers Filenames:
* (1) Allow all chars for the filename, if the filename is quoted with double quotes
* (2) Allow all chars for the filename, if the filename is quoted with single quotes
* (3) If the filename is not quoted, allow only ASCII chars and exclude the following
* chars from the set, referenced as "tspecials" in the RFC:
* <ALL-CTL-CHARS-INCL-DEL>
* <CHAR-SPACE>
* ()<>@,;:\"/[]?=
* To keep the regex as simple as possible, the _allowed_ chars are listed
* with their corresponding hexval.
*/
public static $sFileNameRegex = <<<REGEX
* (1) Allow all chars for the filename, if the filename is quoted with double quotes
* (2) Allow all chars for the filename, if the filename is quoted with single quotes
* (3) If the filename is not quoted, allow only ASCII chars and exclude the following
* chars from the set, referenced as "specials" in the RFC:
* <ALL-CTL-CHARS-INCL-DEL>
* <CHAR-SPACE>
* ()<>@,;:\"/[]?=
* To keep the regex as simple as possible, the _allowed_ chars are listed
* with their corresponding hexval.
*
* @since 3.7.1 N°4281
*/
public const FILENAME_REGEX = <<<REGEX
(("([^"]+)")|('([^']+)')|([\x21\x23-\x27\x2A\x2B\x2D\x2E\x30-\x39\x41-\x5A\x5E-\x7E]+))
REGEX;

Expand Down Expand Up @@ -190,61 +192,51 @@ public function GetMessageId()
public function GetAttachments(&$aAttachments = null, $aPart = null, &$index = 1)
{
static $iAttachmentCount = 0;
if ($aAttachments === null)
{
if ($aAttachments === null) {
$aAttachments = array();
}
if ($aPart === null)
{
if ($aPart === null) {
$aPart = $this->aParts;
} //Init for recursion

if ($aPart['type'] == 'simple')
{
if ($this->IsAttachment($aPart['headers']))
{
$sFileName = '';
if ($aPart['type'] == 'simple') {
if ($this->IsAttachment($aPart['headers'])) {
$sFileName = '';
piRGoif marked this conversation as resolved.
Show resolved Hide resolved
$sContentDisposition = $this->GetHeader('content-disposition', $aPart['headers']);
if (($sContentDisposition != '') && (preg_match('/filename='.self::$sFileNameRegex.'/', $sContentDisposition, $aMatches)))
{
$sFileName = end($aMatches);
if ($sContentDisposition != '') {
$sFileName = static::GetAttachmentFilename($sContentDisposition);
}

$bInline = true;
if (stripos($sContentDisposition, 'attachment;') !== false)
{
if (stripos($sContentDisposition, 'attachment;') !== false) {
$bInline = false;
}

$sType = '';
$sType = '';
$sContentId = $this->GetHeader('content-id', $aPart['headers']);
if (($sContentId != '') && (preg_match('/^<(.+)>$/', $sContentId, $aMatches)))
{
if (($sContentId != '') && (preg_match('/^<(.+)>$/', $sContentId, $aMatches))) {
$sContentId = $aMatches[1];
}
else
{
} else {
$sContentId = 'itop_'.$iAttachmentCount;
$iAttachmentCount++;
}
$sContentType = $this->GetHeader('content-type', $aPart['headers']);
if (($sContentType != '') && (preg_match('/^([^;]+)/', $sContentType, $aMatches)))
{
if (($sContentType != '') && (preg_match('/^([^;]+)/', $sContentType, $aMatches))) {
$sType = $aMatches[1];
}
if (empty($sFileName) && preg_match('/name='.self::$sFileNameRegex.'/', $sContentType, $aMatches))
{
$sFileName = end($aMatches);
if (empty($sFileName)) {
$sContentTypeFilename = static::GetAttachmentFilename($sContentType);
if ($sContentTypeFilename !== '') {
$sFileName = $sContentTypeFilename;
}
}

if (empty($sFileName))
{
if (empty($sFileName)) {
// generate a name based on the type of the file...
$aTypes = explode('/', $sType);
$aTypes = explode('/', $sType);
piRGoif marked this conversation as resolved.
Show resolved Hide resolved
$sFileExtension = $aTypes[1];
// map the type to a useful extension if needed
switch ($aTypes[1])
{
switch ($aTypes[1]) {
case 'rfc822':
// Special case for messages: use the .eml extension
$sFileExtension = 'eml';
Expand All @@ -253,25 +245,41 @@ public function GetAttachments(&$aAttachments = null, $aPart = null, &$index = 1
$sFileName = sprintf('%s%03d.%s', $aTypes[0], $index, $sFileExtension); // i.e. image001.jpg
}
$aAttachments['part_'.$aPart['part_id']] = array(
'filename' => $sFileName,
'mimeType' => $sType,
'filename' => $sFileName,
'mimeType' => $sType,
'content-id' => $sContentId,
'content' => $this->DecodePart($aPart['headers'], $aPart['body']),
'inline' => $bInline,
'content' => $this->DecodePart($aPart['headers'], $aPart['body']),
'inline' => $bInline,
);
}
}
else
{
foreach ($aPart['parts'] as $aSubPart)
{
} else {
foreach ($aPart['parts'] as $aSubPart) {
$aAttachments = array_merge($aAttachments, $this->GetAttachments($aAttachments, $aSubPart, $index));
}
}

return $aAttachments;
}

/**
* Allow to handle RFC 2045 filename syntax in attachments
*
* @param string $sHeaderValue
*
* @return string empty string if none found
* @uses static::FILENAME_REGEX
*
* @since 3.7.1 N°4281
*/
public static function GetAttachmentFilename($sHeaderValue)
{
if (preg_match('/name='.static::FILENAME_REGEX.'/', $sHeaderValue, $aMatches)) {
return end($aMatches);
}

return '';
}

/**
* Create a new RawEmailMessage object by reading the content of the given file
*
Expand Down
9 changes: 3 additions & 6 deletions test/TestRegexAttachmentNames.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,10 @@ public function testNormalizeAttachmentName(string $sInput, string $sExpectedAtt
{
$aMatches = array();
$sNormalizedAttachmentName = null;
if (preg_match(RawEmailMessage::$sFileNameRegex, $sInput, $aMatches))
{
if (preg_match(RawEmailMessage::FILENAME_REGEX, $sInput, $aMatches)) {
$sNormalizedAttachmentName = end($aMatches);
$this->assertEquals($sExpectedAttachmentName, $sNormalizedAttachmentName, "Attachmentname for '".bin2hex($sInput)."' doesn't match. Got'".bin2hex($sNormalizedAttachmentName)."', expected '$sExpectedAttachmentName'.");
}
else
{
} else {
$this->AssertNull($sNormalizedAttachmentName);
}
}
Expand All @@ -53,7 +50,7 @@ public function AttachmentFilenameProvider()
"!#$%&'*+-.0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz{|}~",
],
// Something is odd here: x2C (comma) is not in the list of allowed
// chars in RawEmailMessage::$sFileNameRegex, but nevertheless is
// chars in {@see RawEmailMessage::FILENAME_REGEX}, but nevertheless is
// not filtered by the Regex, resulting in this test case is failing.
// I have no clue, as to why this is happening.
'All not allowed Chars (from the ASCII Table)' => [
Expand Down