From 49f069a2c2862005b192884d8176e76ec9d1568a Mon Sep 17 00:00:00 2001 From: Felix Hammerl Date: Thu, 6 Mar 2014 17:13:36 +0100 Subject: [PATCH] make verification mail handling more resilitent --- src/js/dao/email-dao.js | 69 +++++++++++++++++++-------------- test/new-unit/email-dao-test.js | 13 ++++++- 2 files changed, 50 insertions(+), 32 deletions(-) diff --git a/src/js/dao/email-dao.js b/src/js/dao/email-dao.js index acb8705..32e8cb2 100644 --- a/src/js/dao/email-dao.js +++ b/src/js/dao/email-dao.js @@ -548,6 +548,7 @@ define(function(require) { return; } + // list the messages starting from the lowest new uid to the highest new uid self._imapListMessages({ folder: folder.path, firstUid: Math.min.apply(null, delta4), @@ -559,27 +560,40 @@ define(function(require) { return; } - // if there is a verification message in the synced messages, handle it - var verificationMessage = _.findWhere(messages, { - subject: str.subjectPrefix + str.verificationSubject + // if there are verification messages in the synced messages, handle it + var verificationMessages = _.filter(messages, function(message) { + return message.subject === (str.subjectPrefix + str.verificationSubject); }); - if (verificationMessage) { - handleVerification(verificationMessage, function(err) { - // eliminate the verification mail if the verification worked, otherwise display an error and it in the mail list - if (err) { - callback(err); - } else { - messages = _.filter(messages, function(message) { - return message.subject !== (str.subjectPrefix + str.verificationSubject); - }); - } + // if there are verification messages, continue after we've tried to verify + if (verificationMessages.length > 0) { + var after = _.after(verificationMessages.length, storeHeaders); - storeHeaders(); + verificationMessages.forEach(function(verificationMessage) { + handleVerification(verificationMessage, function(err, isValid) { + // if it was NOT a valid verification mail, do nothing + if (!isValid) { + after(); + return; + } + + // if an error occurred and the mail was a valid verification mail, display the error, but + // keep the mail in the list so the user can see it and verify manually + if (err) { + callback(err); + after(); + return; + } + + // if verification worked, we remove the mail from the list. + messages.splice(messages.indexOf(verificationMessage), 1); + after(); + }); }); return; } + // no verification messages, just proceed as usual storeHeaders(); function storeHeaders() { @@ -776,34 +790,29 @@ define(function(require) { folder: options.folder, message: message }, function(error) { - var verificationUrlPrefix = config.cloudUrl + config.verificationUrl, - uuid, isValidUuid, index; - + // we could not stream the text to determine if the verification was valid or not + // so handle it as if it were valid if (error) { - localCallback(error); + localCallback(error, true); return; } - index = message.body.indexOf(verificationUrlPrefix); - if (index === -1) { - // there's no url in the message, so forget about that. - localCallback(); - return; - } + var verificationUrlPrefix = config.cloudUrl + config.verificationUrl, + uuid = message.body.split(verificationUrlPrefix).pop().substr(0, config.verificationUuidLength), + isValidUuid = new RegExp('[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}').test(uuid); - uuid = message.body.substr(index + verificationUrlPrefix.length, config.verificationUuidLength); - isValidUuid = new RegExp('[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}').test(uuid); + // there's no valid uuid in the message, so forget about it if (!isValidUuid) { - // there's no valid uuid in the message, so forget about that, too. - localCallback(); + localCallback(null, false); return; } + // there's a valid uuid in the message, so try to verify it self._keychain.verifyPublicKey(uuid, function(err) { if (err) { localCallback({ errMsg: 'Verifying your public key failed: ' + err.errMsg - }); + }, true); return; } @@ -814,7 +823,7 @@ define(function(require) { }, function() { // if we could successfully not delete the message or not doesn't matter. // just don't show it in whiteout and keep quiet about it - localCallback(); + localCallback(null, true); }); }); }); diff --git a/test/new-unit/email-dao-test.js b/test/new-unit/email-dao-test.js index 9a3dc50..226516e 100644 --- a/test/new-unit/email-dao-test.js +++ b/test/new-unit/email-dao-test.js @@ -2035,7 +2035,7 @@ define(function(require) { }); it('should not bother about corrupted authentication mails', function(done) { - var invocations, folder, localListStub, imapSearchStub, imapGetStub, imapListMessagesStub, imapDeleteStub; + var invocations, folder, localListStub, imapSearchStub, imapGetStub, imapListMessagesStub, imapDeleteStub, localStoreStub; invocations = 0; folder = 'FOLDAAAA'; @@ -2059,6 +2059,13 @@ define(function(require) { folder: folder, answered: true }).yields(null, []); + + localStoreStub = sinon.stub(dao, '_localStoreMessages').withArgs({ + folder: folder, + emails: [corruptedVerificationMail] + }).yields(); + + imapListMessagesStub = sinon.stub(dao, '_imapListMessages').yields(null, [corruptedVerificationMail]); imapGetStub = sinon.stub(dao, '_imapStreamText').yields(null); keychainStub.verifyPublicKey.withArgs(corruptedVerificationUuid).yields({ @@ -2078,13 +2085,15 @@ define(function(require) { } expect(dao._account.busy).to.be.false; - expect(dao._account.folders[0].messages).to.be.empty; + expect(dao._account.folders[0].messages).to.not.be.empty; expect(localListStub.calledOnce).to.be.true; expect(imapSearchStub.calledThrice).to.be.true; expect(imapGetStub.calledOnce).to.be.true; + expect(localStoreStub.calledOnce).to.be.true; expect(keychainStub.verifyPublicKey.called).to.be.false; expect(imapDeleteStub.called).to.be.false; + done(); }); });