Bug 355473 ��� Make FeedProtocolHandler.newChannel create a new real URI instead of modifying the caller's URI, and hack around bug 408599 by refusing in newURI to create feed URIs that will cause newChannel to create a channel with a spectacularly bad URI, r=sayrer, sr=bz, a=blocker

browser/components/feeds/src/FeedConverter.js

@@ -461,9 +461,8 @@ var FeedResultService = {
 };
 
 /**
- * A protocol handler that converts the URIs of Apple's various bogo protocol
+ * A protocol handler that attempts to deal with the variant forms of feed:
- * schemes into http, as they should be. Mostly, this object just forwards 
+ * URIs that are actually either http or https.
- * things through to the HTTP protocol handler.
  */
 function FeedProtocolHandler(scheme) {
   this._scheme = scheme;
@@ -491,6 +490,23 @@ FeedProtocolHandler.prototype = {
   },
   
   newURI: function FPH_newURI(spec, originalCharset, baseURI) {
+    // See bug 408599 - feed URIs can be either standard URLs of the form
+    // feed://example.com, in which case the real protocol is http, or nested
+    // URIs of the form feed:realscheme:. When realscheme is either http or
+    // https, we deal with the way that creates a standard URL with the
+    // realscheme as the host by unmangling in newChannel; for others, we fail
+    // rather than let it wind up loading something like www.realscheme.com//foo
+
+    const feedSlashes = "feed://";
+    const feedHttpSlashes = "feed:http://";
+    const feedHttpsSlashes = "feed:https://";
+    const NS_ERROR_MALFORMED_URI = 0x804B000A;
+
+    if (spec.substr(0, feedSlashes.length) != feedSlashes &&
+        spec.substr(0, feedHttpSlashes.length) != feedHttpSlashes &&
+        spec.substr(0, feedHttpsSlashes.length) != feedHttpsSlashes)
+      throw NS_ERROR_MALFORMED_URI;
+
     var uri = 
         Cc["@mozilla.org/network/standard-url;1"].
         createInstance(Ci.nsIStandardURL);
@@ -499,26 +515,28 @@ FeedProtocolHandler.prototype = {
     return uri;
   },
   
-  newChannel: function FPH_newChannel(uri) {
+  newChannel: function FPH_newChannel(aUri) {
     var ios = 
         Cc["@mozilla.org/network/io-service;1"].
         getService(Ci.nsIIOService);
     // feed: URIs either start feed://, in which case the real scheme is http:
     // or feed:http(s)://, (which by now we've changed to feed://realscheme//)
+    var feedSpec = aUri.spec;
     const httpsChunk = "feed://https//";
     const httpChunk = "feed://http//";
-    if (uri.spec.substr(0, httpsChunk.length) == httpsChunk)
+    if (feedSpec.substr(0, httpsChunk.length) == httpsChunk)
-      uri.spec = "https://" + uri.spec.substr(httpsChunk.length);
+      feedSpec = "https://" + feedSpec.substr(httpsChunk.length);
-    else if (uri.spec.substr(0, httpChunk.length) == httpChunk)
+    else if (feedSpec.substr(0, httpChunk.length) == httpChunk)
-      uri.spec = "http://" + uri.spec.substr(httpChunk.length);
+      feedSpec = "http://" + feedSpec.substr(httpChunk.length);
     else
-      uri.scheme = "http";
+      feedSpec = feedSpec.replace(/^feed/, "http");
 
+    var uri = ios.newURI(feedSpec, aUri.originCharset, null);
     var channel =
       ios.newChannelFromURI(uri, null).QueryInterface(Ci.nsIHttpChannel);
     // Set this so we know this is supposed to be a feed
     channel.setRequestHeader("X-Moz-Is-Feed", "1", false);
-    channel.originalURI = uri;
+    channel.originalURI = aUri;
     return channel;
   },
   

browser/components/feeds/test/Makefile.in

@@ -41,6 +41,10 @@ VPATH		= @srcdir@
 relativesrcdir  = browser/components/feeds/test
 
 include $(DEPTH)/config/autoconf.mk
+
+MODULE		= test_browser_feeds
+XPCSHELL_TESTS	= unit
+
 include $(topsrcdir)/config/rules.mk
 
 _TEST_FILES =	test_bug408328.html \

browser/components/feeds/test/unit/head_feeds.js

@@ -0,0 +1,5 @@
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+const Cr = Components.results;
+
+var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);

browser/components/feeds/test/unit/test_355473.js

@@ -0,0 +1,49 @@
+const NS_ERROR_MALFORMED_URI = 0x804B000A;
+
+function run_test() {
+  var feedFeedURI = ios.newURI("feed://example.com/feed.xml", null, null);
+  var httpFeedURI = ios.newURI("feed:http://example.com/feed.xml", null, null);
+  var httpURI = ios.newURI("http://example.com/feed.xml", null, null);
+
+  var httpsFeedURI =
+    ios.newURI("feed:https://example.com/feed.xml", null, null);
+  var httpsURI = ios.newURI("https://example.com/feed.xml", null, null);
+
+  var feedChannel = ios.newChannelFromURI(feedFeedURI, null);
+  var httpChannel = ios.newChannelFromURI(httpFeedURI, null);
+  var httpsChannel = ios.newChannelFromURI(httpsFeedURI, null);
+
+  // not setting .originalURI to the original URI is naughty
+  do_check_true(feedFeedURI.equals(feedChannel.originalURI));
+  do_check_true(httpFeedURI.equals(httpChannel.originalURI));
+  do_check_true(httpsFeedURI.equals(httpsChannel.originalURI));
+
+  // actually using the horrible mess that's a feed: URI is suicidal
+  do_check_true(httpURI.equals(feedChannel.URI));
+  do_check_true(httpURI.equals(httpChannel.URI));
+  do_check_true(httpsURI.equals(httpsChannel.URI));
+
+  // trying to create a feed URI with something non-http(s) as the inner URL
+  // should fail, until someone fixes bug 408599
+  try {
+    var dataFeedURI = ios.newURI("feed:data:text/xml,<rss/>", null, null);
+    do_throw("Not reached");
+  } catch (e if (e instanceof Ci.nsIException &&
+                 e.result == NS_ERROR_MALFORMED_URI)) {
+    // do nothing
+  }
+  try {
+    var ftpFeedURI = ios.newURI("feed:ftp://example.com/feed.xml", null, null);
+    do_throw("Not reached");
+  } catch (e if (e instanceof Ci.nsIException &&
+                 e.result == NS_ERROR_MALFORMED_URI)) {
+    // do nothing
+  }
+  try {
+    var fileFeedURI = ios.newURI("feed:file:///var/feed.xml", null, null);
+    do_throw("Not reached");
+  } catch (e if (e instanceof Ci.nsIException &&
+                 e.result == NS_ERROR_MALFORMED_URI)) {
+    // do nothing
+  }
+}