diff --git a/internal/imap/map.go b/internal/imap/map.go new file mode 100644 index 00000000..898a6bde --- /dev/null +++ b/internal/imap/map.go @@ -0,0 +1,44 @@ +// Copyright (c) 2022 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + +package imap + +import "sync" + +type safeMapOfStrings struct { + data map[string]string + mutex sync.RWMutex +} + +func newSafeMapOfString() safeMapOfStrings { + return safeMapOfStrings{ + data: map[string]string{}, + mutex: sync.RWMutex{}, + } +} + +func (m *safeMapOfStrings) get(key string) string { + m.mutex.RLock() + defer m.mutex.RUnlock() + return m.data[key] +} + +func (m *safeMapOfStrings) set(key, value string) { + m.mutex.Lock() + defer m.mutex.Unlock() + m.data[key] = value +} diff --git a/internal/imap/updates.go b/internal/imap/updates.go index cc84eb2c..c011510d 100644 --- a/internal/imap/updates.go +++ b/internal/imap/updates.go @@ -235,7 +235,7 @@ func (iu *imapUpdates) getIDs(address, mailboxName string) (addressID, mailboxID } addressID = user.addressID - if v := user.mailboxIDs[mailboxName]; v != "" { + if v := user.mailboxIDs.get(mailboxName); v != "" { mailboxID = v } diff --git a/internal/imap/user.go b/internal/imap/user.go index 96a450b8..eec20570 100644 --- a/internal/imap/user.go +++ b/internal/imap/user.go @@ -54,8 +54,8 @@ type imapUser struct { // UNSELECT, CLOSE, or LOGOUT. appendExpungeLock sync.Mutex - addressID string // cached value for logs to avoid lock - mailboxIDs map[string]string // cached values for logs to avoid lock + addressID string // cached value for logs to avoid lock + mailboxIDs safeMapOfStrings // cached values for logs to avoid lock } // newIMAPUser returns struct implementing go-imap/user interface. @@ -88,7 +88,7 @@ func newIMAPUser( currentAddressLowercase: strings.ToLower(address), addressID: addressID, - mailboxIDs: map[string]string{}, + mailboxIDs: newSafeMapOfString(), }, err } @@ -133,7 +133,7 @@ func (iu *imapUser) ListMailboxes(showOnlySubcribed bool) ([]goIMAPBackend.Mailb mailboxes := []goIMAPBackend.Mailbox{} for _, storeMailbox := range iu.storeAddress.ListMailboxes() { - iu.mailboxIDs[storeMailbox.Name()] = storeMailbox.LabelID() + iu.mailboxIDs.set(storeMailbox.Name(), storeMailbox.LabelID()) if storeMailbox.LabelID() == pmapi.AllMailLabel && !iu.backend.bridge.IsAllMailVisible() { continue diff --git a/test/fakeapi/controller_control.go b/test/fakeapi/controller_control.go index ddd4f35f..982a26ea 100644 --- a/test/fakeapi/controller_control.go +++ b/test/fakeapi/controller_control.go @@ -80,8 +80,10 @@ func (ctl *Controller) AddUserLabel(username string, label *pmapi.Label) error { ctl.labelsByUsername[username] = []*pmapi.Label{} } + userLabels := ctl.labelsByUsername[username] + labelName := getLabelNameWithoutPrefix(label.Name) - for _, existingLabel := range ctl.labelsByUsername[username] { + for _, existingLabel := range userLabels { if existingLabel.Name == labelName { return fmt.Errorf("folder or label %s already exists", label.Name) } @@ -97,7 +99,9 @@ func (ctl *Controller) AddUserLabel(username string, label *pmapi.Label) error { if label.Path == "" { label.Path = label.Name } - ctl.labelsByUsername[username] = append(ctl.labelsByUsername[username], label) + userLabels = append(userLabels, label) + + ctl.labelsByUsername[username] = userLabels ctl.resetUsers() return nil } diff --git a/test/fakeapi/labels.go b/test/fakeapi/labels.go index c06ae001..215035d9 100644 --- a/test/fakeapi/labels.go +++ b/test/fakeapi/labels.go @@ -88,7 +88,7 @@ func (api *FakePMAPI) listLabels(_ context.Context, labeType string, route strin if err := api.checkAndRecordCall(GET, route+"/"+labeType, nil); err != nil { return nil, err } - return api.labels, nil + return append([]*pmapi.Label{}, api.labels...), nil } func (api *FakePMAPI) createLabel(_ context.Context, label *pmapi.Label, route string) (*pmapi.Label, error) { diff --git a/test/features/imap/mailbox/list.feature b/test/features/imap/mailbox/list.feature index 99a4591e..845e4ea3 100644 --- a/test/features/imap/mailbox/list.feature +++ b/test/features/imap/mailbox/list.feature @@ -38,6 +38,97 @@ Feature: IMAP list mailboxes Then IMAP response contains "Trash" Then IMAP response contains "All Mail" + Scenario: List multiple times in parallel without crash + Given there is "user" with mailboxes + | Folders/mbox1 | + | Folders/mbox2 | + | Folders/mbox3 | + | Folders/mbox4 | + | Folders/mbox5 | + | Folders/mbox6 | + | Folders/mbox7 | + | Folders/mbox8 | + | Folders/mbox9 | + | Folders/mbox10 | + | Folders/mbox11 | + | Folders/mbox12 | + | Folders/mbox13 | + | Folders/mbox14 | + | Folders/mbox15 | + | Folders/mbox16 | + | Folders/mbox17 | + | Folders/mbox18 | + | Folders/mbox19 | + | Folders/mbox20 | + | Labels/lab1 | + | Labels/lab2 | + | Labels/lab3 | + | Labels/lab4 | + | Labels/lab5 | + | Labels/lab6 | + | Labels/lab7 | + | Labels/lab8 | + | Labels/lab9 | + | Labels/lab10 | + | Labels/lab11 | + | Labels/lab12 | + | Labels/lab13 | + | Labels/lab14 | + | Labels/lab15 | + | Labels/lab16 | + | Labels/lab17 | + | Labels/lab18 | + | Labels/lab19 | + | Labels/lab20 | + | Labels/lab1.1 | + | Labels/lab1.2 | + | Labels/lab1.3 | + | Labels/lab1.4 | + | Labels/lab1.5 | + | Labels/lab1.6 | + | Labels/lab1.7 | + | Labels/lab1.8 | + | Labels/lab1.9 | + | Labels/lab1.10 | + | Labels/lab1.11 | + | Labels/lab1.12 | + | Labels/lab1.13 | + | Labels/lab1.14 | + | Labels/lab1.15 | + | Labels/lab1.16 | + | Labels/lab1.17 | + | Labels/lab1.18 | + | Labels/lab1.19 | + | Labels/lab1.20 | + | Labels/lab2.1 | + | Labels/lab2.2 | + | Labels/lab2.3 | + | Labels/lab2.4 | + | Labels/lab2.5 | + | Labels/lab2.6 | + | Labels/lab2.7 | + | Labels/lab2.8 | + | Labels/lab2.9 | + | Labels/lab2.10 | + | Labels/lab2.11 | + | Labels/lab2.12 | + | Labels/lab2.13 | + | Labels/lab2.14 | + | Labels/lab2.15 | + | Labels/lab2.16 | + | Labels/lab2.17 | + | Labels/lab2.18 | + | Labels/lab2.19 | + | Labels/lab2.20 | + And there is IMAP client "A" logged in as "user" + And there is IMAP client "B" logged in as "user" + When IMAP client "A" lists mailboxes + And IMAP client "B" lists mailboxes + Then IMAP response to "A" is "OK" + And IMAP response to "A" contains "mbox1" + And IMAP response to "A" contains "mbox10" + And IMAP response to "A" contains "mbox20" + @ignore-live Scenario: List mailboxes with subfolders # Escaped slash in the name contains slash in the name. diff --git a/test/imap_actions_mailbox_test.go b/test/imap_actions_mailbox_test.go index e85fcce2..775db60b 100644 --- a/test/imap_actions_mailbox_test.go +++ b/test/imap_actions_mailbox_test.go @@ -28,6 +28,7 @@ func IMAPActionsMailboxFeatureContext(s *godog.ScenarioContext) { s.Step(`^IMAP client renames mailbox "([^"]*)" to "([^"]*)"$`, imapClientRenamesMailboxTo) s.Step(`^IMAP client deletes mailbox "([^"]*)"$`, imapClientDeletesMailbox) s.Step(`^IMAP client lists mailboxes$`, imapClientListsMailboxes) + s.Step(`^IMAP client "([^"]*)" lists mailboxes$`, imapClientNamedListsMailboxes) s.Step(`^IMAP client selects "([^"]*)"$`, imapClientSelects) s.Step(`^IMAP client gets info of "([^"]*)"$`, imapClientGetsInfoOf) s.Step(`^IMAP client "([^"]*)" gets info of "([^"]*)"$`, imapClientNamedGetsInfoOf) @@ -63,8 +64,12 @@ func imapClientDeletesMailbox(mailboxName string) error { } func imapClientListsMailboxes() error { - res := ctx.GetIMAPClient("imap").ListMailboxes() - ctx.SetIMAPLastResponse("imap", res) + return imapClientNamedListsMailboxes("imap") +} + +func imapClientNamedListsMailboxes(clientName string) error { + res := ctx.GetIMAPClient(clientName).ListMailboxes() + ctx.SetIMAPLastResponse(clientName, res) return nil }