Fix for #5894 - .il Domain checking #5900

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

moderndeveloperllc commented Mar 3, 2014

Fix for Issue #5894

  • .il validation for Hebrew and Latin domain names;
  • Update validTLDs from IANA website;
  • Add a few more Cyrillic IDN checking strings from IANA website;
  • Use UTF8 string wrapper for strlen calls;
  • Clean up TLD checking (if we are using getTldCheck() == true we don't need any further checking of the TLD validity);
  • Add new tests for some of the new UTF-8 TLDs

There is more cleaning up to do in this class:

  • TLDs like .asia can take Chinese, Japanese, and Korean characters. Current logic only allows one file to be imported at a time.
  • Should probably have a idn_to_utf8() short-circuit in decodePunycode() for those with the intl extension.
  • Need to check the other new TLDs like .tokyo to see if they use IDN strings too

moderndeveloperllc added some commits Mar 3, 2014

Fix for issue #5894 - .il validation for Hebrew and Latin domain name…
…s; update validTLDs from IANA website; Add a few more Cyrillic IDN checking strings from IANA website; Use UTF8 string wrapper for strlen calls; Clean up TLD checking (if we are using getTldCheck() == true we don't need any further checking of the TLD validity); Add new tests for some of the new UTF-8 TLDs
Contributor

moderndeveloperllc commented Mar 4, 2014

Not sure why Travis failed. The unit tests I updated passed and there were no formatting issues.

Member

Ocramius commented Mar 4, 2014

@moderndeveloperllc the failure is not related with your issue

@weierophinney weierophinney added this to the 2.2.6 milestone Mar 5, 2014

weierophinney added a commit that referenced this pull request Mar 5, 2014

weierophinney added a commit that referenced this pull request Mar 5, 2014

@weierophinney weierophinney self-assigned this Mar 5, 2014

@moderndeveloperllc moderndeveloperllc deleted the moderndeveloperllc:feature/5894 branch Mar 5, 2014

Contributor

dol commented on 9fe9f29 Mar 18, 2014

This commit is incomplete. No updated test cases are provided. I guess the commit breaks the validation of TLD's longer than 10 characters (e.g. .photography). See https://github.com/zendframework/zf2/blob/master/library/Zend/Validator/Hostname.php#L525

Contributor

moderndeveloperllc replied Mar 18, 2014

@dol Good catch. That regex isn't new logic, but I did miss it when I was updating the TLD checking logic. New tests were put in from non-latin TLDs, but not for one of the new, longer latin TLDs. I'll have to check and see if there are actually any restrictions on TLD length anymore. It'll make me check the list again too. In the new wild west of gTLDs, it grows on a daily basis.

Contributor

garte replied Mar 18, 2014

I already issued a PR #5992 addressing this issue.

weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015

weierophinney added a commit to zendframework/zend-validator that referenced this pull request May 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment