Bug 1550945 - Part 5: Fix tests so they don't create URL via protocol handler, rework SMTP URL creation. rs=bustage-fix
authorJorg K <jorgk@jorgk.com>
Wed, 29 May 2019 23:29:55 +0200
changeset 35712 5c0f43def1f3bcb2f0a487a006338ef056e084b2
parent 35711 69ac929f4cbbe773fa321c0d501bb7731dcad62f
child 35713 f5addc9ca311ff981e651dba9b1cf33752714145
push id392
push userclokep@gmail.com
push dateMon, 02 Sep 2019 20:17:19 +0000
reviewersbustage-fix
bugs1550945
Bug 1550945 - Part 5: Fix tests so they don't create URL via protocol handler, rework SMTP URL creation. rs=bustage-fix
mailnews/base/util/nsNewMailnewsURI.cpp
mailnews/compose/public/nsISmtpUrl.idl
mailnews/compose/src/nsSmtpService.cpp
mailnews/compose/src/nsSmtpService.h
mailnews/compose/src/nsSmtpUrl.cpp
mailnews/compose/test/unit/test_smtpProtocols.js
mailnews/imap/test/unit/test_imapProtocols.js
mailnews/local/test/unit/test_mailboxProtocol.js
mailnews/news/test/unit/head_server_setup.js
mailnews/news/test/unit/test_nntpProtocols.js
mailnews/news/test/unit/test_nntpUrl.js
mailnews/news/test/unit/test_server.js
mailnews/news/test/unit/test_uriParser.js
--- a/mailnews/base/util/nsNewMailnewsURI.cpp
+++ b/mailnews/base/util/nsNewMailnewsURI.cpp
@@ -6,17 +6,16 @@
 #include "nsNewMailnewsURI.h"
 #include "nsURLHelper.h"
 #include "nsSimpleURI.h"
 #include "nsStandardURL.h"
 
 #include "../../local/src/nsPop3Service.h"
 #include "../../local/src/nsMailboxService.h"
 #include "../../compose/src/nsSmtpService.h"
