From 35b37c709728043fd5350d49e60b49569b86f6ae Mon Sep 17 00:00:00 2001
From: James Houlahan <james.houlahan@protonmail.com>
Date: Mon, 17 Aug 2020 10:49:56 +0200
Subject: [PATCH] fix (GODT-597): duplicate send when draft creation takes a
 long time

---
 Changelog.md                        |  5 ++---
 internal/smtp/send_recorder.go      | 32 +++++++++++++++++++++++++----
 internal/smtp/send_recorder_test.go |  3 ++-
 internal/smtp/user.go               |  4 +++-
 4 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index 1a9ad113..15220207 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -4,8 +4,6 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 ## Unreleased
 
-## [v1.3.x] Emma (beta since 2020-08-05)
-
 ### Added
 * GODT-633 Persistent anonymous API cookies for better load balancing and abuse detection. 
 
@@ -40,9 +38,10 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
 ### Fixed
 * GODT-454 Fix send on closed channel when receiving unencrypted send confirmation from GUI.
+* GODT-597 Duplicate sending when draft creation takes too long
 
 
-## [v1.3.x] Emma (beta 2020-07-XXX)
+## [v1.3.x] Emma (v1.3.2 beta 2020-08-04, v1.3.3 beta 2020-08-06, v1.3.3 live 2020-08-12)
 
 ### Added
 * GODT-554 Detect and notify about "bad certificate" IMAP TLS error.
diff --git a/internal/smtp/send_recorder.go b/internal/smtp/send_recorder.go
index f891700d..20abfdcc 100644
--- a/internal/smtp/send_recorder.go
+++ b/internal/smtp/send_recorder.go
@@ -69,14 +69,31 @@ func (q *sendRecorder) getMessageHash(message *pmapi.Message) string {
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func (q *sendRecorder) addMessage(hash, messageID string) {
+func (q *sendRecorder) addMessage(hash string) {
 	q.lock.Lock()
 	defer q.lock.Unlock()
 
 	q.deleteExpiredKeys()
 	q.hashes[hash] = sendRecorderValue{
-		messageID: messageID,
-		time:      time.Now(),
+		time: time.Now(),
+	}
+}
+
+func (q *sendRecorder) removeMessage(hash string) {
+	q.lock.Lock()
+	defer q.lock.Unlock()
+
+	q.deleteExpiredKeys()
+	delete(q.hashes, hash)
+}
+
+func (q *sendRecorder) setMessageID(hash, messageID string) {
+	q.lock.Lock()
+	defer q.lock.Unlock()
+
+	if val, ok := q.hashes[hash]; ok {
+		val.messageID = messageID
+		q.hashes[hash] = val
 	}
 }
 
@@ -89,6 +106,12 @@ func (q *sendRecorder) isSendingOrSent(client messageGetter, hash string) (isSen
 	if !ok {
 		return
 	}
+
+	// If we have a value but don't yet have a messageID, we are in the process of uploading the draft.
+	if value.messageID == "" {
+		return true, false
+	}
+
 	message, err := client.GetMessage(value.messageID)
 	// Message could be deleted or there could be an internet issue or whatever,
 	// so let's assume the message was not sent.
@@ -107,7 +130,8 @@ func (q *sendRecorder) isSendingOrSent(client messageGetter, hash string) (isSen
 	if message.Type == pmapi.MessageTypeSent || message.Type == pmapi.MessageTypeInboxAndSent {
 		wasSent = true
 	}
-	return
+
+	return isSending, wasSent
 }
 
 func (q *sendRecorder) deleteExpiredKeys() {
diff --git a/internal/smtp/send_recorder_test.go b/internal/smtp/send_recorder_test.go
index 84cc12be..1c4f272b 100644
--- a/internal/smtp/send_recorder_test.go
+++ b/internal/smtp/send_recorder_test.go
@@ -365,7 +365,8 @@ func TestSendRecorder_getMessageHash(t *testing.T) {
 
 func TestSendRecorder_isSendingOrSent(t *testing.T) {
 	q := newSendRecorder()
-	q.addMessage("hash", "messageID")
+	q.addMessage("hash")
+	q.setMessageID("hash", "messageID")
 
 	testCases := []struct {
 		hash          string
diff --git a/internal/smtp/user.go b/internal/smtp/user.go
index cb74f0fc..b2a79268 100644
--- a/internal/smtp/user.go
+++ b/internal/smtp/user.go
@@ -230,11 +230,13 @@ func (su *smtpUser) Send(from string, to []string, messageReader io.Reader) (err
 		return nil
 	}
 
+	su.backend.sendRecorder.addMessage(sendRecorderMessageHash)
 	message, atts, err := su.storeUser.CreateDraft(kr, message, attReaders, attachedPublicKey, attachedPublicKeyName, parentID)
 	if err != nil {
+		su.backend.sendRecorder.removeMessage(sendRecorderMessageHash)
 		return
 	}
-	su.backend.sendRecorder.addMessage(sendRecorderMessageHash, message.ID)
+	su.backend.sendRecorder.setMessageID(sendRecorderMessageHash, message.ID)
 
 	// We always have to create a new draft even if there already is one,
 	// because clients don't necessarily save the draft before sending, which