search

zendframework / zend-validator

Validator component from Zend Framework
BSD 3-Clause "New" or "Revised" License
181 stars 136 forks source link

Email validator hostname check for not empty #99

Open GeeH opened 8 years ago

GeeH commented 8 years ago

This issue has been moved from the zendframework repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.html

Original Issue: https://api.github.com/repos/zendframework/zendframework/issues/7138 User: @freax Created On: 2015-01-19T20:16:40Z Updated At: 2015-01-25T01:39:34Z Body Sometimes there is a warning because of empty hostname parameter in checkdnsrr function.

Comment

User: @Ocramius Created On: 2015-01-19T20:19:26Z Updated At: 2015-01-19T20:19:26Z Body @freax please rebase and provide a test case for the hostname affected by this bug.

Comment

User: @DASPRiD Created On: 2015-01-19T20:53:17Z Updated At: 2015-01-19T20:54:58Z Body @Ocramius adding a hostname to the test suite which could fail any time? Sounds like a bad idea.

Comment

User: @Ocramius Created On: 2015-01-19T22:40:55Z Updated At: 2015-01-19T22:40:55Z Body I mean that data-providers need to be adjusted ;-) On Jan 19, 2015 9:53 PM, "Ben Scholzen" notifications@github.com wrote:

@Ocramius https://github.com/Ocramius adding a hostname to the test suit which could fail any time? Sounds like a bad idea.

— Reply to this email directly or view it on GitHub https://github.com/zendframework/zf2/pull/7138#issuecomment-70558642.

Comment

User: @freax Created On: 2015-01-20T17:12:07Z Updated At: 2015-01-20T17:12:07Z Body I don't think this PR needs a test case, because it's nothing to break actually. This condition fails if hostname will be empty anyway, but this PR provides no more thipple warning in this case. I got this warnings twise and I can't reproduce it. Actually, I think it's a pretty rare case when script can't get MX-record for hostname.

Comment

User: @Ocramius Created On: 2015-01-25T01:39:34Z Updated At: 2015-01-25T01:39:34Z Body

I don't think this PR needs a test case, because it's nothing to break actually.

code changes affecting branching of execution paths always require testing, regardless of the impact they have.

This condition fails if hostname will be empty anyway, but this PR provides no more thipple warning in this case.

And your test should actually try to reproduce the warning instead: reaching the point where you can reproduce the warning will also give you a better understanding the problem.

Actually, I think it's a pretty rare case when script can't get MX-record for hostname.

You can inject a fake hostname validator to get to that.

weierophinney commented 4 years ago

This repository has been closed and moved to laminas/laminas-validator; a new issue has been opened at https://github.com/laminas/laminas-validator/issues/30.