-#include "../../compose/src/nsSmtpUrl.h"
 #include "../../../ldap/xpcom/src/nsLDAPURL.h"
 #include "../../imap/src/nsImapService.h"
 #include "../../news/src/nsNntpService.h"
 #include "../../addrbook/src/nsAddbookProtocolHandler.h"
 #include "../src/nsCidProtocolHandler.h"
 
 nsresult NS_NewMailnewsURI(nsIURI** aURI, const nsACString& aSpec,
                            const char* aCharset /* = nullptr */,
@@ -28,30 +27,27 @@ nsresult NS_NewMailnewsURI(nsIURI** aURI
 
   if (scheme.EqualsLiteral("mailbox") ||
       scheme.EqualsLiteral("mailbox-message")) {
     return nsMailboxService::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
   if (scheme.EqualsLiteral("imap") || scheme.EqualsLiteral("imap-message")) {
     return nsImapService::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
-  if (scheme.EqualsLiteral("smtp")) {
-    rv = NS_MutateURI(new nsSmtpUrl::Mutator()).SetSpec(aSpec).Finalize(aURI);
-    NS_ENSURE_SUCCESS(rv, rv);
-    nsCOMPtr<nsISmtpUrl> url(do_QueryInterface(*aURI));
-    NS_ENSURE_TRUE(url, NS_ERROR_UNEXPECTED);
-    return url->Init(aSpec);
+  if (scheme.EqualsLiteral("smtp") || scheme.EqualsLiteral("smtps")) {
+    return nsSmtpService::NewSmtpURI(aSpec, aCharset, aBaseURI, aURI);
   }
   if (scheme.EqualsLiteral("mailto")) {
-    return nsSmtpService::NewURI(aSpec, aCharset, aBaseURI, aURI);
+    return nsSmtpService::NewMailtoURI(aSpec, aCharset, aBaseURI, aURI);
   }
   if (scheme.EqualsLiteral("pop3")) {
     return nsPop3Service::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
-  if (scheme.EqualsLiteral("news") || scheme.EqualsLiteral("snews")) {
+  if (scheme.EqualsLiteral("news") || scheme.EqualsLiteral("snews") ||
+      scheme.EqualsLiteral("news-message") || scheme.EqualsLiteral("nntp")) {
     return nsNntpService::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
   if (scheme.EqualsLiteral("cid")) {
     return nsCidProtocolHandler::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
   if (scheme.EqualsLiteral("addbook")) {
     return nsAddbookProtocolHandler::NewURI(aSpec, aCharset, aBaseURI, aURI);
   }
--- a/mailnews/compose/public/nsISmtpUrl.idl
+++ b/mailnews/compose/public/nsISmtpUrl.idl
@@ -10,17 +10,16 @@ interface nsIMsgIdentity;
 interface nsIPrompt;
 interface nsIAuthPrompt;
 interface nsISmtpServer;
 interface nsIInterfaceRequestor;
 interface nsIFile;
 
 [scriptable, uuid(da22b8ac-059d-4f82-bf99-f5f3d3c8202d)]
 interface nsISmtpUrl : nsISupports {
-  void init(in AUTF8String aSpec);
   /**
    * SMTP Parse specific getters.
    * These retrieve various parts from the url.
    */
 
   /**
    * This list is a list of all recipients to send the email to.
    * Each name is NULL terminated.
--- a/mailnews/compose/src/nsSmtpService.cpp
+++ b/mailnews/compose/src/nsSmtpService.cpp
@@ -255,33 +255,56 @@ NS_IMETHODIMP nsSmtpService::GetProtocol
   *result = URI_NORELATIVE | ALLOWS_PROXY | URI_LOADABLE_BY_ANYONE |
             URI_NON_PERSISTABLE | URI_DOES_NOT_RETURN_DATA |
             URI_FORBIDS_COOKIE_ACCESS;
   return NS_OK;
 }
 
 // the smtp service is also the protocol handler for mailto urls....
 
-nsresult nsSmtpService::NewURI(
+nsresult nsSmtpService::NewMailtoURI(
     const nsACString &aSpec,
     const char *aOriginCharset,  // ignored, always UTF-8.
     nsIURI *aBaseURI, nsIURI **_retval) {
-  // get a new smtp url
   nsresult rv;
 
   nsCOMPtr<nsIURI> mailtoUrl;
   rv = NS_MutateURI(new nsMailtoUrl::Mutator())
            .SetSpec(aSpec)
            .Finalize(mailtoUrl);
   NS_ENSURE_SUCCESS(rv, rv);
 
   mailtoUrl.forget(_retval);
   return NS_OK;
 }
 
+nsresult nsSmtpService::NewSmtpURI(const nsACString &aSpec,
+                                   const char *aOriginCharset, nsIURI *aBaseURI,
+                                   nsIURI **_retval) {
+  NS_ENSURE_ARG_POINTER(_retval);
+  *_retval = 0;
+  nsresult rv;
+  nsCOMPtr<nsIMsgMailNewsUrl> aSmtpUri =
+      do_CreateInstance(NS_SMTPURL_CONTRACTID, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (aBaseURI) {
+    nsAutoCString newSpec;
+    rv = aBaseURI->Resolve(aSpec, newSpec);
+    NS_ENSURE_SUCCESS(rv, rv);
+    rv = aSmtpUri->SetSpecInternal(newSpec);
+    NS_ENSURE_SUCCESS(rv, rv);
+  } else {
+    rv = aSmtpUri->SetSpecInternal(aSpec);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  aSmtpUri.forget(_retval);
+
+  return rv;
+}
+
 NS_IMETHODIMP nsSmtpService::NewChannel(nsIURI *aURI, nsILoadInfo *aLoadInfo,
                                         nsIChannel **_retval) {
   NS_ENSURE_ARG_POINTER(aURI);
   // create an empty pipe for use with the input stream channel.
   nsCOMPtr<nsIAsyncInputStream> pipeIn;
   nsCOMPtr<nsIAsyncOutputStream> pipeOut;
   nsCOMPtr<nsIPipe> pipe = do_CreateInstance("@mozilla.org/pipe;1");
   nsresult rv = pipe->Init(false, false, 0, 0);
--- a/mailnews/compose/src/nsSmtpService.h
+++ b/mailnews/compose/src/nsSmtpService.h
@@ -18,19 +18,24 @@
 // to urls easier. I'm not sure if this service will go away when the new
 // networking model comes on line (as part of the N2 project). So I reserve the
 // right to change my mind and take this service away =).
 ////////////////////////////////////////////////////////////////////////////////////////
 
 class nsSmtpService : public nsISmtpService, public nsIProtocolHandler {
  public:
   nsSmtpService();
-  static nsresult NewURI(const nsACString &aSpec,
-                         const char *aOriginCharset,  // ignored, always UTF-8.
-                         nsIURI *aBaseURI, nsIURI **_retval);
+  static nsresult NewMailtoURI(
+      const nsACString &aSpec,
+      const char *aOriginCharset,  // ignored, always UTF-8.
+      nsIURI *aBaseURI, nsIURI **_retval);
+  static nsresult NewSmtpURI(
+      const nsACString &aSpec,
+      const char *aOriginCharset,  // ignored, always UTF-8.
+      nsIURI *aBaseURI, nsIURI **_retval);
   NS_DECL_ISUPPORTS
 
   ////////////////////////////////////////////////////////////////////////
   // we support the nsISmtpService interface
   ////////////////////////////////////////////////////////////////////////
   NS_DECL_NSISMTPSERVICE
 
   //////////////////////////////////////////////////////////////////////////
--- a/mailnews/compose/src/nsSmtpUrl.cpp
+++ b/mailnews/compose/src/nsSmtpUrl.cpp
@@ -572,19 +572,16 @@ nsSmtpUrl::nsSmtpUrl() : nsMsgMailNewsUr
 
   m_isPostMessage = true;
   m_requestDSN = false;
   m_verifyLogon = false;
 }
 
 nsSmtpUrl::~nsSmtpUrl() {}
 
-NS_IMETHODIMP
-nsSmtpUrl::Init(const nsACString &aSpec) { return SetSpecInternal(aSpec); }
-
 NS_IMPL_ISUPPORTS_INHERITED(nsSmtpUrl, nsMsgMailNewsUrl, nsISmtpUrl)
 
 ////////////////////////////////////////////////////////////////////////////////////
 // Begin nsISmtpUrl specific support
 
 ////////////////////////////////////////////////////////////////////////////////////
 
 NS_IMETHODIMP
--- a/mailnews/compose/test/unit/test_smtpProtocols.js
+++ b/mailnews/compose/test/unit/test_smtpProtocols.js
@@ -1,12 +1,13 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /*
  * Test suite for getting smtp urls via the protocol handler.
  */
+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var defaultProtocolFlags =
   Ci.nsIProtocolHandler.URI_NORELATIVE |
   Ci.nsIProtocolHandler.URI_DANGEROUS_TO_LOAD |
   Ci.nsIProtocolHandler.URI_NON_PERSISTABLE |
   Ci.nsIProtocolHandler.ALLOWS_PROXY |
   Ci.nsIProtocolHandler.URI_FORBIDS_AUTOMATIC_DOCUMENT_REPLACEMENT;
 
@@ -31,17 +32,17 @@ function run_test() {
     Assert.equal(pH.defaultPort, protocols[part].defaultPort);
     Assert.equal(pH.protocolFlags, defaultProtocolFlags);
 
     // Whip through some of the ports to check we get the right results.
     for (let i = 0; i < 1024; ++i)
       Assert.equal(pH.allowPort(i, ""), (i == protocols[part].defaultPort));
 
     // Check we get a URI when we ask for one
-    var uri = pH.newURI(protocols[part].urlSpec);
+    var uri = Services.io.newURI(protocols[part].urlSpec);
 
     uri.QueryInterface(Ci.nsISmtpUrl);
 
     Assert.equal(uri.spec, protocols[part].urlSpec);
 
     try {
       // This call should throw NS_ERROR_NOT_IMPLEMENTED. If it doesn't,
       // then we should implement a new test for it.
--- a/mailnews/imap/test/unit/test_imapProtocols.js
+++ b/mailnews/imap/test/unit/test_imapProtocols.js
@@ -1,12 +1,13 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /*
  * Test suite for IMAP nsIProtocolHandler implementations.
  */
+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var defaultProtocolFlags =
   Ci.nsIProtocolHandler.URI_NORELATIVE |
   Ci.nsIProtocolHandler.URI_FORBIDS_AUTOMATIC_DOCUMENT_REPLACEMENT |
   Ci.nsIProtocolHandler.URI_DANGEROUS_TO_LOAD |
   Ci.nsIProtocolHandler.ALLOWS_PROXY |
   Ci.nsIProtocolHandler.URI_FORBIDS_COOKIE_ACCESS |
   Ci.nsIProtocolHandler.ORIGIN_IS_FULL_SPEC;
@@ -41,15 +42,15 @@ function run_test() {
     Assert.equal(pH.protocolFlags, defaultProtocolFlags);
 
     // Whip through some of the ports to check we get the right results.
     // IMAP allows connecting to any port.
     for (let i = 0; i < 1024; ++i)
       Assert.ok(pH.allowPort(i, ""));
 
     // Check we get a URI when we ask for one
-    var uri = pH.newURI(protocols[part].urlSpec);
+    var uri = Services.io.newURI(protocols[part].urlSpec);
 
     uri.QueryInterface(Ci.nsIImapUrl);
 
     Assert.equal(uri.spec, protocols[part].urlSpec);
   }
 }
--- a/mailnews/local/test/unit/test_mailboxProtocol.js
+++ b/mailnews/local/test/unit/test_mailboxProtocol.js
@@ -1,12 +1,13 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /*
  * Test suite for getting mailbox urls via the protocol handler.
  */
+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var defaultProtocolFlags =
   Ci.nsIProtocolHandler.URI_NORELATIVE |
   Ci.nsIProtocolHandler.URI_DANGEROUS_TO_LOAD |
   Ci.nsIProtocolHandler.URI_FORBIDS_AUTOMATIC_DOCUMENT_REPLACEMENT |
   Ci.nsIProtocolHandler.URI_FORBIDS_COOKIE_ACCESS |
   Ci.nsIProtocolHandler.ORIGIN_IS_FULL_SPEC;
 
@@ -29,17 +30,17 @@ function run_test() {
     Assert.equal(pH.defaultPort, protocols[part].defaultPort);
     Assert.equal(pH.protocolFlags, defaultProtocolFlags);
 
     // Whip through some of the ports to check we get the right results.
     for (let i = 0; i < 1024; ++i)
       Assert.equal(pH.allowPort(i, ""), false);
 
     // Check we get a URI when we ask for one
-    var uri = pH.newURI(protocols[part].urlSpec);
+    var uri = Services.io.newURI(protocols[part].urlSpec);
 
     uri.QueryInterface(Ci.nsIMailboxUrl);
 
     Assert.equal(uri.spec, protocols[part].urlSpec);
 
     // XXX This fails on Windows
     // do_check_neq(pH.newChannel(uri), null);
   }
--- a/mailnews/news/test/unit/head_server_setup.js
+++ b/mailnews/news/test/unit/head_server_setup.js
@@ -131,25 +131,23 @@ function setupLocalServer(port, host = "
   subscribeServer(server);
 
   _server = server;
   _account = serverAndAccount.account;
 
   return server;
 }
 
-var URLCreator = MailServices.nntp.QueryInterface(Ci.nsIProtocolHandler);
-
 // Sets up a protocol object and prepares to run the test for the news url
 function setupProtocolTest(port, newsUrl, incomingServer) {
   var url;
   if (newsUrl instanceof Ci.nsIMsgMailNewsUrl) {
     url = newsUrl;
   } else {
-    url = URLCreator.newURI(newsUrl);
+    url = Services.io.newURI(newsUrl);
   }
 
   var newsServer = incomingServer;
   if (!newsServer)
     newsServer = setupLocalServer(port);
 
   var listener = {
     onStartRequest() {},
@@ -163,17 +161,17 @@ function setupProtocolTest(port, newsUrl
     onDataAvailable() {},
     QueryInterface: ChromeUtils.generateQI(["nsIStreamListener"]),
   };
   listener.called = false;
   newsServer.loadNewsUrl(url, null, listener);
 }
 
 function create_post(baseURL, file) {
-  var url = URLCreator.newURI(baseURL);
+  var url = Services.io.newURI(baseURL);
   url.QueryInterface(Ci.nsINntpUrl);
 
   var post = Cc["@mozilla.org/messenger/nntpnewsgrouppost;1"]
                .createInstance(Ci.nsINNTPNewsgroupPost);
   post.postMessageFile = do_get_file(file);
   url.messageToPost = post;
   return url;
 }
--- a/mailnews/news/test/unit/test_nntpProtocols.js
+++ b/mailnews/news/test/unit/test_nntpProtocols.js
@@ -1,12 +1,13 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /*
  * Test suite for getting news urls via the protocol handler.
  */
+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
 var defaultProtocolFlags =
   Ci.nsIProtocolHandler.URI_NORELATIVE |
   Ci.nsIProtocolHandler.URI_LOADABLE_BY_ANYONE |
   Ci.nsIProtocolHandler.ALLOWS_PROXY |
   Ci.nsIProtocolHandler.URI_FORBIDS_AUTOMATIC_DOCUMENT_REPLACEMENT |
   Ci.nsIProtocolHandler.URI_FORBIDS_COOKIE_ACCESS |
   Ci.nsIProtocolHandler.ORIGIN_IS_FULL_SPEC;
@@ -36,15 +37,15 @@ function run_test() {
     Assert.equal(pH.protocolFlags, defaultProtocolFlags);
 
     // Whip through some of the ports to check we get the right results.
     // NEWS allows connecting to any port.
     for (let i = 0; i < 1024; ++i)
       Assert.ok(pH.allowPort(i, ""));
 
     // Check we get a URI when we ask for one
-    var uri = pH.newURI(protocols[part].urlSpec);
+    var uri = Services.io.newURI(protocols[part].urlSpec);
 
     uri.QueryInterface(Ci.nsINntpUrl);
 
     Assert.equal(uri.spec, protocols[part].urlSpec);
   }
 }
--- a/mailnews/news/test/unit/test_nntpUrl.js
+++ b/mailnews/news/test/unit/test_nntpUrl.js
@@ -1,23 +1,22 @@
 /* -*- Mode: JavaScript; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* ***** BEGIN LICENSE BLOCK *****
  *
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/licenses/publicdomain/
  *
  * ***** END LICENSE BLOCK ***** */
 
+var {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 var {MailServices} = ChromeUtils.import("resource:///modules/MailServices.jsm");
 
 function getMessageHeaderFromUrl(aUrl) {
-  let msgUrl = MailServices.nntp
-                .QueryInterface(Ci.nsIProtocolHandler)
-                .newURI(aUrl)
-                .QueryInterface(Ci.nsIMsgMessageUrl);
+  let msgUrl = Services.io.newURI(aUrl)
+                       .QueryInterface(Ci.nsIMsgMessageUrl);
   return msgUrl.messageHeader;
 }
 
 function run_test() {
   // This is crash test for Bug 392729
   try {
     // msgkey is invalid for news:// protocol
     getMessageHeaderFromUrl("news://localhost:119" +
--- a/mailnews/news/test/unit/test_server.js
+++ b/mailnews/news/test/unit/test_server.js
@@ -87,17 +87,17 @@ function testRFC977() {
 
 function testConnectionLimit() {
   var server = makeServer(NNTP_RFC977_handler, daemon);
   server.start(NNTP_PORT);
 
   var prefix = "news://localhost:" + NNTP_PORT + "/";
 
   // To test make connections limit, we run two URIs simultaneously.
-  var url = URLCreator.newURI(prefix + "*");
+  var url = Services.io.newURI(prefix + "*");
   _server.loadNewsUrl(url, null, null);
   setupProtocolTest(NNTP_PORT, prefix + "TSS1@nntp.invalid");
   server.performTest();
   // We should have length one... which means this must be a transaction object,
   // containing only us and them
   Assert.ok("us" in server.playTransaction());
   server.stop();
 
@@ -118,17 +118,17 @@ function testReentrantClose() {
       // Spin the event loop (entering nsNNTPProtocol::ProcessProtocolState)
       let thread = gThreadManager.currentThread;
       while (thread.hasPendingEvents())
         thread.processNextEvent(true);
     },
   };
   // Nice multi-step command--we can close while executing this URL if we are
   // careful.
-  var url = URLCreator.newURI("news://localhost:" + NNTP_PORT +
+  var url = Services.io.newURI("news://localhost:" + NNTP_PORT +
     "/test.filter");
   url.QueryInterface(Ci.nsIMsgMailNewsUrl);
   url.RegisterListener(listener);
 
   _server.loadNewsUrl(url, null, null);
   server.performTest("GROUP");
   dump("Stopping server\n");
   gThreadManager.currentThread.dispatch({
--- a/mailnews/news/test/unit/test_uriParser.js
+++ b/mailnews/news/test/unit/test_uriParser.js
@@ -137,33 +137,32 @@ var invalid_uris = [
   "nntp://localhost/a.group/hello",
   "nntp://localhost/a.group/0",
   "nntp:a.group",
 ];
 
 function run_test() {
   // We're not running the server, just setting it up
   localserver = setupLocalServer(119);
-  let nntpService = MailServices.nntp.QueryInterface(Ci.nsIProtocolHandler);
   for (let test of tests) {
     dump("Checking URL " + test.uri + "\n");
-    let url = nntpService.newURI(test.uri);
+    let url = Services.io.newURI(test.uri);
     url.QueryInterface(Ci.nsIMsgMailNewsUrl);
     url.QueryInterface(Ci.nsINntpUrl);
     for (let prop in test) {
       if (prop == "uri")
         continue;
       Assert.equal(url[prop], test[prop]);
     }
   }
 
   for (let fail of invalid_uris) {
     try {
       dump("Checking URL " + fail + " for failure\n");
-      nntpService.newURI(fail);
+      Services.io.newURI(fail);
       Assert.ok(false);
     } catch (e) {
       Assert.equal(e.result, Cr.NS_ERROR_MALFORMED_URI);
     }
   }
 
   // The password migration is async, so trigger an event to prevent the logon
   // manager from trying to migrate after shutdown has started.