Skip to content

Commit

Permalink
Check if attachment is actually(!) referred to (#9585)
Browse files Browse the repository at this point in the history
* Check if "inline" msg part is actually referred to

If there's no reference to it in a sibling HTML part then we handle it
as a classic attachment (which is shown as downloadable).

* Fetch all msg headers also for images to always get Content-Location

Previously all headers were only fetched for message/rfc822, or
if the Content-Type's "name" parameter was set, or if a Content-ID was
set.
The RFC doesn't require neither the "name" parameter nor a Content-ID
for using Content-Location, though, so we shouldn't depend on those.

Instead now all headers are also fetched if the main part of the
Content-Type is "image", to catch more cases.

* Parse HTML for references only on demand

* Typos and comment formatting

* Don't skip test anymore

We want it tested!

* More MR tests with images

* Remove early special handling for "inline" images

We decide later, which attachment is considered "inline" and which
isn't.

* Remove early resolving of references in TNEF parts

* Testing message rendering of TNEF emails

* Don't use image disposition, it's unreliable

* Split adding raw parts and attachments

* Fix renaming variable

* Rename file to make its test be run

* Remove outdated script

* Annotate test cases with GitHub issue numbers

* Fix test case class name

* remove comment

* Test inline image message rendering

* Rename test file to reflect cases better

* Reduce image used in test email

It doesn't change much, but there's also no sense in decoding big images
that we don't use.

* Remove unused variable initialisation
  • Loading branch information
pabzm authored Feb 9, 2025
1 parent 49d8639 commit 752b152
Show file tree
Hide file tree
Showing 13 changed files with 2,652 additions and 131 deletions.
4 changes: 2 additions & 2 deletions program/lib/Roundcube/rcube_imap.php
Original file line number Diff line number Diff line change
Expand Up @@ -2230,11 +2230,11 @@ protected function structure_part($part, $count = 0, $parent = '', $mime_headers
}
}

// fetch message headers if message/rfc822 or named part (could contain Content-Location header)
// fetch message headers if message/rfc822 or image or named part (could contain Content-Location header)
if (
empty($mime_headers)
&& (
$struct->ctype_primary == 'message'
$struct->ctype_primary == 'message' || $struct->ctype_primary == 'image'
|| (!empty($struct->ctype_parameters['name']) && !empty($struct->content_id))
)
) {
Expand Down
201 changes: 119 additions & 82 deletions program/lib/Roundcube/rcube_message.php

Large diffs are not rendered by default.

34 changes: 34 additions & 0 deletions tests/MessageRendering/InlineImageTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<?php

namespace Tests\MessageRendering;

/**
* Test class to test simple messages.
*/
class InlineImageTest extends MessageRenderingTestCase
{
public function testImageFromDataUri()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('trinity-eb9e559b-1926-4b09-990d-80e9da9a9c35-1723163091112@3c-app-mailcom-bs14');

$this->assertSame('***SPAM*** wir gratulieren Ihnen recht herzlich.', $this->getScrubbedSubject($domxpath));

$divElements = $domxpath->query('//div[@class="rcmBody"]/div/div');
$this->assertCount(3, $divElements, 'Body HTML DIV elements');

$this->assertSame('wir gratulieren Ihnen recht herzlich.', $divElements[0]->textContent);

$img = $divElements[1]->firstChild->firstChild;
$this->assertSame('img', $img->nodeName);
$src = $img->attributes->getNamedItem('src')->textContent;
$this->assertStringContainsString('?_task=mail&_action=get&_mbox=INBOX&_uid=', $src);
$this->assertStringContainsString('&_part=2&_embed=1&_mimeclass=image', $src);

$this->assertSame('v1signature', $divElements[2]->attributes->getNamedItem('class')->textContent);
// This matches a non-breakable space.
$this->assertMatchesRegularExpression('|^\x{00a0}$|u', $divElements[2]->textContent);

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(0, $attchNames, 'Attachments');
}
}
55 changes: 55 additions & 0 deletions tests/MessageRendering/SingleAttachedImageNoTextTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

