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() #246

Closed
wants to merge 2 commits into from

Conversation

abulhol
Copy link

@abulhol abulhol commented Oct 18, 2019

This commit is a proposed fix for the following issue:
#84
It introduces an optional parameter novalidatecert both for POP3 and IMAP, which makes it possible to connect to mail servers with self-signed certificates, e.g. for development purposes.

@michalbundyra
Copy link
Member

Change of method signature is BC Break. Can we implement it the other way without changing the method signature?

@abulhol
Copy link
Author

abulhol commented Oct 18, 2019

Change of method signature is BC Break. Can we implement it the other way without changing the method signature?

This is why I added this as an optional parameter, which is indeed BC because the params are an array.
How else is the method caller supposed to signal that SSL certificates should not be checked, if not through the method's arguments?

@michalbundyra
Copy link
Member

We can add it to parameter in the constructor, set it to property and use in connect inside.
It is not the nicest solution but it will work without BC Break.
We can also add another method: "connectSsl" or something similar where ssl will be set to true but we will be able provide param if we want verify cert or not.

There are multiple options to implement it without BC break, we just need to choose the best one, which will be the best for the future major release.

BTW. Thanks for creating this PR :) 👍

@abulhol
Copy link
Author

abulhol commented Oct 18, 2019

I see, thanks! You can close this PR, I will try to implement it like this and create a new PR.

@michalbundyra
Copy link
Member

@abulhol no need for new PR, you can continue on this.
Please also remember to update the documentation !

Thanks again :)

@michalbundyra
Copy link
Member

@abulhol Also, remember to target develop branch instead of master as it is feature, so will be planned for next minor release (you can change it on editing PR, but you need also rebase the branch).

@abulhol
Copy link
Author

abulhol commented Oct 18, 2019

I have created a PR to the develop branch (I did not manage to redirect this one here).
Where do I update the documentation? I mean this here: https://docs.zendframework.com/zend-mail/

@froschdesign
Copy link
Member

@abulhol
See the Markdown files in https://github.com/zendframework/zend-mail/tree/master/docs/book

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants