Differential D25660

PHPMailer: Set defaults for required parameters followed by optional parameters in Authorise() method signature
ClosedPublic
Actions

Authored by aklapper on May 17 2024, 14:14.

Details

Reviewers

valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15834: Parameter order of Authorise() signature in phpmailer/class.pop3.php is deprecated since PHP 8.0

Commits

rPd9e543ba970b: PHPMailer: Set defaults for required parameters followed by optional parameters…

Summary

Required parameters $password and $username follow the optional parameter $tval.
This is deprecated since PHP 8.0 per https://www.php.net/manual/en/migration80.deprecated.php
Define defaults for those required parameters to fix the issue - https://github.com/PHPMailer/PHPMailer/blob/master/src/POP3.php#L199 seems to also have survived so far without complaints by PHP 8 users (and PHPStan also does not complain anymore after this change).

Closes T15834

Test Plan

Carefully read the code.

Diff Detail

Repository

rP Phorge

Branch

T15834popSignature (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 1283
Build 1283: arc lint + arc unit

Event Timeline

aklapper created this revision.May 17 2024, 14:14

Owners added a reviewer: O1: Blessed Committers.May 17 2024, 14:14

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan, tobiaswiese. · View Herald TranscriptMay 17 2024, 14:14

aklapper requested review of this revision.May 17 2024, 14:14

Harbormaster completed remote builds in B1283: Diff 2014.May 17 2024, 14:14

sgtm

Thanks for the upstream link

This revision is now accepted and ready to land.May 30 2024, 10:38

restore newline

This revision now requires review to proceed.May 30 2024, 10:39

Harbormaster completed remote builds in B1308: Diff 2053.May 30 2024, 10:39

valerio.bozzolan accepted this revision.May 30 2024, 10:39

This revision is now accepted and ready to land.May 30 2024, 10:39

aklapper retitled this revision from PHPMailer: Set defaults for required parameters followed by required parameters in Authorise() method signature to PHPMailer: Set defaults for required parameters followed by optional parameters in Authorise() method signature.May 30 2024, 13:40

Closed by commit rPd9e543ba970b: PHPMailer: Set defaults for required parameters followed by optional parameters…. · Explain WhyMay 30 2024, 13:40

This revision was automatically updated to reflect the committed changes.

aklapper added a commit: rPd9e543ba970b: PHPMailer: Set defaults for required parameters followed by optional parameters….

Since this is a third-party dependency can we check if this was fixed upstream, and that an update to phpmailer would fix this?

Oh I see you did confirm it’s fixed upstream. Should we just update the version we’re using?

In D25660#18437, @speck wrote:

Oh I see you did confirm it’s fixed upstream. Should we just update the version we’re using?

Ideally yes but the "just" is quite complex given about 13 years of no updating and no test setup locally. (Stuff like D25631 is at least pretty testable.)

Revision Contents
Changeset List

Path

Size

Packages

externals/

phpmailer/

class.pop3.php

4 lines

O1: Blessed Committers

Diff 2014

View Options

externals/phpmailer/class.pop3.php

PHPMailer: Set defaults for required parameters followed by optional parameters in Authorise() method signatureClosedPublicActions