namespace Tests\MessageRendering;

/**
* Test class to test "interesting" messages.
*/
class SingleAttachedImageNoTextTest extends MessageRenderingTestCase
{
/**
* Test that of a multipart/mixed message which contains only one
* image, that image is shown. (GitHub issue #9443)
*/
public function testShowMultipartMixedSingleImageToo()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$this->assertSame('Not OK', $this->getScrubbedSubject($domxpath));

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(1, $attchNames, 'Attachments');
$this->assertStringStartsWith('Resized_20240427_200026(1).jpeg', $attchNames[0]->textContent);
}

/**
* Test that an image, that has a Content-ID, which is not accompanied by
* an HTML part (and thus is not referred to), is shown as attachment.
* (GitHub issue #9565)
*/
public function testShowUnreferredToImagesWithContentId()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$this->assertSame('test', $this->getScrubbedSubject($domxpath));

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(1, $attchNames, 'Attachments');
$this->assertStringStartsWith('тест.jpg', $attchNames[0]->textContent);
}

/**
* Test that an image, that has a Content-ID, but is not referred to in the
* accompanying HTML-part, is shown as attachment. (GitHub issue #9685)
*/
public function testShowUnreferredToImagesWithContentIdInMultipartAlternative()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$this->assertSame('Multipart/alternative with attached but unreferenced image', $this->getScrubbedSubject($domxpath));

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(1, $attchNames, 'Attachments');
$this->assertStringStartsWith('Stg Aiki - SB - 22-23 Fév 2025.jpg', $attchNames[0]->textContent);
}
}
29 changes: 0 additions & 29 deletions tests/MessageRendering/SingleImageNoTextTest.php

This file was deleted.

55 changes: 55 additions & 0 deletions tests/MessageRendering/TnefEmailsTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

namespace Tests\MessageRendering;

/**
* Test class to test "interesting" messages.
*/
class TnefEmailsTest extends MessageRenderingTestCase
{
public function testTnefEmail1()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$this->assertSame('', $this->getBody($domxpath));

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(1, $attchNames, 'Attachments');
$this->assertStringStartsWith('AUTHORS', $attchNames[0]->textContent);
}

public function testTnefEmail2()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$this->assertStringStartsWith('THE BILL OF RIGHTSAmendments 1-10 of the', $this->getBody($domxpath));

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(0, $attchNames, 'Attachments');
}

public function testTnefEmail3()
{
$domxpath = $this->runAndGetHtmlOutputDomxpath('[email protected]');

$bodyParagraphs = $domxpath->query('//div[@class="rcmBody"]/p');
$this->assertCount(8, $bodyParagraphs, 'Body HTML paragraphs');
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[0]->textContent);
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[1]->textContent);
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[2]->textContent);
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[3]->textContent);
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[4]->textContent);
$this->assertSame(' ', $bodyParagraphs[5]->textContent);
$this->assertSame('Casdasdfasdfasd', $bodyParagraphs[6]->textContent);
$this->assertSame(' ', $bodyParagraphs[7]->textContent);

$attchNames = $domxpath->query('//span[@class="attachment-name"]');
$this->assertCount(2, $attchNames, 'Attachments');
$this->assertStringStartsWith('zappa_av1.jpg', $attchNames[0]->textContent);
$this->assertStringStartsWith('bookmark.htm', $attchNames[1]->textContent);

$inlineShownImages = $domxpath->query('//p[@class="image-attachment"]/span[@class="image-filename"]');
$this->assertCount(1, $inlineShownImages, 'Inline shown images');
$this->assertSame('zappa_av1.jpg', $inlineShownImages[0]->textContent);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
From: <[email protected]>
To: <[email protected]>
Date: Wed, 1 Jul 2024 12:00:00 +0000
Subject: Multipart/alternative with attached but unreferenced image
Message-Id: <[email protected]>
Content-Type: multipart/mixed; boundary="00000000000000f0c80629385fe7"

