fix: TypeError fsockopen(): Argument #2 ($port) must be of type int, string given Issue

[cangir]

Update emailer function to handle type casting and default values

Changes:

• Added type casting for certain configuration values retrieved from settings, ensuring correct data types are used.
• Handled default values for configuration settings to prevent potential null values, ensuring consistent behavior.
• Updated comments for clarity and documentation.

Reasoning:

• Type casting ensures that the configuration values are of the correct data type expected by the Email class, preventing potential type errors.
• Handling default values ensures that if a configuration value is not present in the settings or is null, a fallback default value from the Email class is used, ensuring consistent behavior and preventing runtime errors.
• Updated comments for clarity and documentation to improve readability and maintainability of the codebase.

This change fixes this error:

TypeError
fsockopen(): Argument #2 ($port) must be of type int, string given 
SYSTEMPATH/Email/Email.php at line 1896

[datamweb]

Hello. Thank you for your participation. Can you provide the output of the following code?

dd($config);

[datamweb]

service('settings')->set('Email.userAgent', 'Example userAgent');
service('settings')->set('Email.SMTPPort', 25);
service('settings')->set('Email.BCCBatchMode', true);

Output for:

$config = [
'userAgent'     => setting('Email.userAgent'),
'SMTPPort'      => setting('Email.SMTPPort'),
'BCCBatchMode'  => setting('Email.BCCBatchMode'),
];
dd($config);

$config array (3)
⇄userAgent => string (17) "Example userAgent"
⇄SMTPPort => integer 25
⇄BCCBatchMode => boolean true

The result is that this is not an bug, because service('settings')->set('Email.SMTPPort', '25'); is different from service('settings')->set('Email.SMTPPort', 25);. The developer must pass the correct value to set().

However, I think we can consider this as a code improvement.

[kenjis]

Handled default values for configuration settings to prevent potential null values, ensuring consistent behavior. Handling default values ensures that if a configuration value is not present in the settings or is null, a fallback default value from the Email class is used, ensuring consistent behavior and preventing runtime errors.

Where is the code?

Updated comments for clarity and documentation.

Where?

[kenjis]

How can we reproduce the error?

TypeError
fsockopen(): Argument #2 ($port) must be of type int, string given 
SYSTEMPATH/Email/Email.php at line 1896

[datamweb]

How can we reproduce the error?

@kenjis Configure file app/Config/Email.php.

Run the following code to set the wrong port value.

<?php

namespace App\Controllers;

class Home extends BaseController
{
public function index(): string
{
+service('settings')->set('Email.SMTPPort', '25');
-return view('welcome_message');
}
}

Then try to login from the magic link.

[kenjis]

That code should be a TypeError because it stores the wrong type of data (string 25). Do we need to force a type conversion? Even if a dev sets '25_wrong_data', it works. Should the code work?

@cangir Why do you need this PR? Why should the wrong code be made to work?

[cangir]

function emailer(array $overrides = []): Email
    {
        $config = [
            'userAgent'     => setting('Email.userAgent'),
            'protocol'      => setting('Email.protocol'),
            'mailPath'      => setting('Email.mailPath'),
            'SMTPHost'      => setting('Email.SMTPHost'),
            'SMTPUser'      => setting('Email.SMTPUser'),
            'SMTPPass'      => setting('Email.SMTPPass'),
            'SMTPPort'      => (int) setting('Email.SMTPPort'),
            'SMTPTimeout'   => (int) setting('Email.SMTPTimeout'),
            'SMTPKeepAlive' => (bool) setting('Email.SMTPKeepAlive'),
            'SMTPCrypto'    => setting('Email.SMTPCrypto'),
            'wordWrap'      => (bool) setting('Email.wordWrap'),
            'wrapChars'     => (int) setting('Email.wrapChars'),
            'mailType'      => setting('Email.mailType'),
            'charset'       => setting('Email.charset'),
            'validate'      => (bool) setting('Email.validate'),
            'priority'      => (int) setting('Email.priority'),
            'CRLF'          => setting('Email.CRLF'),
            'newline'       => setting('Email.newline'),
            'BCCBatchMode'  => (bool) setting('Email.BCCBatchMode'),
            'BCCBatchSize'  => (int) setting('Email.BCCBatchSize'),
            'DSN'           => (bool) setting('Email.DSN'),
        ];

        if ($overrides !== []) {
            $config = array_merge($config, $overrides);
        }

        /** @var Email $email */
        $email = service('email');

        return $email->initialize($config);
    }

I have fixed the problem by defining the value types this way on email_helper.php Maybe there is a better solution, but this is what I could offer.

[kenjis]

@cangir Thank you for the feedback. What was your problem? Why do you (and we) need to force type conversions?

[cangir]

On route localhost/login/magic-link I'm having this error message:

[kenjis]

@cangir Thank you! Do you know why you got the TypeError? How come you set the incorrect typed value for it?

[jozefrebjak]

@cangir @kenjis

I've been reflecting on the issue and it seems that the root cause might be related to improperly casted values in the saved settings.

We encountered a similar issue in our project after an update. It was resolved when we ensured that the Email settings were properly casted. @cangir, you might want to check if the same applies to your project.

I believe that merging this PR might not address the underlying problem.

[kenjis]

@jozefrebjak Thank you for your feedback!

[kenjis]

In my option I oppose to merge this. Because the root cause of the reported error is passing an incorrect typed value. It is a bug in application code, not a bug in Shield.

[kenjis]

No member actively supports this PR. So I close.