PHPCS 3.6.2 added a sniff for a PSR-12 rule which was previously not strictly checked: "No blank line after the opening brace of a class".
This fixes the newly flagged issues.
So far, this method did not have dedicated tests.
The test file this commit introduces, tests all aspects of the method as well as documents the current behaviour of the method for specific, outlier situations.
So far, the methods related to text localization - `PHPMailer::setLanguage()`, `PHPMailer::getTranslations()` and `PHPMailer::lang()` - did not have dedicated tests.
The test file this commit introduces, tests all aspects of these methods, including the changes introduced in response to 2418 and 2419, as well as documents the current behaviour of the methods for specific, outlier situations.
The `mb_decode_mimeheader()` function uses the Mbstring internal encoding to decode.
In PHP 5.5, the default internal encoding was `ISO-8859-1`.
As of PHP 5.6, the default internal encoding was changed to use the value from the `default_charset` ini setting. Additionally, in UTF-8, the value for `default_charset` was changed to `UTF-8`.
This means that when the charset is not explicitly set, the `mb_decode_mimeheader()` function may return garbled nonsense if the charset used to _encode_ does not match the charset per PHP's `default_charset` or - in PHP 5.5 - the Mbstring internal_encoding default.
So far, this wasn't making tests fail because of some hard-coded ini settings being passed in the CI.
However, changing the default ini values creates an assumption that that configuration will be used on all servers on which the PHPMailer code will be run.
This assumption is undocumented (not in the Readme or mentioned elsewhere) and will in most cases be incorrect.
The non-default ini values change the behaviour of PHP and were the cause of test failures against PHP 5.5 which I've been seeing for some of the new tests I've been creating.
Removing the changes fixes those errors, but exposes failing tests in the existing tests for `PHPMailer::parseAddresses()`.
These undocumented _changes_ to the default PHP configuration were **required** for PHPMailer to be able to parse the addresses successfully. As this library is open source and used in a wide variety of environments, those kind of assumptions can not safely be made.
So.... the hard-coded ini settings in the CI configuration ought to be removed.
This then causes the tests for the `PHPMailer::parseAddresses()` function to start failing on PHP 5.5.
> Note: the tests are only failing on PHP 5.5 as the test case causing the failure uses a UTF-8 encoded name and as of PHP 5.6, the default encoding used in PHP is UTF-8, which matches.
> If a test case would be added with a name encoded in a different charset, the tests would also start failing on PHP 5.6+.
To fix those failures and to make the code PHP cross-version compatible, including with PHP installs configured to use a different `default_encoding`:
* We need to make sure that the Mbstring "internal encoding" is set correctly based on the Charset used for PHPMailer.
* And then need to _reset_ the internal encoding after the use of the `mb_decode_mimeheader()` to prevent any impact of this change on the wider application context in which PHPMailer may be used.
As the `PHPMailer::parseAddresses()` method is `static`, it does not have access to the (non-static) `PHPMailer::$charSet` variable.
Knowing that, I've elected to add an additional, optional variable to the `PHPMailer::parseAddresses()` method to allow for passing in the charset and have set the default value for the parameter to be in line with the default value of the `PHPMailer::$charSet` variable.
I have adjusted existing method calls to this method to explicitly pass the charset.
Both of the adjusted function calls are in the "postSend" part of the PHPMailer logic when the charset will be known and final, so can be safely passed.
I've also made minimal changes to the unit test file to allow for passing the charset in the tests.
This implementation is based on the assumption that names can be encoded in different charsets.
If the name encoding only happens when the charset is UTF-8, the new function parameter can be removed and the charset can be set to UTF-8 directly.
As I'm not completely read-in on the RFC specs for the address header being parsed and when encoding happens, I'd like a second opinion on the currently chosen implementation.
If this is the correct way to go, then additional tests need to be added to safeguard that things works correctly when a different encoding is used.
If the encoding only happens for UTF-8, the implementation can be simplified.
Update: the current implementation is correct and a `@todo` note has been added to add more tests with different encodings during a next iteration on these tests.
Refs:
* https://www.php.net/manual/en/migration56.deprecated.php#migration56.deprecated.iconv-mbstring-encoding
* https://php-legacy-docs.zend.com/manual/php5/en/ini.core#ini.default-charset
As noted in 2380, the addressing tests do not belong in the mail transport tests and as the `ReplyToGetSetClearTest` now covers this extensively, the assertions can be removed from the `MailTransportTest`.
The "fakefunctions" are all nice and dandy to get past the `idnSupported()` check, but if either of these functions is not _really_ available, the actual behaviour of the `PHPMailer::addReplyTo()` function is to _fail_ (on the address validation call in `PHPMailer::addAnAddress()`) and return `false`.
In other words, this test was nonsensical as it tested for something which can, and should, never happen.
With this in mind, the test has been rewritten to reflect the *real* behaviour when either Mbstring or the `idn_to_ascii` function is not available.
This commit:
* Renames the `testDuplicateIDNRemoved()` method to `testNoDuplicateReplyToAddresses()`.
* Removes the call to `clearReplyTos()` at the start of the function as it is redundant. Each test gets a fresh instance of the PHPMailer class.
* Adds a number of additional assertions to the method to test the enqueuing and duplication removal in more depth.
* Ensures that all assertions are accompanied by a "failure" message so it can more easily be determined which assertion failed (in case of failure).
Includes adding `@covers` tags for the test.
This commit:
* Renames the `testConvertEncoding()` method to `testEnqueueAndAddIdnAddress()`.
* Removes the call to `clearReplyTos()` at the start of the function as it is redundant. Each test gets a fresh instance of the PHPMailer class.
* Adds a number of additional assertions to the method to test the enqueuing in more depth.
Includes adding `@covers` tags for the test.
... to test that the header gets set correctly when multiple reply-to addresses have been set, as well as when a reply-to address has been set without a name.
This commit:
* Renames the `testLowPriority()` method to `testReplyToInMessageHeader()`.
This test method was originally basically testing two things: the priority and the reply to header being set. For this class, it now just focusses on the reply to header, so letting the name reflect that.
* Enhances the test by actually testing that the reply-to header is correctly found in the fully composed message.
* Breaks out the test case to a data provider to allow for adding additional test cases more easily.
... to test that passing an invalid email address in combination with an instance of the `PHPMailer` class which was instantiated with `$exceptions = true` results in an exception.
Includes adding `@covers` tags for this specific method.
This commit:
* Adds two extra test cases.
* Adds handling to allow the "expected" registered values (and key) being different from the original input values.
This commit:
* Splits the primary tests previously contained in the `testAddressing()` method into two distinct test methods, each using a data provider to allow for adding additional test cases more easily.
* Enhances the tests by not only testing the return value of the `addReplyTo()` method, but also verifying that the `ReplyTo property has been set correctly and contains the expected information.
Includes adding `@covers` tags for these specific methods.
This commit moves the "set a property to null and then back" test to a separate test method.
Note: based on the code in the method, this test doesn't really add any value, but I also see no reason to remove it.
Marcus Bointon
JT Smith
jrfnl
Fonata
Julien BERNARD
Ikko Ashimine