Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct bugs for multiple SMTP server insertion #1094

Closed
wants to merge 1 commit into from

Conversation

eros23
Copy link

@eros23 eros23 commented Jul 17, 2017

preg_match scheme error

@Synchro
Copy link
Member

Synchro commented Jul 18, 2017

Can you provide test cases to show what this fixes please?

@Synchro
Copy link
Member

Synchro commented Jul 18, 2017

I've had more of a look at this: removing that : will break IPv6 literals, which is why it's there along with [ and ]. The match is not terribly thorough, but it's not meant to be.

@Synchro Synchro closed this Jul 18, 2017
@eros23
Copy link
Author

eros23 commented Jul 18, 2017

When i use ssl://smtp.name.it:25;tls://smtp.test2.it:586
The script creates this situation:
Connection: opening to tls://smtp.test2.it:586:25, timeout=300, options=array (....

It does not find the fourth optional parameter (port) and add the standard port.

This is for domain name. For ipv6, i think i need to make a special regex

For this:
^((ssl|tls):\/\/)*([a-zA-Z0-9\.-]*):?([0-9]*)$

I have:

Full match | 0-23 | tls://smtp.test2.it:586
Group 1. | 0-6 | tls://
Group 2. | 0-3 | tls
Group 3. | 6-19 | smtp.test2.it
Group 4. | 20-23 | 586

For this:
^((ssl|tls):\/\/)*([a-zA-Z0-9:\[\]\.-]*):?([0-9]*)$

I have:

Full match | 0-23 | tls://smtp.test2.it:586
Group 1. | 0-6 | tls://
Group 2. | 0-3 | tls
Group 3. | 6-23 | smtp.test2.it:586
Group 4. | 23-23 | null<------that is the question

Synchro added a commit that referenced this pull request Jul 26, 2017
Exercise more bad host checks
@Synchro
Copy link
Member

Synchro commented Jul 27, 2017

@eros23 You should find this is fixed now, and released as 5.2.24. I changed it to check for IPv6 separately.

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

Successfully merging this pull request may close these issues.

None yet

2 participants