User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.108 Safari/537.36 OPR/50.0.2762.45 Steps to reproduce: I am importing few RSS feeds that have magnet: torrent links for article links. A handmade example of the feed contents attached. Actual results: Prior to 52.5.2 (I am now running 52.5.0 and it is ok) <link> tag contents were imported and shown as Website field along with Author and Subject, imported message source had proper <base href="..."> contents. Expected results: With 52.5.2, links are no more imported and shown in the Website field, imported message source <base href=""> is empty.

Seems like I've mixed up 'expected results' / 'actual results' in the form, but it should be clear what's wrong.

We've done some changes to RSS feeds between 52.5.0 and 52.5.2 due to a security issue/vulnerability, so maybe something is too restrictive now.

Probably so. Is there any way to relax this? :)

Seems like this is indeed broken with https://hg.mozilla.org/releases/comm-esr52/rev/01f2c40a0ce9 I understand it's to prevent dataurls and file:// references. "if (/^https?:\/\//.test(link))", maybe also allow 'magnet:' and 'ftp://' to be +/- complete?

Indeed, --- a/chrome/newsblog/content/messenger-newsblog/feed-parser.js.orig 2010-01-01 00:00:00.000000000 +0300 +++ b/chrome/newsblog/content/messenger-newsblog/feed-parser.js 2018-01-05 20:35:25.757459100 +0300 @@ -939,1 +939,1 @@ - if (/^https?:\/\//.test(link)) + if (/^((https?|ftp):\/\/|magnet:)/.test(link)) Did the trick. Can this be integrated somehow? People still may be using these kinds of links for something good :)

I'm not the feed or security expert here, so we should ask them. If they agree, we can integrate that change.

perhaps the reporter can submit a patch, or if jorg would be so kind as to create one for the one liner, that would be great. r+ in advance.

even if the magnet link points to evilsite.com it's no different than an http(s) protocol doing the same, i believe. but lets ask anyway.

I've uploaded a simple diff (the one line in question plus comment changed). Don't know if it's enough.

By the way, I'm not sure if ^ in the regexp should not be ^\s* like that: if (/^\s*((https?|ftp):\/\/|magnet:)/.test(link)) So links having some leading spaces by some occasion are allowed as well (trim is done immediately after check anyways).

OK, I prepared this in Alex's name, first patch, haha. I'm always on the lookout for a good commit message, so please suggest one. We had: Bug 1411716 - Disallow non http* <link> type urls (eg data: urls) so now: Bug 1428374 - Bug 1411716 follow-up: allow ftp: and magnet: type URLs. r=alta88

Comment on attachment 8940309 [details] [diff] [review] Plain diff solving the issue (adding ftp:// and magnet: link types) Looks like wires crossed here, sorry. Let's use my patch, I even dug out your name from your website for a proper header ;-) And I didn't add trailing spaces which are not well-regarded here.

Sure thing, it's better that way.

How do magnet: urls work? Do you have an extension that provides torrent support, or do you just click on the links and load them in your external torrent client? Either way seems reasonable to support. We will be dropping ftp: support in the future but I guess it doesn't hurt to include that one while it's still supported. The problem being solved in bug 1411716 was protocols that inherit the origin of their opening context, such as data: urls.

In my case, magnet: URLs work just the operating system association way, no special extensions. When first opened, the type was asked normal way for what is it to be opened with, being added to 'Attachments' configuration set with "Use <application> (default)".

So yes, it is external client receiving these links.

From what I read here, the patch will go ahead, also given that we have r+ in advance. So the last question to answer is whether we want to use if (/^\s*((https?|ftp):\/\/|magnet:)/.test(link)) Alex, Alta?

I did not check code paths leading to validLink() and whether they do trim() in advance, but there is link.trim() after checking, so using ^\s* variant here seems quite logical.

Comment on attachment 8940311 [details] [diff] [review] 1428374-ftp-magnet.patch I don't think testing for leading whitespace is necessary, only a publisher who is sloppy would do that, tough. The answer is always read the spec and that such accommodations penalize every other user on ever single parse of every proper feed.

Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/0af788114887 Bug 1411716 follow-up: allow ftp: and magnet: type URLs. r=alta88

Comment on attachment 8940311 [details] [diff] [review] 1428374-ftp-magnet.patch Too late for another TB 58 beta, but TB 59 beta is coming soon.

TB 52.6 ESR: https://hg.mozilla.org/releases/comm-esr52/rev/6ff6d626d269a73fac3835600266a37ee1bd26a8