Periodically Update TLD list from Current IANA List

New patch for this issue

Look good for me.

I like the idea of this, and wouldn't mind it with a couple of changes:

1) I've already added code that allows users to update the list of TLDs with a variable get, and created a function called _link_domains() that does a similar job to _link_get_TLD_regex. Personally, I prefer the function name _link_domains(), especially since:

2) I don't like the idea that a call to validate a value will trigger an attempt to load data from offsite - I'd rather we restricted that entirely to hook_cron, and just checked the current value when validating.

If you can fix those two up, I think we might have a winner!

This is an alternative solution adding a drush command instead writing a file. It's feels a bit too much for a variable.

Also, the patch above doesn't deal with Punycode. My version considers both the Punycode encoded and decoded versions valid. That's the only reason for the size of the patch, otherwise it's really small. The userspace Punycode library used is https://github.com/true/php-punycode/ MIT so GPL compatible.

Patch #5 has worked great for me.

Patch applied cleanly, ran cron, tested 'drush vget link_iana_domains' and had a whole load of lovely domains.

Tested on a field, the validation that was previously causing a '.global' domain to fail now allowed the new domain.

Added back the drush command -- sorry for leaving it out. And I guess the important part of this, the Punycode part could be merged with #5. #5 IMO could just do DrupalQueue::get('link_iana')->createItem(NULL) and leave the loop out of cron and the argument out in the worker callback. And perhaps it could use the file approach? Not sure but I do not like having such a huge variable around loaded for each page.

Oh. Yeah. Ancient PHP.

Nevermind. #5 is the one to review. It's impossible to make this 5.2 compatible since it's namespace and it's not worth the bothering. Feel free to use it if you have at least 5.4.

OK, one last try.

OK, withdrawn. I tried but I can't put in more time to get it working with such an ancient PHP. #5 is the one to review.

Re-rolling.

#5 works nicely here, updating the list of LTDs via cron.

I tested it too, works for me. I run drush manually though, and don't see anything that would pull it automatically on cron runs? I guess we need to manually add the "drush link-extra-domains" task to our crons... Right?
In any case, working for me too. Thanks!

If you use patch #5 then the normal Drupal cron task will do the work for you, as the updating is done via hook_cron() once per day. All Drupal sites need cron, if only the built-in fake cron, to do things like checking for updates, update the search index, etc.

If you use patch #21 then you'd need to explicitly call the drush command separately from cron at whatever frequency you wish. Gives a bit more flexibility, perhaps, but I prefer patch #5 as being more "plug and play".

Thanks for the feedback :)

I understood we were testing/using #5. I tested that patch.
cron: Did not see in the patch the cron_hook so I assumed it wasn't being called.
Any way we can test the pull is being done on the cron run?

This is new version doing a variable set rather than dumping to a file, otherwise the same as #21.

#28 I have not tested Cyrillic addresses (PunyCode)

