Mail: Cannot download attachments

[ostasevych]

Expected behavior

When I click the download icon (arrow down) under the text of the received message I have to be prompted to download a file.

Actual behavior

Instead of I am getting error on the web-page indicating:

The server was unable to complete your request.
If this happens again, please send the technical details below to the server administrator.
More details can be found in the server log.

Technical details
Remote Address: xxx.xxx.xxx.xxx
Request ID: swzZpzchV0RK9SLMVSOk

In the Logging section I see:
TypeError: Argument 2 passed to OCA\Mail\Http\AttachmentDownloadResponse::__construct() must be of the type string, null given, called in /var/www/html/test/apps/mail/lib/Controller/MessagesController.php on line 257

Mail app

Mail app version: 0.14.00
Mailserver or service: local hosted Postfix+Dovecot

Server configuration

Operating system: CentOS 6.10
Web server: Apache
Database: MySQL
PHP version: 7.2.18
Nextcloud Version: 16.0.0

Client configuration

Browser: Google Chrome 74.0.3729
Operating system: Windows 10, 64-bit

[ChristophWurst]

Sounds like on some attachments the content type is either not set or unknown. A possible fix would be to set the MIME to application/octet-stream as a fallback at https://github.com/nextcloud/mail/blob/251b3b3e58c0b636e0f5346ae6d67a8c93fc39af/lib/Http/AttachmentDownloadResponse.php#L42.

@ostasevych would you be interested in contributing a fix?

[ostasevych]

Sounds like on some attachments the content type is either not set or unknown. A possible fix would be to set the MIME to application/octet-stream as a fallback at

https://github.com/nextcloud/mail/blob/251b3b3e58c0b636e0f5346ae6d67a8c93fc39af/lib/Http/AttachmentDownloadResponse.php#L42

.
@ostasevych would you be interested in contributing a fix?

Hi! Thank you for the response. How to set this MIME? I am more tester rather than programmer.... If you explain, how to set the MIME, I may test the patch.

[ChristophWurst]

I am more tester rather than programmer

No problem. Thanks for testing and writing bug reports. This is highly appreciated :raised_hands:

It's not something that you could set. Does this only happen with specific emails or with all that have attachments?

[der-peer]

Hi,
is there anything new about the issue?

I can confirm, that the problem still persists. My setup:

• Nextcloud 16.0.4, selfhosted on Apache 2.4
• Mail 0.15.2
• External User Support 0.6.4
• external Postfix Mail server with IMAP,
• Users are authenticated against IMAP, Mail accounts automatically created based on IMAP authentication.

Single attachments seem to work in most cases. Image attachments are shown inline with thumbnail and can be downloaded. PDFs or other attachments can be downloaded.

I can confirm at least this case, where I am sure that leads to the mentioned error: if the received Mail is PGP signed, then display of the attachment is broken (thumbnail can not be displayed, image not shown inline). The same email without PGP signature displays fine.

[herrep]

Hi,

I encounter the same problem with Nextcloud 16.0.4 and Mail 0.15.2.

However, when looking onto the error message and the code, it appears that the attachment file name is null:

/var/www/html/nextcloud/apps/mail/lib/Controller/MessagesController.php, lines 256-257:

AttachmentDownloadResponse(
                        $attachment->getContents(), $attachment->getName(), $attachment->getType());

The log files reads:

TypeError: Argument 2 passed to OCA\Mail\Http\AttachmentDownloadResponse::__construct() must be of the type string, null given, called in /var/www/html/nextcloud/apps/mail/lib/Controller/MessagesController.php on line 257

/var/www/html/nextcloud/apps/mail/lib/Controller/MessagesController.php - line 257:
OCA\Mail\Http\AttachmentDownloadResponse->__construct("\r\n--Apple ... n", null, "application/octet-stream")

According to my log file, the MIME content type is set to "application/octet-stream", but the file name is null.

Does anybody know as how to overcome this issue?

Best regards,
Peter

[herrep]

It is not sufficient to set the null file name to a default value. When setting the file name to a default, I am able to store the file, but it contents is not readable while I verified from another mail client that the content is valid.

[StCyr]

I have the same problem here with a .doc file.

Nextcloud: 16.0.4
Mail: 0.17.0

Here's the message's headers related to this attachment:

----boundary_11_39b67e12-1e84-4709-95f0-9232a0af189a
Content-Type: application/octet-stream;
 name="=?utf-8?B?UG9pbnQgZGUgdnVlIG9ww6lyYXRldXIgMDUwOTE5MTYzODI4LmRvYw==?="
Content-Transfer-Encoding: base64
Content-Disposition: attachment

e1xydGYxXGFuc2lcdWMxXGRlZmYwXGRlZmxhbmcxMDMzXGRlZmxhbmdmZTEwMzNcYW5z
aWNwZzEyNTJcZGVmdGFiMA0Ke1xpbmZvfQ0Ke1wqXHVzZXJwcm9wcw0Ke1xwcm9wbmFt
ZSBCdXNpbmVzc1x+T2JqZWN0c1x+Q29udGV4dFx+SW5mb3JtYXRpb259DQpccHJvcHR5
cGUzMA0Ke1xzdGF0aWN2YWwgMDFFNzQ5NEFBQUU2QjRGNEUzMjlDODMxQTBENEU5QkUw

It seems that the problem comes from the fact that Content-Disposition: attachment is set but no filename is provided:

