From 4dbf37708a2d03a904b1fae9a1a3853b547c19a5 Mon Sep 17 00:00:00 2001 From: David Bienvenu Date: Mon, 2 Aug 2010 11:24:11 -0700 Subject: [PATCH] fix handling of stat errors and don't stat noselect folders when checking individual folders for new messsages, r/sr=standard8 bug 581707 --- mailnews/imap/src/nsImapIncomingServer.cpp | 27 ++-- .../imap/test/unit/test_dontStatNoSelect.js | 119 ++++++++++++++++++ mailnews/test/fakeserver/imapd.js | 12 +- 3 files changed, 141 insertions(+), 17 deletions(-) create mode 100644 mailnews/imap/test/unit/test_dontStatNoSelect.js diff --git a/mailnews/imap/src/nsImapIncomingServer.cpp b/mailnews/imap/src/nsImapIncomingServer.cpp index 3cb1aca1f9..7e1483c221 100644 --- a/mailnews/imap/src/nsImapIncomingServer.cpp +++ b/mailnews/imap/src/nsImapIncomingServer.cpp @@ -2366,7 +2366,7 @@ nsImapIncomingServer::OnStopRunningUrl(nsIURI *url, nsresult exitCode) } // if we get an error running the url, it's better // not to chain the next url. - if (NS_FAILED(exitCode)) + if (NS_FAILED(exitCode) && exitCode != NS_MSG_ERROR_IMAP_COMMAND_FAILED) m_foldersToStat.Clear(); if (m_foldersToStat.Count() > 0) m_foldersToStat[0]->UpdateStatus(this, nsnull); @@ -2994,19 +2994,20 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder, // or if we are forced to check all folders PRUint32 flags = 0; aFolder->GetFlags(&flags); - if ((forceAllFolders && - !(flags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::Trash | nsMsgFolderFlags::Junk | - nsMsgFolderFlags::ImapNoselect | nsMsgFolderFlags::Virtual))) || - (flags & nsMsgFolderFlags::CheckNew)) + nsresult rv; + nsCOMPtr imapFolder = do_QueryInterface(aFolder, &rv); + NS_ENSURE_SUCCESS(rv, rv); + PRBool canOpen; + imapFolder->GetCanOpenFolder(&canOpen); + if (canOpen && ((forceAllFolders && + !(flags & (nsMsgFolderFlags::Inbox | nsMsgFolderFlags::Trash | + nsMsgFolderFlags::Junk | nsMsgFolderFlags::Virtual))) || + flags & nsMsgFolderFlags::CheckNew)) { // Get new messages for this folder. aFolder->SetGettingNewMessages(PR_TRUE); if (performingBiff) - { - nsCOMPtr imapFolder(do_QueryInterface(aFolder)); - if (imapFolder) - imapFolder->SetPerformingBiff(PR_TRUE); - } + imapFolder->SetPerformingBiff(PR_TRUE); PRBool isOpen = PR_FALSE; nsCOMPtr mailSession = do_GetService(NS_MSGMAILSESSION_CONTRACTID); if (mailSession && aFolder) @@ -3022,9 +3023,7 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder, } if (gUseStatus && !isOpen) { - nsCOMPtr imapFolder = do_QueryInterface(aFolder); - if (imapFolder && !isServer && - m_foldersToStat.IndexOf(imapFolder) == -1) + if (!isServer && m_foldersToStat.IndexOf(imapFolder) == -1) m_foldersToStat.AppendObject(imapFolder); } else @@ -3033,7 +3032,7 @@ nsImapIncomingServer::GetNewMessagesForNonInboxFolders(nsIMsgFolder *aFolder, // Loop through all subfolders to get new messages for them. nsCOMPtr enumerator; - nsresult rv = aFolder->GetSubFolders(getter_AddRefs(enumerator)); + rv = aFolder->GetSubFolders(getter_AddRefs(enumerator)); if (NS_FAILED(rv)) return rv; diff --git a/mailnews/imap/test/unit/test_dontStatNoSelect.js b/mailnews/imap/test/unit/test_dontStatNoSelect.js new file mode 100644 index 0000000000..a8c527f0ad --- /dev/null +++ b/mailnews/imap/test/unit/test_dontStatNoSelect.js @@ -0,0 +1,119 @@ +// This file tests that checking folders for new mail with STATUS +// doesn't try to STAT noselect folders. + +var gServer, gImapServer; +var gIMAPInbox, gIMAPFolder1, gIMAPFolder2; +var gFolder2Mailbox; + +load("../../mailnews/resources/messageGenerator.js"); + +const nsIIOService = Cc["@mozilla.org/network/io-service;1"] + .getService(Ci.nsIIOService); + +function run_test() { + var daemon = new imapDaemon(); + daemon.createMailbox("folder 1", {subscribed : true}); + let folder1Mailbox = daemon.getMailbox("folder 1"); + folder1Mailbox.flags.push("\\Noselect"); + daemon.createMailbox("folder 2", {subscribed : true}); + gFolder2Mailbox = daemon.getMailbox("folder 2"); + addMessageToFolder(gFolder2Mailbox); + gServer = makeServer(daemon, ""); + + gImapServer = createLocalIMAPServer(); + gImapServer.maximumConnectionsNumber = 1; + + loadLocalMailAccount(); + + // We need an identity so that updateFolder doesn't fail + let acctMgr = Cc["@mozilla.org/messenger/account-manager;1"] + .getService(Ci.nsIMsgAccountManager); + let localAccount = acctMgr.createAccount(); + let identity = acctMgr.createIdentity(); + localAccount.addIdentity(identity); + localAccount.defaultIdentity = identity; + localAccount.incomingServer = gLocalIncomingServer; + acctMgr.defaultAccount = localAccount; + + // Let's also have another account, using the same identity + let imapAccount = acctMgr.createAccount(); + imapAccount.addIdentity(identity); + imapAccount.defaultIdentity = identity; + imapAccount.incomingServer = gImapServer; + + // Get the folder list... + gImapServer.performExpand(null); + gServer.performTest("SUBSCRIBE"); + // pref tuning: one connection only, turn off notifications + let prefBranch = Cc["@mozilla.org/preferences-service;1"] + .getService(Ci.nsIPrefBranch); + // Make sure no biff notifications happen + prefBranch.setBoolPref("mail.biff.play_sound", false); + prefBranch.setBoolPref("mail.biff.show_alert", false); + prefBranch.setBoolPref("mail.biff.show_tray_icon", false); + prefBranch.setBoolPref("mail.biff.animate_dock_icon", false); + + let rootFolder = gImapServer.rootFolder; + gIMAPInbox = rootFolder.getFolderWithFlags(Ci.nsMsgFolderFlags.Inbox); + gFolder1 = rootFolder.getChildNamed("folder 1"); + gFolder2 = rootFolder.getChildNamed("folder 2"); + gFolder1.setFlag(Ci.nsMsgFolderFlags.CheckNew); + gFolder2.setFlag(Ci.nsMsgFolderFlags.CheckNew); + do_test_pending(); + // imap fake server's resetTest resets the authentication state - charming. + // So poke the _test member directly. + gServer._test = true; + gIMAPInbox.getNewMessages(null, null); + gServer.performTest("STATUS"); + // We want to wait for the STATUS to be really done before we issue + // more STATUS commands, so we do a NOOP on the + // INBOX, and since we only have one connection with the fake server, + // that will essentially serialize things. + gServer._test = true; + gIMAPInbox.updateFolder(null); + gServer.performTest("NOOP"); + do_timeout_function(0, testCheckStatError); +} + +function testCheckStatError() { + // folder 2 should have been stat'd, but not folder 1. All we can really check + // is that folder 2 was stat'd and that its unread msg count is 1 + do_check_eq(gFolder2.getNumUnread(false), 1); + addMessageToFolder(gFolder2Mailbox); + gFolder1.clearFlag(Ci.nsMsgFolderFlags.ImapNoselect); + gServer._test = true; + // we've cleared the ImapNoselect flag, so we will attempt to STAT folder 1, + // which will fail. So we verify that we go on and STAT folder 2, and that + // it picks up the message we added to it above. + gIMAPInbox.getNewMessages(null, null); + gServer.performTest("STATUS"); + gServer._test = true; + gServer.performTest("STATUS"); + do_timeout_function(0, endTest); +} + +function addMessageToFolder(mbox) { + // make a couple messges + let messages = []; + let gMessageGenerator = new MessageGenerator(); + messages = messages.concat(gMessageGenerator.makeMessage()); + + let msgURI = + nsIIOService.newURI("data:text/plain;base64," + + btoa(messages[0].toMessageString()), + null, null); + let message = new imapMessage(msgURI.spec, mbox.uidnext++); + mbox.addMessage(message); +} + +function endTest() +{ + do_check_eq(gFolder2.getNumUnread(false), 2); + // Clean up the server in preparation + gServer.resetTest(); + gImapServer.closeCachedConnections(); + gServer.performTest(); + gServer.stop(); + + do_test_finished(); +} diff --git a/mailnews/test/fakeserver/imapd.js b/mailnews/test/fakeserver/imapd.js index 6eefea7f62..d96bfa1792 100644 --- a/mailnews/test/fakeserver/imapd.js +++ b/mailnews/test/fakeserver/imapd.js @@ -750,7 +750,7 @@ IMAP_RFC3501_handler.prototype = { this._lastCommand = command; // Are we allowed to execute this command? if (this._enabledCommands[this._state].indexOf(command) == -1) - return this._tag + " BAD illegal command for current state"; + return this._tag + " BAD illegal command for current state " + this._state; try { // Format the arguments nicely @@ -1001,8 +1001,11 @@ IMAP_RFC3501_handler.prototype = { var mbox = this._daemon.getMailbox(args[0]); if (!mbox || mbox.name == "") return "NO no such mailbox"; - if (mbox._children.length > 0 && "\\Noselect" in mbox.flags) - return "NO cannot delete mailbox"; + if (mbox._children.length > 0) { + for (let i = 0; i < mbox.flags.length; i++) + if (mbox.flags[i] == "\\Noselect") + return "NO cannot delete mailbox"; + } this._daemon.deleteMailbox(mbox); return "OK DELETE completed"; }, @@ -1055,6 +1058,9 @@ IMAP_RFC3501_handler.prototype = { var box = this._daemon.getMailbox(args[0]); if (!box) return "NO no such mailbox exists"; + for (let i = 0; i < box.flags.length; i++) + if (box.flags[i] == "\\Noselect") + return "NO STATUS not allowed on Noselect folder"; var parts = []; for each (var status in args[1]) { var line = status + " ";