--00000000000000f0c80629385fe7
Content-Type: multipart/alternative; boundary="00000000000000f0c30629385fe5"
--00000000000000f0c30629385fe5
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Hugs !
---------- Forwarded message ---------
De : Sender name <[email protected]>
Date: ven. 13 d=C3=A9c. 2024 =C3=A0 20:58
Subject: Stage 222 23 f=C3=A9vrier 2025
To: Recipient name <[email protected]>

--=20
=E7=84=A1=E5=BE=97=E4=BC=9A INTERNATIONAL ADDRESS: http://mywebsite.org

"Bienheureux les f=C3=AAl=C3=A9s, car ils laisseront passer la lumi=C3=A8re=
. "
Michel Audiard=EF=BB=BF

--00000000000000f0c30629385fe5
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_default" style=3D"font-family:verdana,=
sans-serif;font-size:large">Hugs !<br></div><br><div class=3D"gmail_quote g=
mail_quote_container"><div dir=3D"ltr" class=3D"gmail_attr">---------- Forw=
arded message ---------<br>De=C2=A0: <b class=3D"gmail_sendername" dir=3D"a=
uto">Sender Name</b> <span dir=3D"auto">&lt;<a href=3D"mailto:sender=
[email protected]">[email protected]</a>&gt=
;</span><br>Date: ven. 13 d=C3=A9c. 2024 =C3=A0=C2=A020:58<br>Subject: Stag=
e 222 23 f=C3=A9vrier 2025<br>To: Previuos recipient &lt;<a href=
=3D"mailto:[email protected]">[email protected]</a>&gt;<br></div><br><b=
r><div dir=3D"ltr"><br></div>
</div><div><br clear=3D"all"></div><br><span class=3D"gmail_signature_prefi=
x">-- </span><br><div dir=3D"ltr" class=3D"gmail_signature" data-smartmail=
=3D"gmail_signature">=E7=84=A1=E5=BE=97=E4=BC=9A =C2=A0INTERNATIONAL ADDRES=
S: <a href=3D"http://mywebsite.org" target=3D"_blank">http://mywebsite.org<=
/a><br><br>&quot;Bienheureux les f=C3=AAl=C3=A9s, car ils laisseront passer=
la lumi=C3=A8re.=C2=A0&quot;<br>Michel Audiard=EF=BB=BF<br><br><br><br><br=
><br><br></div></div>

