Fix E_NOTICE on undefined $socket property#99
Conversation
|
I am not sure about the best way to handle conflict between Maybe the But I do not get why Last point, I was wondering if following code should be also moved in if (($result = stream_set_timeout($this->socket, self::TIMEOUT_CONNECTION)) === false) {
throw new Exception\RuntimeException('Could not set stream timeout');
} |
| throw new Exception\RuntimeException(sprintf('Invalid remote: %s', $remote)); | ||
| } | ||
|
|
||
| $this->setupSocket($matches['host'], $matches['port'], self::TIMEOUT_CONNECTION); |
There was a problem hiding this comment.
This part I'm not certain about.
In #94, @trasher specifically did NOT add the ProtocolTrait to the SMTP implementation. As such, we run a risk when changing this, UNLESS those changes also apply to the SMTP connection protocol.
Can somebody check this, please?
Also, I'm not sure why we'd add the extra verifications here, and not in setupSocket().
There was a problem hiding this comment.
I've checked, and we should be supporting self-signed certs for SMTP connections made over SSL or TLS as well.
I'm about to push a set of changes back to your branch that addresses this issue more comprehensively.
There was a problem hiding this comment.
Indeed, it make sense to support it for SMTP connection.
I may not have time to work on it for next 2 weeks, but I can, at least, take time to review som changes.
There was a problem hiding this comment.
I've got the changes pushed. :)
Signed-off-by: Cédric Anne <cedric.anne@gmail.com>
Signed-off-by: Cédric Anne <cedric.anne@gmail.com>
…ties per-protocol.
This patch does the following:
- Refactors the `ProtocolTrait` to add scalar and return type hints (as
current develop branch targets PHP 7.1+).
- Updates the `ProtocolTrait::setupSocket()` method as follows:
- Adds a new initial `string` argument, `$transport`.
- Returns the socket created.
- Uses the transport, host, and port to create the `$remote` argument
for `stream_socket_client()`.
- Adds a verification that we can set the stream timeout.
- Updates each of the `Imap` and `Pop3` protocol implementations to:
- Add a `$socket` property.
- Set the `$socket` property from the return value of
`$this->setupSocket()`.
- Marshal the `$transport` argument to send to `setupSocket()`, and no
longer prepend it to the `$host` argument. By default, the
`$transport` is `tcp` (per the `stream_socket_client()`
documentation).
- Adds a `$socket` property to the `AbstractProtocol` class.
- Updates the `Smtp` proptocol implementation as follows:
- It now composes the `ProtocolTrait`.
- It checks for the `novalidatecert` configuration option during
construction, and passes the value to `setNoValidateCert()`.
- `connect()` now proxies to `setupSocket()`, and assign its inherited
`$socket` property to the return value. This ensures that the SMTP
protocol can also talk to servers with self-signed certificates (e.g.,
during development or QA).
- Deprecates the `AbstractProtocol::_connect()` method, as it is no
longer used internally.
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
Signed-off-by: Matthew Weier O'Phinney <matthew@weierophinney.net>
2 participants
Description
I am working with current state of develop branch and code is generating some E_NOTICE.
To reproduce :
This is not a common use case, so I only experiencing it in an automated test suite.
Declaring the property
$socketonLaminas\Mail\Protocol\ProtocolTraitshould fix this issue.