#21 I have not tested Cyrillic addresses (PunyCode)
(The value http://www.сайт.рф provided for Link is not a valid URL)

Coming over here from #2299657: Allow any TLD because site admins can never keep up with ICANN.

Although neat, this seems like an overly complicated bit of code to maintain. Perhaps we'd be better off if the logic in Link module were reversed. Instead of including a white-listing by default, it instead allowed all domain names (using a regex instead). To keep the functionality of strictly allowing certain (or valid) domain names, we could change Link's current "link_additional_domains" to "link_allowed_domains". If empty (the default) then all domains (even invalid ones) would be allowed. This would be good enough for almost all Link module users. If you needed strict checking, you could use a contrib module to maintain the list via cron jobs and update this variable.

Or, better yet, you'd probably want real link checking and just validate that the server can actually hit the URL directly (see #2262307: Add Server Validation Option).

But in general, I think both concepts are a too heavy for Link module. I'd prefer to see these solutions implemented as add-on modules.

Also, if this were to move forward. I think you could avoid using any custom Punycode and just using the PHP built-in functions for domain conversion: idn_to_ascii and its opposite idn_to_utf8, available in PHP 5.3.0 and higher.

oh! I had no idea that has been reimplemented in PHP core without intl.

If this functionality is added to a add-on module, we should adopt blacklisting instead of whitelisting as #31 proposes. Link should accept every url regardless, and the link submodule could be called link_validate or something similar.

oh! I had no idea that has been reimplemented in PHP core without intl.

Well, I just found out that indeed it's *not* in PHP core all the time. If you have PHP 5.3 and have not compiled with --enable-intl, it's not present. PHP 5.4 appears to have it all the time.

This output can be reproduced easily enough at https://3v4l.org/UuOYG, though on my localhost with MAMP, intl is enabled so the function exists in PHP 5.3.

In any case, at this point I think that's a suitable level of acceptance. Perhaps just check for the function if it exists before escaping.

So something like this?

function _link_validate(&$item, $delta, $field, $entity, $instance, $langcode, &$optional_field_found, &$errors) {
  if ($item['url'] && !(isset($instance['default_value'][$delta]['url']) && $item['url'] === $instance['default_value'][$delta]['url'] && !$instance['required'])) {
    // Validate the link.
    if (version_compare(PHP_VERSION, '5.4.0') >= 0 || extension_loaded('intl')) {
      // validate domain
      $domain = idn_to_ascii($item['url']);

      // ..... continue
    }
  }
}

Just a straight check on the function is a lot simpler and likely to be more accurate:

    // Validate the link.
    if (function_exists('idn_to_ascii') {
      // validate domain
      $domain = idn_to_ascii($item['url'])
   }

However, you need to split out the domain portion of the URL, you can't run that function on the entire thing, otherwise it will convert other parts of the URL like the path or query string as well.

I agree with quicksketch's general ideas in #31 and later to simplify link core code, and make full validation optional in a submodule. The perfunctory validation in the main module can be a lot like email address validation, just a regex to catch obvious issues and prevent script embedding and such.

that's are right for me,

Hi,

I've just add a hook_cron every month and call on install or update

Thanks

Without the name-error on hook_cron

Please don't include code for other contrib projects in the patches you provide as they won't apply. Which you just did!!!

Ohh.. sorry

That's better ?

I have a .photo domain and confirm I can use that now with Link after applying the patch from #44. Many thanks!

Patch in #44 is working great, thank you very much!

I re-rolled the patch in #44 against current HEAD since the last one failed testing.

I tested patch #49 and once applied, was able to create a link to a url using a .farm extension domain.

It seems to be working but I don't like the way you did it. I mean, the module code should not call the drush command but declare its own helper that drush is going to use as well as the cron, the install and update scripts.
One should not have to open the drush.inc file to understand how the module works.

I also dislike the dependency on Punycode. Don't we have enough tools in our API to deal with that file properly?
If the answer is "no" I suggest to put that Punycode declaration in a separate include file.

One more thought is that we should provide a button somewhere in the UI to allow the site admin to force the refresh of the list. Sadly, not everyone know how to use drush or delete a variable.

This is a great solution but there are still some cases that are not supported:
1. Tor, the anonymity network, defines its own TLD (.onion) .
2. Custom TLDs, internal to private networks are not supported

For 1. we can hardcode the .onion domain in the variable
For 2. we can add a drupal_alter.

What do you think?

I support the idea of an alter hook.
We could use it to properly provide the .onion extension instead of hardcoding it.

Instead of updating the list in hopes of ever keeping up with ICANN I'm going with a much simpler approach of just allowing any letters as top level domain: #2747561: Allow any TLD because site admins can never keep up with ICANN

Agree 100% with Klausi. There is no way for module maintainers to keep up with ever changing TLDs now that we have "Branded top-level domains" I now see that this issue is supposed to get around the issue of manually "keeping up" by an automated sync with a list, but then it also ties the codebase to a text file produced by the third party IANA website, which could break or stop working at any point in the future.

I like the idea of allowing any TLD and if someone wants to specifically prohibit some TLDs that should be doable in a submit validation hook or via a variable exposed in admin UI to let you blacklist domains.

A fix has been committed in #2299657: Allow any TLD because site admins can never keep up with ICANN this issue should be closed now. The fix in the linked issue is working.