Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Add option novalidatecert to connect() #247

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
Open

Add option novalidatecert to connect() #247

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ece418b
Bumped version
weierophinney Jun 7, 2018
ad8ba1a
Add option novalidatecert to connect()
abulhol Oct 18, 2019
36d133b
New implementation of novalidatecert
abulhol Oct 18, 2019
cbc3e4f
Added setter method for new variable; removed @ for socket_connect
abulhol Oct 21, 2019
767e170
Added new param to docs
abulhol Oct 21, 2019
c2eae3c
Fix empty lines and line lengths
abulhol Oct 21, 2019
76aa3e0
Fix formatting multi line function call
abulhol Oct 21, 2019
48e9ee1
Final indentation fix
abulhol Oct 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions src/Protocol/Imap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ class Imap
*/
const TIMEOUT_CONNECTION = 30;

/**
* Do not validate the SSL certificate if set to true
* @var null|bool
*/
public $novalidatecert;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we do not have typed properties, we can not use public here. You can use the constructor to set a value for this property.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'd add either:

  • a 4th constructor argument, $validateCert = true
  • a 4th constructor argument, array $options, and document a "validate_cert" or "validateCert" option.

The latter makes it easier to expand options, but leads to more validation once we add typehints. The former is self-documenting, but means if we add more options down the line, the constructor signature gets really large.

I'm fine with either approach, but agree with @froschdesign here that a public property is not a great idea.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi guys, thank you for the feedback!
I have added the variable to the constructor, but this does not really fix the issue. The thing is that the way you create a POP3 or IMAP connection is by creating an instance of Zend\Mail\Storage\Pop3 or Zend\Mail\Storage\Imap3.
The constructor then calls the Zend\Mail\Protocol constructor without arguments, followed by calling connect().
But because I cannot add a new parameter to connect() (see previous PR), and I have to leave the constructor call without arguments as it is, I have now added a setter method for novalidatecert.
Please see my latest commit.


/**
* socket to imap server
* @var resource|null
Expand Down Expand Up @@ -85,9 +91,23 @@ public function connect($host, $port = null, $ssl = false)
$port = 143;
}
}


$socket_options = [];

if ($this->novalidatecert) {
$socket_options = [
'ssl' => [
'verify_peer_name' => false,
'verify_peer' => false,
]
];
}

$socket_context = stream_context_create($socket_options);

ErrorHandler::start();
$this->socket = fsockopen($host, $port, $errno, $errstr, self::TIMEOUT_CONNECTION);
$this->socket = @stream_socket_client($host . ":" . $port, $errno, $errstr, self::TIMEOUT_CONNECTION, STREAM_CLIENT_CONNECT, $socket_context);

$error = ErrorHandler::stop();
if (! $this->socket) {
throw new Exception\RuntimeException(sprintf(
Expand Down
24 changes: 22 additions & 2 deletions src/Protocol/Pop3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ class Pop3
*/
public $hasTop = null;

/**
* Do not validate the SSL certificate if set to true
* @var null|bool
*/
public $novalidatecert;

/**
* socket to pop3
* @var null|resource
Expand Down Expand Up @@ -90,9 +96,23 @@ public function connect($host, $port = null, $ssl = false)
$port = 110;
}
}


$socket_options = [];

if ($this->novalidatecert) {
$socket_options = [
'ssl' => [
'verify_peer_name' => false,
'verify_peer' => false,
]
];
}

$socket_context = stream_context_create($socket_options);

ErrorHandler::start();
$this->socket = fsockopen($host, $port, $errno, $errstr, self::TIMEOUT_CONNECTION);
$this->socket = @stream_socket_client($host . ":" . $port, $errno, $errstr, self::TIMEOUT_CONNECTION, STREAM_CLIENT_CONNECT, $socket_context);

$error = ErrorHandler::stop();
if (! $this->socket) {
throw new Exception\RuntimeException(sprintf(
Expand Down
5 changes: 5 additions & 0 deletions src/Storage/Imap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ public function __construct($params)
$ssl = isset($params->ssl) ? $params->ssl : false;

$this->protocol = new Protocol\Imap();

if (isset($params->novalidatecert)) {
$this->protocol->novalidatecert = $params->novalidatecert;
}

$this->protocol->connect($host, $port, $ssl);
if (! $this->protocol->login($params->user, $password)) {
throw new Exception\RuntimeException('cannot login, user or password wrong');
Expand Down
7 changes: 6 additions & 1 deletion src/Storage/Pop3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,13 @@ public function __construct($params)
$password = isset($params->password) ? $params->password : '';
$port = isset($params->port) ? $params->port : null;
$ssl = isset($params->ssl) ? $params->ssl : false;

$this->protocol = new Protocol\Pop3();

if (isset($params->novalidatecert)) {
$this->protocol->novalidatecert = $params->novalidatecert;
}

$this->protocol->connect($host, $port, $ssl);
$this->protocol->login($params->user, $password);
}
Expand Down