URL filter: Use filter_allowed_protocols variable

Works and passes for me.

+++ modules/filter/filter.module	4 Mar 2010 19:04:29 -0000
@@ -1111,8 +1111,19 @@ function _filter_url($text, $filter) {
+  foreach ($protocols as $key => $value) {
+    $protocols[$key] .= ':';
+    if ($value != 'mailto') {
+      $protocols[$key] .= '//';
+    }
+  }

I think we should drop this magic and add "://" to each entry in the $protocols array. Why? RFC 1738 (Uniform Resource Locators) describes a URL as follows: <scheme>:<scheme-specific-part>. The "//" is used to indicate that the scheme-specific-part (not the scheme itself!) complies with a common scheme syntax (i.e. username@password:domain:port). Having '//' is not a hard requirement. For example, a news URL takes one of two forms:

     news:<newsgroup-name>
     news:<message-id>

The nntp:// URL scheme is an alternative method of referencing news articles but news: is valid too. In other words, the current code makes assumptions that are not part of the specification. That is why I recommend removing the magic to automatically add "://".

PS: Skype uses 'callto://' which might be worth adding?

+++ modules/filter/filter.module	4 Mar 2010 19:04:29 -0000
@@ -1111,8 +1111,19 @@ function _filter_url($text, $filter) {
+  // @see filter_xss_bad_protocol()
+  $protocols = variable_get('filter_allowed_protocols', array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'telnet', 'webcal'));

Hm. Problem with that is that the filter_allowed_protocols variable is effectively *re-used* here. The variable already exists and is used like since always in filter_xss_bad_protocol().

And filter_xss_bad_protocol() would become very complex if the protocols would already contain those suffixes/separators.

I'd therefore recommend to just enhance the PCRE to always optionally match (?://) after $protocols...

+++ modules/filter/filter.module	4 Mar 2010 19:04:29 -0000
@@ -1111,8 +1111,19 @@ function _filter_url($text, $filter) {
+  $text = preg_replace_callback("`(<p>|<li>|<br\s*/?>|[ \n\r\t\(])(({$protocols})([a-zA-Z0-9@:%_+*~#?&=.,/;-]*[a-zA-Z0-9@:%_+*~#&=/;-]))([.,?!]*?)(?=(</p>|</li>|<br\s*/?>|[ \n\r\t\)]))`i", '_filter_url_parse_full_links', $text);

d'oh. Looking more closely at the already matches characters after $protocols, '//' is already matched.

Powered by Dreditor.

Works!

Whether skype: or any other protocol belongs into this default list is a separate issue. At first sight, my gut feeling was: If we add skype:, then what about xmpp: or jabber: or whatever else? Also, Skype is proprietary... and, heh, searching for "skype" in contrib quickly revealed: http://drupal.org/project/filter_protocols by Dave Reid ;) We may want to consider to move that functionality into core.

This looks pretty RTBC to me, and I'd like to move on to other input filter processing bugs in the queue, from which most need to change the very same lines of code.

As mentioned before, I'd really like to see a separate issue to quickly move http://drupal.org/project/filter_protocols into core.

Wait.

Would it make sense to configure the list of protocols per text format?

Would it make sense to configure the list of protocols per text format?

No, let's defer that to D8.

RTBC, anyone?

Coming in from #753278: URL Filter converts unsupported protocols and this looks good.

The project description page for my Drupher module has a similar situation to what corey.aufang is describing in #753278: URL Filter converts unsupported protocols. I typed in a Gopher URL and the filter is unhelpfully chopping off the "gopher:" from the front when it automatically linkifies it. Not linkifying it at all would have been better behavior.

#5: drupal.filter-url-protocols.5.patch queued for re-testing.

Postponing on #161217: URL filter breaks generated href tags

This had to be incorporated into #161217: URL filter breaks generated href tags to make the new URL filter tests pass.