--00000000000000f0c30629385fe5--
--00000000000000f0c80629385fe7
Content-Type: image/jpeg; name="=?UTF-8?B?U3RnIEFpa2kgLSBTQiAtIDIyLTIzIEZlzIF2IDIwMjUu?=
=?UTF-8?B?anBn?="
Content-Disposition: attachment;
filename="=?UTF-8?B?U3RnIEFpa2kgLSBTQiAtIDIyLTIzIEZlzIF2IDIwMjUuanBn?="
Content-Transfer-Encoding: base64
Content-ID: <f_m4n674et0>
X-Attachment-Id: f_m4n674et0
/9j/4AAQSkZJRgABAQEASABIAAD/2wBDAAUDBAQEAwUEBAQFBQUGBwwIBwcHBw8LCwkMEQ8SEhEP
ERETFhwXExQaFRERGCEYGh0dHx8fExciJCIeJBweHx7/2wBDAQUFBQcGBw4ICA4eFBEUHh4eHh4e
Hh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh4eHh7/wAARCABuAGQDASIA
AhEBAxEB/8QAHAAAAgMBAQEBAAAAAAAAAAAABQYABAcDAgEI/8QAPBAAAgECBAQDBAkDAgcAAAAA
AQIDBBEABRIhBhMxUSJBYRQycYEHFSNCYpGhscEWUtFT8AgzgoSS4fH/xAAaAQADAQEBAQAAAAAA
AAAAAAABAgMEAAUG/8QAIxEAAgICAQQDAQEAAAAAAAAAAAECEQMSIQQTMUEiUXEFYf/aAAwDAQAC
EQMRAD8AWYoIKji2gRtWlaOpa+srbdMHvYoEYsskoA2uJmP84WJZJBxVRx09MKhqmgnREJayjUlz
t0+OCLHMKZBzFpYza2kSMxPqTptfHwOWNqP4fQphSWmQAHXUEnpeVt/1xwenUbF6jfoOY2364A1m
aZjTMRyIXXqNLPv89OKEOb8QVFTHS0eUTTu72jWOQjUT6legwscLfhhchspcsqqycU1GJ5Zieits
L9Lnyxdk4br6SZ4JDaoUi6h1INx3tgtk2V1uUZapzdFirHOubRJqVNvdBHUgW374K5zNJUU1DVIC
JZG5TX/vHS+M0pNOhlyKdHw3nsy/Y0coUbapHWxPpj7UcO59TQmWSgYqBclQGv8Ak2H6izKGqi5c
loatQA6Ntqt37jtbFyKoE0bBUJ5Z0+AdD2wHNvwEx5WBmEBZQ4Juu6m/b448sVPuDSQbk6r3ww/S
/lcFPlX9QU9O4qIWCz8tRqlQ9CPgcZnQ51OTHzoJmkB8TLSkX+FjjTjx7xtCOVMbFlJYENcgnEWU
sbg2IG4JwDNczsDy6r1Psz7/AKY6rmka3RoakHuYXH8YMsTR2yDK1E6iyByPOzDriYCpmsQBH2nX
zjf/ABiYXVg2R5jnJ4yonUaeRl1St+tzrTFs1877KymxsMCnNR9ehIVleWSikClSth9onfF6lkqa
UBPq8SgbFjKlz67Ni+VKWv4BF1qqSRbPKbgbhALjGlfRrkUVFR/WdQperqRdS6+4nlt5G2MxWsk5
gZ6B1DMB76mw26DVjcstKCjg5W8ZjXSe4tjHmeqGSspcTwiTLahSN9ive9thheEMtbRlUl5eoAya
jbS69CME88rI4c6ioqhiErAViY+7zF3K37lWFh1NjiJR3JEYGht+vfGdKlY5Tnp/aaqNUKvKAGYr
/vphf4roJqzjTLaGk1NLNlshBStkiaCRZFAkTT97frvcA3w6SiKioGlneGGOBftHc7ab3uT5W6YW
eDoKjiLi2p4raB4aAxiny1pAVLxKSWfT+JjqHoo740YvinNiy8nv6U556f6MqmPMJYZaqSNImMSk
KX1ixsd77HGKUktTrB1L6b4df+IDiSnqMypOHoJfDTjnVOgMRc+4u3mPEbeoxnWW1qqdcxVWvtpV
zj0OnxSjj/SMqsaIJJyoJY3wSpWltcbt64C02YUlgzSp843/AMYJwZnQHxCdFIH4hiM1K+R0kEA1
QPIYmKa5nQsLrMPX44mEphpEyYJJn80msKUo9KepMg/fBxlicKUVSP3wrw08lXxB7NFFER7OZCXd
wV0uNNiDgsFzWLYCnYeYu9j874OROzohCSKM6rqo22JXDhwPxPTinhymumEcikLDK50q6jYIb9CP
1xnU8tcnialoyQL257jFSqkzeop5Zo8spxFGuppOcSCPQlbYTsufDYbo3jiPI8vziikpayJyCAyl
mKsh8nBG4I8rYVFyPjWhk9loOKqeWnPuGuohMw9CwZSfiQScZHTcd8V5LyIKCpZInJ+zlkEiKANX
mPX4YeeG+JeNcxy6lrjWUh5ilnPsq7Nfph+xkwoXfYa4+CjmRim4uzuozoqRookiWClBBuCUXeS3
4jt2xY4o4hipwMqyxEaqdWUMo+zittvYAEjsOmM/kzrinMc6rKCqzAvHGA0YpnWAWuffspJHzGCv
CMbT0cCSsqclSG0dA3mB6dvTE8rettjIx3P46qnz6tSvkeSraY8yWTrISb6sEMqk12ICqCbHYG2N
2ThTKOI6h4a+KmqkYeC62Km3UEeeBcH0F0NPHRzpnWYmnq6hI4oxFEzAuuoWNu3fGuHVQnCiTjry
ImVwpKbRyJpHUnBMoIyAniA2uFFsF/pH+jnMOAqdc1jnnr8oP2busQM0BJ6uF2Zb7XHTCd9f5fpF
oKyS3UmmItiLhKXMRk1Qf5rpsNPfp/7xMBDnlIdxBUj/ALdv8YmE7eQOyJkElPHnklRUTJEEotJL
sAL80d8MUkcbAFSLfvhWy1Gqs9np4+Qsb0ZaTmQazfmbWBwfjjzGSUL7RS6QfHppith/5YbKuTkS
opo2D6dKuiamIO4F+2Buf5g0eVZjRuzurRGyDw7FbA37nYEeWGlpaCko1WoKKvMZVYptIO+EXih1
zKr00aESMQpZjYA/dPwsR+WFxJbJs52U5KOjzCXmD7cLGIwVYEh30gdOnunr3w98GR0GSpFw1TCT
Pa5VM9Y/M0U1KCNwzruxHboThdosooKSiRqt6hl8DSL3VTtYDfDtR5hkWWUF8qaFqQrzFBlVRO59
xWIOrSOrE9PPFc07VLk5RoL0tBSwuWTLqSOaVdkjgVC4PU2tdV9W644f0+sdXdY0HKbYxeErft5D
54FR55kFbGY/6gpoY3a5V6tVepk82axuq9h5jEhosroalIIEraN5wZKeSnqXMMmkdFkvs3nZuuMf
bmhrSDdFR12Qc3MKOVeSnjmWaUhVUXuT2GC2XfSNl9dwvkUiiSj9nzOOOUCW6HQjg2Fr2K9PljJ+
L67PKatgymsziatyqtDSxFgqOShu0b2NzpNj63OKXJSUUiKqq/NDQkk2LW627DucaFh0V/YrlfB+
iKriGh4wy6thqJIvq2ZDAyjo1ha4+HX42x+eq7L5KHMqiiedXFNKyatxrAOx+Y3wdyWtko6L2FJm
MkV1lisb6vTvgHx9ULSVtLmjRytzzyJVji3Vxvv8tvlhcSyOVBdJHZY5NIvNe/riYDQcQ0BiGtK4
t5nkH/GJjT2cgm0QnwvHrzasmPVaIAeIf6hwyzQSTRkWQItjfoQe/rhKpqqWmz008NliakBe43Fp
D1/M4vz8RtDEyRcyrlO1wNKJ6Fj+wxKeKUnwMmkGGpRPUTPPokVhpDMd9v7R5YB1tblmWSFIhGZy
3/LBufmf5wGrcyzTMCKFKtllm3VIFKoFB8Wonpbvj2cnhpKcopGoteR3uzP/AL9OmKxxKK+TFc/o
p1+bV1aXFWoZTZBEl9It5m3XENdLAqh1hgAWyokWkW/fFtmejcpEEIYe85F8AswEk9Wl2LM5NiOh
72xWNPhCNsuLGk4aZAjwKLKswW9/P1wZyLNa/K6RqCkmqEpnZTKslnjuOwPQ+VxvheiL07CMsWUA
3APrgvAh0O6KH1fi6nAn9MKbLXEVTV5vDTibMEM9M/NVjELvIF03Y+epbD5XxyoKmpcM1VeMhrB/
9Nh1B7DscfRHVb3hUggHY2OB2a1bUFUKuSOZYDZKqMrquPJx6jpb546K7i1O9jZWnnUbVCzPHOUC
MFP3b3/O9/livn6tNlFTRCMvLTRCqSQHdl2Nz626/HFPJK+KPTFIddNLcREMDpHnG3YHrc4aFpUm
pqjWEZoIRGbix02a9/z/AGxnaeKXJRK0IFLVURhDFlBO9m64mA1aktHWT0knPvDIU1CE+Kx69PPE
x6Hbb5Jjq1EKivq35qQyxUAaPWwAe8hNr4W6rMUEmlo5JZF2WGKxI+J6gDDfk8dZ/Uk1NDVNDD9X
h3GhGuebYXDD1x3rVrZ5Up6msSSCByQhhCjX5e6On6Yyqag+R9bBGU0vsVMJZkLVEyqz2N7ADZPQ
evnizHJS+44YsT5HVf49sXfYUmX7dmk89IYqfji8mVZWtOJI+dGxW5Mm6sfjiE5qTsOoErY4hKsY
iA1LfxYCyxwU1XSwWRpSXYgDyPrhiaFJJyjqF2632PwwFzsxRZnTBFA9nFz5lgdgPXfFMcuRZIqR
QLJOsSxGM2BN8EI4I4I3Mk2nSwIvsOmO+XQwLCHdVaY21nqSfPBd4YWQPHADqXbUMLLJyFRFyaYI
RokJN98dBLFMo1EEnZgfPt1xYq6bSjqyRhR4h4BqscCpEhiUXd1JNgSNsMm/KB4KWaVK0M8UsKsu
tgs8JuqsL9bdCSMOHDdZLFCIAwmppY7o39tvun4YW6+laWjZIdDF0sCy9G8jfywNy7Ma2CgNI1LO
jE2YoAfy7Y0Tx9yP+gTovZ3rrc4q6iGUrGZSq+I722viYqQzz6PFT1g320oLW/PEwdZL2dsh34Re
KKorqiRuY60MUafiZpGJ/K2LRVox4yWvbzNyccsspIoIhUh31tGoOwIAW9hb/qxdhjM04V5CLeYx
5+SXNlYFikdtLGVLgHqD0xelZUon3UEIQLjbHHlQQ0csziSQg9C1sUZIqllKxNEvmQ92G+JDHiMQ
nfWr3BIIwg8fNTUhWoNQUrKnwrHGoLMegAHXrhxqfbkpJHNZyKZFZ+XBGLsVFzcnyv0HljEcx4sz
WbPqjN8uqZKV7aInuDIqjYXJBsbdrY9b+d0ksktn4M/UZFHg0jhKtmqoFhqaRqXMY4ws0bjxEDo4
PS2GamqpkQgqDcn7u+MKHF3Er1kNZUZ5mNVJC+oCWpcjbYjcm4Ixs2TcQx5vldJWU1Nymlj1Okvi
CsG+6b3th+v6F4nuvAmDLfDKlZWRM8h0nWxsf/mB80qSEWYFf5wZzIs6nmadSkbKNht5YWY4DUSK
4KxNqcBlFyTfa/pjHipoo3yE6QO94rJp7nFOX2R8zqKRJSJ4Qjyr6EbWwQo4nDr7QE1C4Om5Bt6H
p8MAMurIKXiDNKmaJneeXlrpsNKhbD4b74vCKkpX6BL0HIaNNGz7fHExbpM6y0QgGGq69wf5xMY3
LkOqP//Z
--00000000000000f0c80629385fe7--

Loading

0 comments on commit 752b152

Please sign in to comment.