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.

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

Null exception when sending a mail defining envelope without from #50261

Closed
Sharptsa opened this issue Feb 26, 2024 · 3 comments
Closed

Null exception when sending a mail defining envelope without from #50261

Sharptsa opened this issue Feb 26, 2024 · 3 comments

Comments

@Sharptsa
Copy link

Laravel Version

10.45.1

PHP Version

8.1.27

Database Driver & Version

No response

Description

Sending a Mailable with an envelope without the from argument throws an error, even when a global from address is set in the config.
I seems that as long as an envelope is defined, Laravel tries to get the sender address from it without defaulting to the config sender.

Attempt to read property "address" on null

  at vendor/laravel/framework/src/Illuminate/Mail/Mailables/Envelope.php:272
    268▕      */
    269▕     public function isFrom(string $address, string $name = null)
    270▕     {
    271▕         if (is_null($name)) {
  ➜ 272▕             return $this->from->address === $address;
    273▕         }
    274▕ 
    275▕         return $this->from->address === $address &&
    276▕                $this->from->name === $name;

Explicitly setting the from argument to the config value fixes the issue but it would be better to rely on the implicit global configuration as stated in the documentation.

Steps To Reproduce

Mailable:

<?php

namespace App\Mail;

use Illuminate\Bus\Queueable;
use Illuminate\Mail\Mailable;
use Illuminate\Mail\Mailables\Content;
use Illuminate\Mail\Mailables\Envelope;
use Illuminate\Queue\SerializesModels;

class PasswordReset extends Mailable
{
    use Queueable;
    use SerializesModels;

    /**
     * Create a new message instance.
     */
    public function __construct(protected string $url)
    {
    }

    /**
     * Get the message envelope.
     */
    public function envelope(): Envelope
    {
        return new Envelope(
            subject: 'Password Reset',
        );
    }

    /**
     * Get the message content definition.
     */
    public function content(): Content
    {
        return new Content(
            view: 'emails.resetPassword',
            with: [
                'url' => $this->url,
            ]
        );
    }
}

config/mail.php:

'from' => [
    'address' => env('MAIL_SENDER', '[email protected]'),
    'name' => 'Sender',
],

Method that sends the email:

public function sendEmail(User $user, string $token): void
{
    $url = $this->generateUrl($token);
    $mail = new PasswordReset($url);
    Mail::to($user->email)->send($mail);
}
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@driesvints driesvints added the bug label Feb 27, 2024
@arifszn
Copy link
Contributor

arifszn commented Feb 28, 2024

I couldn't reproduce the issue. Is there any reproducible repository, @Sharptsa ?

@Sharptsa
Copy link
Author

Sharptsa commented Feb 28, 2024

I was just looking into it and it's not actually a bug with the mailer.

The error is coming from a test asserting that the sender is correct :

Mail::assertSent(PasswordReset::class, static function (PasswordReset $mail) use ($user) {
    return $mail->hasTo($user->email) &&
        $mail->hasFrom('[email protected]');
});

The assertion is made on the Mailable instance and checks the from parameter on the envelope. Since the envelope has no from defined, it throws an error down the line.

It only affects tests, so definitely not a big deal. I'm not even sure this is really a bug. I guess it could be handled better and not throw an error but that's it.

Sorry for the initial misleading report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants