Type of returning for functions of the prepareSubject and the prepare… by artemii-karkusha · Pull Request #241 · laminas/laminas-mail

artemii-karkusha · 2023-05-22T16:31:21Z

Type of returning for functions of the prepareSubject and the prepareParameters have been fixed

Updates the CHANGELOG.md to set the release date.

Updates the CHANGELOG.md file to add a changelog entry for a new 2.14.0 version.

Updates the CHANGELOG.md file to add a changelog entry for a new 2.13.1 version.

- Removes `.travis.yml` - Adds continuous-integration GHA workflow - Updates badges in `README.md` - Removes `CHANGELOG.md`; changelogs will be kept in milestone descriptions and propagated to tag and release notes Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

…urce

Related to glpi-project/glpi#8495 Signed-off-by: Edgard <edgardmessias@gmail.com>

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Line lengths Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

….x_6026c1d0016d83.30821085 Merge release 2.13.1 into 2.14.x

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Signed-off-by: AdrienClairembault <aclairembault@teclib.com>

[2.14]: QA: Add specific type hints for $params

[2.14]: QA: Fix markdown lint errors

…mpty-continuation-lines Fix laminas#25; Skip empty continuation lines

Keep original ZF references refs: - 4665599 Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

[2.14]: Tests: Fix references to ZF issue tracker

To deal with the 998/78 character limitations per line, long lines can be split into a multiple-line representation separated by CRLF + WSP; this is called "folding". Correct folding is particularly important for long header fields. Signed-off-by: Vlad Safronov <vladislav.safronov@oracle.com>

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

This isolates the logic of handling incomplete reads to own unit. Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

As the buffer max size is known, can check last byte with isset Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>

Update to PHPUnit 10

…Parameters have been fixed Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Ocramius

We'd need a test to verify this change: I suppose you expect getSubject() to always return a string?

Xerkus · 2023-05-22T16:44:31Z

dockblock and usage checks out, it is expected to always return string.

Return values are passed to mail() with signature

 mail(
    string $to,
    string $subject,
    string $message,
    array|string $additional_headers = [],
    string $additional_params = ""
): bool

Ocramius · 2023-05-22T16:49:16Z

Yes, I mean a test case (additions to our PHPUnit test suite :D )

The test should fail before this change, and pass after this change.

artemii-karkusha · 2023-05-22T19:24:29Z

We'd need a test to verify this change: I suppose you expect getSubject() to always return a string?

I will add for this case

artemii-karkusha · 2023-05-23T09:41:58Z

@Ocramius
Result of test the testAllowMessageWithEmptySubjectButHasBccHeader before changes for SendMail was made.

Ocramius

@artemii-karkusha looking much better now, thanks!

What's missing now is some cleanup around the test case.

Ocramius · 2023-05-23T09:46:37Z

+        $this->transport->setCallable([$this->transport, 'mailHandler']);
+        try {
+            $this->transport->send($message);
+        } catch (\RuntimeException $runtimeException) {


Is a RuntimeException expected to be thrown at all times?

If so, PHPUnit can help you here:

$this->expectException(\RuntimeException::class); $this->expectExceptionMessage('Something different from a warning');

The try-catch can then be removed :)

@Ocramius No. It is not expected to throw all time. It was added to be sure logic will not be broken for mailHandler in future.

Ok, but then this test has no assertions, and is a risky test 🤔

Can we somehow verify the outcome with a valid assertion?

/cc @Slamdunk for ideas

It is not real to cover outcome. Function doesn't return nothing. What we can cover is exception which is thrown when someone will change behaviour for subject and it will influence on function the mail(..., #subject).
prepareSubject which can be better place for test case is protected.
I don't have any ideas how can we cover it in another way. If you have let it will share with me, please.

Yep, that's why I poked @Slamdunk to get some more brains :)

IMHO we don't need tests here, it's a type mismatch so getting rid of PSalm suppressions should be enough:

diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 88d83696..0140e43e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1737,18 +1737,12 @@ <InvalidArgument> <code><![CDATA[[$this, 'handleMailErrors']]]></code> </InvalidArgument> - <InvalidNullableReturnType> - <code>string</code> - </InvalidNullableReturnType> <MissingReturnType> <code>mailHandler</code> </MissingReturnType> <MixedAssignment> <code>$param</code> </MixedAssignment> - <MixedInferredReturnType> - <code>string</code> - </MixedInferredReturnType> <MixedOperand> <code>$param</code> </MixedOperand> diff --git a/src/Transport/Sendmail.php b/src/Transport/Sendmail.php index aae76f49..58868dda 100644 --- a/src/Transport/Sendmail.php +++ b/src/Transport/Sendmail.php @@ -9,6 +9,7 @@ use Laminas\Mail\Transport\Exception\InvalidArgumentException; use Laminas\Mail\Transport\Exception\RuntimeException; use Traversable; +use function assert; use function count; use function escapeshellarg; use function gettype; @@ -200,10 +201,12 @@ class Sendmail implements TransportInterface { $headers = $message->getHeaders(); if (! $headers->has('subject')) { - return; + return ''; } - $header = $headers->get('subject'); - return $header->getFieldValue(HeaderInterface::FORMAT_ENCODED); + $header = $headers->get('subject'); + $fieldValue = $header->getFieldValue(HeaderInterface::FORMAT_ENCODED); + assert(is_string($fieldValue)); + return $fieldValue; } /** @@ -259,7 +262,7 @@ class Sendmail implements TransportInterface protected function prepareParameters(Mail\Message $message) { if ($this->isWindowsOs()) { - return; + return ''; } $parameters = (string) $this->parameters;

I'm happy with this outcome too 👍

@Ocramius
It has to be closed. I did mistake when changed author for commits. I have created new PL with all needed changes.
#242

…mail has been added. Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

artemii-karkusha · 2023-05-23T14:56:13Z

@Ocramius Please let me know if something is needed to changing. It is ready for merging

This reverts commit 7c2afd0.

This reverts commit f9f43a3.

This reverts commit f07d863.

…sending mail has been added." This reverts commit fab234c.

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

…ng' for Laminas\Mail\Transport\Envelope::getFrom is not nullable, but 'null|string' contains null (see https://psalm.dev/144)" has been fixed Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Slamdunk · 2023-05-25T09:08:02Z

See #242

laminas-bot and others added 30 commits January 26, 2021 13:40

2.13.0 readiness

08cee5a

Updates the CHANGELOG.md to set the release date.

Bumps changelog version to 2.14.0

b733462

Updates the CHANGELOG.md file to add a changelog entry for a new 2.14.0 version.

Bumps changelog version to 2.13.1

b0ee2eb

Updates the CHANGELOG.md file to add a changelog entry for a new 2.13.1 version.

fix TypeError: fclose(): supplied resource is not a valid stream reso…

9850025

…urce

clean uneeded stuff

840b9e2

Fixed multibyte string encoding

a160266

Related to glpi-project/glpi#8495 Signed-off-by: Edgard <edgardmessias@gmail.com>

qa: skip a test that fails when testing against lowest versions

cf22408

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: add composer lockfile

4bc4b58

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: updated lockfile

21218b5

Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

qa: fix CS issues

eb4c90d

Line lengths Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>

Merge pull request laminas#137 from laminas/2.13.x-merge-up-into-2.14…

33c9846

….x_6026c1d0016d83.30821085 Merge release 2.13.1 into 2.14.x

Add specific type hints for $params

c96798a

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Skip line length phpcs check for Pop3/Imap constructors

1ea9b8f

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Apply markdownlint --fix

47218a2

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Fix non-incremented heading in character-sets.md

9bc19d6

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Fix laminas#25; Skip empty continuation lines

2714eeb

Signed-off-by: AdrienClairembault <aclairembault@teclib.com>

Merge pull request laminas#126 from glensc/codestyle-types

ea7b5cd

[2.14]: QA: Add specific type hints for $params

Merge pull request laminas#139 from glensc/md-fix

b003597

[2.14]: QA: Fix markdown lint errors

Merge pull request laminas#145 from AdrienClairembault/fix/multiple-e…

2b0e569

…mpty-continuation-lines Fix laminas#25; Skip empty continuation lines

Revert ZF->Laminas changes for tickets

605169b

Keep original ZF references refs: - 4665599 Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Replace @group ZF/ZF2 with @see links

8a81529

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Merge pull request laminas#141 from glensc/zf-refs

b733298

[2.14]: Tests: Fix references to ZF issue tracker

Use Headers::FOLDING when folding

e739da1

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Use fgets and detect partial line reads

862527f

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Add unit test for long header wrapping

ec145be

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Refactor data reader in Protocol\Smtp to use Generator

3005bf8

This isolates the logic of handling incomplete reads to own unit. Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Optimize out strlen call

16e3ffc

As the buffer max size is known, can check last byte with isset Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Test for max length of all SMTP data lines

02374c1

Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>

Slamdunk and others added 3 commits May 22, 2023 09:57

Update to laminas/laminas-servicemanager:v3.21

2e27485

Signed-off-by: Filippo Tessarotto <zoeslam@gmail.com>

Merge pull request laminas#233 from Slamdunk/phpunit_10

c45bd26

Update to PHPUnit 10

Type of returning for functions of the prepareSubject and the prepare…

b35eaa7

…Parameters have been fixed Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

artemii-karkusha mentioned this pull request May 22, 2023

Laminas\Mail\Transport\Exception\RuntimeException: Unable to send mail: mail(): Passing null to parameter #2 ($subject) of type string is deprecated in /app/vendor/laminas/laminas-mail/src/Transport/Sendmail.php:310 #240

Closed

Ocramius requested changes May 22, 2023

View reviewed changes

Ocramius requested changes May 23, 2023

View reviewed changes

artemii-karkusha force-pushed the fix-for-sendmail-class-mail branch from 695288c to 7faa761 Compare May 23, 2023 09:51

artemii-karkusha added 4 commits May 23, 2023 11:51

Test case for covering of case with empty the subject during sending …

f5a1abe

…mail has been added. Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Name of class the SendmailTest has been reverted

60e8ffa

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

use statement for function str_contains has been added.

d397c8f

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

PHPCodesniffer issues have been fixed

2b1d023

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Slamdunk added the Bug Something isn't working label May 24, 2023

Slamdunk added this to the 2.23.0 milestone May 24, 2023

artemii-karkusha added 8 commits May 24, 2023 16:22

Revert "PHPCodesniffer issues have been fixed"

c19baaa

This reverts commit 7c2afd0.

Revert "use statement for function str_contains has been added."

22afd0b

This reverts commit f9f43a3.

Revert "Name of class the SendmailTest has been reverted"

75d221e

This reverts commit f07d863.

Revert "Test case for covering of case with empty the subject during …

5ad1270

…sending mail has been added." This reverts commit fab234c.

Suggested changes have been applied.

314ecbb

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Issue the " InvalidNullableReturnType: The declared return type 'stri…

1795bca

…ng' for Laminas\Mail\Transport\Envelope::getFrom is not nullable, but 'null|string' contains null (see https://psalm.dev/144)" has been fixed Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Value of propery the $from has been change to ''.

945b1d4

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

Default value of propery the $to has been reverted to null.

bab9ae9

Signed-off-by: Artemii Karkusha <artemii.karkusha@alshaya.com>

artemii-karkusha force-pushed the fix-for-sendmail-class-mail branch from fac3277 to bab9ae9 Compare May 24, 2023 16:30

Slamdunk mentioned this pull request May 25, 2023

Fix return-type for Sendmail::prepareSubject and the Sendmail::prepareParameters #242

Merged

Slamdunk closed this May 25, 2023

Uh oh!

Conversation

artemii-karkusha commented May 22, 2023

Uh oh!

Ocramius left a comment

Choose a reason for hiding this comment

Uh oh!

Xerkus commented May 22, 2023

Uh oh!

Ocramius commented May 22, 2023

Uh oh!

artemii-karkusha commented May 22, 2023

Uh oh!

artemii-karkusha commented May 23, 2023

Uh oh!

Ocramius left a comment

Choose a reason for hiding this comment

Uh oh!

Ocramius May 23, 2023

Choose a reason for hiding this comment

Uh oh!

artemii-karkusha May 23, 2023

Choose a reason for hiding this comment

Uh oh!

Ocramius May 23, 2023

Choose a reason for hiding this comment

Uh oh!

artemii-karkusha May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ocramius May 23, 2023

Choose a reason for hiding this comment

Uh oh!

Slamdunk May 24, 2023

Choose a reason for hiding this comment

Uh oh!

Ocramius May 24, 2023

Choose a reason for hiding this comment

Uh oh!

artemii-karkusha May 24, 2023

Choose a reason for hiding this comment

Uh oh!

artemii-karkusha commented May 23, 2023

Uh oh!

Slamdunk commented May 25, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

artemii-karkusha May 23, 2023 •

edited

Loading