Envelope incorrectly processes Address object

[snapey]

Laravel Version

10.47.0

PHP Version

8.2.16

Database Driver & Version

No response

Description

passing a single Address object into Envelope causes two Address objects being created, with the second having the name as the email address and causing a malformed email address error

Steps To Reproduce

Reproduced in Tinkerwell

Creating an Address object

$to = new Address("peter@dragon.co.uk", "Peter Jones");

results in

Illuminate\Mail\Mailables\Address {#1383
    +address: "peter@dragon.co.uk",
    +name: "Peter Jones",
  }

A seemingly correct Address object with address and name correctly assigned

However, pass this object into the envelope;

$to = new Address("peter@dragon.co.uk", "Peter Jones");

$en = new Envelope(to: $to);

results in the two properties of Address to be turned into two separate Address objects

Illuminate\Mail\Mailables\Envelope {#4873
    +from: null,
    +to: [
      "address" => Illuminate\Mail\Mailables\Address {#4399
        +address: "peter@dragon.co.uk",
        +name: null,
      },
      "name" => Illuminate\Mail\Mailables\Address {#3631
        +address: "Peter Jones",
        +name: null,
      },
    ],
    +cc: [],
    +bcc: [],
    +replyTo: [],
    +subject: null,
    +tags: [],
    +metadata: [],
    +using: [],
  }

I suspect this could be a quite common issue, but with devs finding some alternate syntax that works for them, such as wrapping the address in an array;

$en = new Envelope(to: [$to]);

correctly gives

Illuminate\Mail\Mailables\Envelope {#4469
    +from: null,
    +to: [
      Illuminate\Mail\Mailables\Address {#1619
        +address: "peter@dragon.co.uk",
        +name: "Peter Jones",
      },
    ],

[kokoshneta]

This seems to be intentional. The Envelope class doc-hints the $from property as Address|string|NULL, but the $to, $cc, $bcc and $replyTo properties as array only.

The actual processing happens in normalizeAddresses(), which is also built around the expectation that those properties should be passed as arrays.

I agree, though, that it should be possible to pass a simple Address object without the whole thing failing. It would make sense to check if the $addresses parameter to normalizeAddresses() is an Address object, and if so, just wrap it in an array and be done with it.

[snapey]

Agreed.

    protected function normalizeAddresses($addresses)
    {
        if( $addresses instanceof Address) return Arr::wrap($addresses);
        
        return collect($addresses)->map(function ($address) {
            return is_string($address) ? new Address($address) : $address;
        })->all();
    }

Or enforce types on the constructor, although this could be breaking change for some apps.

[crynobone]

After further investigation, this seems to be more of docblock improvement.

[crynobone]

Hey there, thanks for reporting this issue.

If you notice improper DocBlock, PHPStan, or IDE warnings while using Laravel, do not create a GitHub issue. Instead, please submit a pull request to fix the problem. Also see our contribution guide.

Thanks!

[snapey]

I disagree with your interpretation. This is not a docblock issue, and by stating that the recipients can be instances of Address you have compounded the issue.

The issue is that recipients CANNOT be a single address instance since normalizeAddresses will iterate over the address object and create two new Address objects only one of which is valid.

Second opinion requested.

[crynobone]

The issue is that recipients CANNOT be a single address instance since normalizeAddresses will iterate over the address

The docblock of $to in Envelope constructor explicitly only declares array, there's no code or tests that suggest you can pass an instance of Address. Is there any documentation by Laravel that suggests using other than array<int, \Illuminate\Mail\Mailables\Address> as suggested in the docblock? If not, you are asking for an enhancement to the code and that's not a bug report.