Mail notification for new share generates invalid email from (Error: Swift_RfcComplianceException: Address in mailbox given [...] does not comply with RFC 2822, 3.6.2.)

[0x47]

The code at:

server/lib/private/Share20/Manager.php

Line 925 in d2ea068

$message->setFrom([\OCP\Util::getDefaultEmailAddress($instanceName) => $senderName]);

creates invalid e-mail from addresses if the server is not configured to use fixed custom mail_from_address. Error messages are:

Error: Swift_RfcComplianceException: Address in mailbox given [...] does not comply with RFC 2822, 3.6.2.

As can be seen in

server/lib/public/Util.php

Line 291 in d2ea068

$user_part = $config->getSystemValue('mail_from_address', $user_part);

the parameter $user_part is used if mail_from_address has not been set for the instance. The instanceName given by the sendMailNotification function may contain spaces which is not allowed.

server/lib/private/Share20/Manager.php

Line 917 in d2ea068

$instanceName = $this->defaults->getName();

Please use a $user_part similar to the lost password function:

server/core/Application.php

Line 68 in 9de329a

return Util::getDefaultEmailAddress('lostpassword-noreply');

Here, the $user_part makes sense and is not dependent on arbitrary strings defined in the admin section. Maybe use something like shares-noreply.

[MorrisJobke]

I noticed this on a setup where no email is configured and it took Powered by Company@localhost.localdomain as sender. Setup check was green. Maybe it should also warn about a missing mail setup in there.

cc @kesselb

[MorrisJobke]

The response to the POST request to /shares was:

{
    "ocs": {
        "meta": {
            "status": "faulure",
            "statuscode": 403,
            "message": "Address in mailbox given [Powered by Company@localhost.localdomain| does not comply with RFC 2822, 3.6.2."
        }
    }
}

That email username is coming most likely from the theming as #21120 (comment)

[MorrisJobke]

cc @juliushaertl for the theming part

[0x47]

Setup check was green. Maybe it should also warn about a missing mail setup in there.

In my opinion the issue is not a missing mail setup but simply a bug in the code. Why would you ever use $instanceName as the sender of a mail? It can be an arbitrary string with spaces in it.

Having no sender part in the server mail configuration is a perfectly fine use case. It should (and will in at least two cases) use specific sender parts (noreply@... and lostpassword-noreply@...) for each purpose.

EDIT: Just to clarify, $instanceName is filled with $this->defaults->getName() as described in the first comment, check

server/lib/private/Share20/Manager.php

Line 917 in d2ea068

$instanceName = $this->defaults->getName();

[MorrisJobke]

In my opinion the issue is not a missing mail setup but simply a bug in the code. Why would you ever use $instanceName as the sender of a mail? It can be an arbitrary string with spaces in it.

True point. 👍 That should be fixed.

[MorrisJobke]

Fix is in #27199