In function load() of lib/Attachment.php (which initialises the attachment we want to download here) we have:

                // Extract headers from part
                $contentDisposition = $mimeHeaders->getValue('content-disposition', \Horde_Mime_Headers::VALUE_PARAMS);
                if (!is_null($contentDisposition)) {
                        $vars = ['filename'];
                        foreach ($contentDisposition as $key => $val) {
                                if(in_array($key, $vars)) {
                                        $this->mimePart->setDispositionParameter($key, $val);
                                }
                        }
                } else {
                        $contentDisposition = $mimeHeaders->getValue('content-type', \Horde_Mime_Headers::VALUE_PARAMS);
                        $vars = ['name'];
                        foreach ($contentDisposition as $key => $val) {
                                if(in_array($key, $vars)) {
                                        $this->mimePart->setContentTypeParameter($key, $val);
                                }
                        }
                }

So, we can see that because of this Content-Disposition: attachment mail expects a filename parameter, which is not there.

But, "why should it be a problem, maybe mail handles this case perfectly well?", you might say.

Well, it doesn't seem so:

In function downloadAttachment in lib/Controller/MessagesController.php line 257 (the line raising the error), we have:

                return new AttachmentDownloadResponse(
                        $attachment->getContents(), $attachment->getName(), $attachment->getType());

And, $attachment->getName() is:

        public function getName() {
                return $this->mimePart->getName();
        }

And, mimePart->getName() is (source: https://dev.horde.org/api/FRAMEWORK_4/lib/Mime/source-class-Horde_Mime_Part.html):

362:      public function getName($default = false)
 363:     {
 364:         if (!($name = $this->getDispositionParameter('filename')) &&
 365:             !($name = $this->getContentTypeParameter('name')) &&
 366:             $default) {
 367:             $name = preg_replace('|\W|', '_', $this->getDescription(false));
 368:         }
 369: 
 370:         return $name;
 371:     }

So, we can see that Horde_Mime_Part returns null and this value is passed up the stack to AttachmentDownloadResponse::_construct.

But, "Maybe this constructor handles the case perfectly well?", you might say....

Well, no: AttachmentDownloadResponse::_construct passes this valur to its parent's constructor DownloadResponse::_construct which is (source: https://github.com/nextcloud/server/blob/stable13/lib/public/AppFramework/Http/DownloadResponse.php):

    public function __construct($filename, $contentType) {
        $this->filename = $filename;
        $this->contentType = $contentType;
        $this->addHeader('Content-Disposition', 'attachment; filename="' . $filename . '"');
        $this->addHeader('Content-Type', $contentType);
    }

So, nobody handle such case.

[StCyr]

The bug doesn't occur for me when I change Attachment::getName() function as follow:

        public function getName() {
//              return $this->mimePart->getName();
                return 'attachment';
        }

But this is just a dirty hack :-)

A better fix would probably be to change function Attachment::load() as follow:

                // Extract headers from part
                $contentDisposition = $mimeHeaders->getValue('content-disposition', \Horde_Mime_Headers::VALUE_PARAMS);
                if (!is_null($contentDisposition)) {
                        $vars = ['filename'];
                        foreach ($contentDisposition as $key => $val) {
                                if(in_array($key, $vars)) {
                                        $this->mimePart->setDispositionParameter($key, $val);
                                }
                        }
                if (!is_null($contentDisposition) &&  !is_null($this->mimePart->getDispositionParameter('filename'))) {
                // Handle case when the 'content-disposition' header is set but no filename parameter could be found
                        $contentDisposition = $mimeHeaders->getValue('content-type', \Horde_Mime_Headers::VALUE_PARAMS);
                        $vars = ['name'];
                        foreach ($contentDisposition as $key => $val) {
                                if(in_array($key, $vars)) {
                                        $this->mimePart->setDispositionParameter($key, $val);
                                }
                        }               
                } else if (is_null($contentDisposition) {
                        $contentDisposition = $mimeHeaders->getValue('content-type', \Horde_Mime_Headers::VALUE_PARAMS);
                        $vars = ['name'];
                        foreach ($contentDisposition as $key => $val) {
                                if(in_array($key, $vars)) {
                                        $this->mimePart->setContentTypeParameter($key, $val);
                                }
                        }
                }

[herrep]

When I manipulated the name of a test attachment (PDF file), I realized that the downloaded file was at least incomplete, as the file size was much smaller than it should be. On other mail clients, I could download the attachment, it had a bigger file size and was opened correctly by the PDF viewer. So it appears that the problem does not only concern the proper naming of the attachment, but rather the entire decoding of the attachment which does not only result in a wrong file name, but also in not correctly decoded content.

[StCyr]

this seems to cause problem also:

--Apple-Mail-E382DA81-E442-483A-90B0-C7A37692073D
Content-Type: application/pdf;
    name="Reservation fete d'anniversaire-10342.pdf";
    x-apple-part-url=0E476D1D-BE3F-4E66-8D0D-B9964DD29E88
Content-Disposition: inline;
    filename*=utf-8''R%C3%A9servation%20f%C3%AAte%20d%27anniversaire%2D10342.pdf
Content-Transfer-Encoding: base64

[zerotation]

Mail app

Mail app version: 0.17.00
Mailserver or service: local hosted Exchange and davmail

Server configuration

Operating system:Virtual "Debian GNU/Linux 9 (stretch)"
Web server: Apache2
Database: MySQL
PHP version: 7.3.9
Nextcloud Version: 16.0.5.1

Client configuration

Browser: Firefox 68.1.0esr (64-bit)
Operating system: OpenSuSE Leap 15.1

---

Same problems as described by ostasevych

PDF, images *.png can't be downloaded, nor is it possible, to store attachments to NC.

Thunderbird desktop-client will download attachments correctly via davmail.

With best regards

[ChristophWurst]

Please see https://github.com/nextcloud/mail/pull/2213