From 73dcd7256ec03e84386152ff0b164e4ff780e568 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Tue, 9 Sep 2025 15:05:26 +0200 Subject: [PATCH] refactor(groupware): add max requests check * move jmap.request() to jmap.Client.request() and pass the Session and a Logger to introduce checking the number of methodCalls within a request not exceeding the limit of the Session, as well as error handling and logging there instead of in each caller * a few bugfixes: - add a few missing Send() calls in logs - correct the response tag matching for GetMailboxChangesForMultipleAccounts - fix typo in Identity.ReplyTo json serialization rune - fix response tag in pkg/jmap/testdata/mailboxes1.json after changing them to be prefixed by the accountId --- pkg/jmap/jmap_api_blob.go | 29 +++-- pkg/jmap/jmap_api_email.go | 169 +++++++++++++----------------- pkg/jmap/jmap_api_identity.go | 35 +++---- pkg/jmap/jmap_api_mailbox.go | 72 ++++++------- pkg/jmap/jmap_api_vacation.go | 31 +++--- pkg/jmap/jmap_client.go | 26 +++++ pkg/jmap/jmap_error.go | 6 ++ pkg/jmap/jmap_model.go | 10 +- pkg/jmap/jmap_test.go | 17 ++- pkg/jmap/jmap_tools.go | 12 ++- pkg/jmap/testdata/mailboxes1.json | 2 +- 11 files changed, 202 insertions(+), 207 deletions(-) diff --git a/pkg/jmap/jmap_api_blob.go b/pkg/jmap/jmap_api_blob.go index b53edc1de2..cf91f8861f 100644 --- a/pkg/jmap/jmap_api_blob.go +++ b/pkg/jmap/jmap_api_blob.go @@ -15,24 +15,22 @@ type BlobResponse struct { } func (j *Client) GetBlob(accountId string, session *Session, ctx context.Context, logger *log.Logger, id string) (BlobResponse, SessionState, Error) { - cmd, err := request( + cmd, jerr := j.request(session, logger, invocation(CommandBlobUpload, BlobGetCommand{ AccountId: accountId, Ids: []string{id}, Properties: []string{BlobPropertyData, BlobPropertyDigestSha512, BlobPropertySize}, }, "0"), ) - if err != nil { - logger.Error().Err(err) - return BlobResponse{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + if jerr != nil { + return BlobResponse{}, "", jerr } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (BlobResponse, Error) { var response BlobGetResponse - err = retrieveResponseMatchParameters(body, CommandBlobGet, "0", &response) + err := retrieveResponseMatchParameters(logger, body, CommandBlobGet, "0", &response) if err != nil { - logger.Error().Err(err) - return BlobResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return BlobResponse{}, err } if len(response.List) != 1 { @@ -96,28 +94,25 @@ func (j *Client) UploadBlob(accountId string, session *Session, ctx context.Cont Properties: []string{BlobPropertyDigestSha512}, } - cmd, err := request( + cmd, jerr := j.request(session, logger, invocation(CommandBlobUpload, upload, "0"), invocation(CommandBlobGet, getHash, "1"), ) - if err != nil { - logger.Error().Err(err) - return UploadedBlob{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + if jerr != nil { + return UploadedBlob{}, "", jerr } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (UploadedBlob, Error) { var uploadResponse BlobUploadResponse - err = retrieveResponseMatchParameters(body, CommandBlobUpload, "0", &uploadResponse) + err := retrieveResponseMatchParameters(logger, body, CommandBlobUpload, "0", &uploadResponse) if err != nil { - logger.Error().Err(err) - return UploadedBlob{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return UploadedBlob{}, err } var getResponse BlobGetResponse - err = retrieveResponseMatchParameters(body, CommandBlobGet, "1", &getResponse) + err = retrieveResponseMatchParameters(logger, body, CommandBlobGet, "1", &getResponse) if err != nil { - logger.Error().Err(err) - return UploadedBlob{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return UploadedBlob{}, err } if len(uploadResponse.Created) != 1 { diff --git a/pkg/jmap/jmap_api_email.go b/pkg/jmap/jmap_api_email.go index 02a799b5e2..1ec92265c6 100644 --- a/pkg/jmap/jmap_api_email.go +++ b/pkg/jmap/jmap_api_email.go @@ -39,17 +39,16 @@ func (j *Client) GetEmails(accountId string, session *Session, ctx context.Conte get.MaxBodyValueBytes = maxBodyValueBytes } - cmd, err := request(invocation(CommandEmailGet, get, "0")) + cmd, err := j.request(session, logger, invocation(CommandEmailGet, get, "0")) if err != nil { - logger.Error().Err(err) + logger.Error().Err(err).Send() return Emails{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (Emails, Error) { var response EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "0", &response) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "0", &response) if err != nil { - logger.Error().Err(err) - return Emails{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return Emails{}, err } return Emails{Emails: response.List, State: response.State}, nil }) @@ -84,27 +83,25 @@ func (j *Client) GetAllEmailsInMailbox(accountId string, session *Session, ctx c get.MaxBodyValueBytes = maxBodyValueBytes } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailQuery, query, "0"), invocation(CommandEmailGet, get, "1"), ) if err != nil { - logger.Error().Err(err) - return Emails{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return Emails{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (Emails, Error) { var queryResponse EmailQueryResponse - err = retrieveResponseMatchParameters(body, CommandEmailQuery, "0", &queryResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailQuery, "0", &queryResponse) if err != nil { - logger.Error().Err(err) - return Emails{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return Emails{}, err } var getResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "1", &getResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &getResponse) if err != nil { - logger.Error().Err(err) - return Emails{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return Emails{}, err } return Emails{ @@ -148,7 +145,7 @@ func (j *Client) GetEmailsSince(accountId string, session *Session, ctx context. getUpdated.MaxBodyValueBytes = maxBodyValueBytes } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailChanges, changes, "0"), invocation(CommandEmailGet, getCreated, "1"), invocation(CommandEmailGet, getUpdated, "2"), @@ -159,24 +156,23 @@ func (j *Client) GetEmailsSince(accountId string, session *Session, ctx context. return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (MailboxChanges, Error) { var changesResponse EmailChangesResponse - err = retrieveResponseMatchParameters(body, CommandEmailChanges, "0", &changesResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailChanges, "0", &changesResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return MailboxChanges{}, err } var createdResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "1", &createdResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &createdResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return MailboxChanges{}, err } var updatedResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "2", &updatedResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "2", &updatedResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return MailboxChanges{}, err } return MailboxChanges{ @@ -227,29 +223,25 @@ func (j *Client) QueryEmailSnippets(accountId string, filter EmailFilterElement, }, } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailQuery, query, "0"), invocation(CommandSearchSnippetGet, snippet, "1"), ) - if err != nil { - logger.Error().Err(err) - return EmailSnippetQueryResult{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return EmailSnippetQueryResult{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (EmailSnippetQueryResult, Error) { var queryResponse EmailQueryResponse - err = retrieveResponseMatchParameters(body, CommandEmailQuery, "0", &queryResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailQuery, "0", &queryResponse) if err != nil { - logger.Error().Err(err) - return EmailSnippetQueryResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailSnippetQueryResult{}, err } var snippetResponse SearchSnippetGetResponse - err = retrieveResponseMatchParameters(body, CommandSearchSnippetGet, "1", &snippetResponse) + err = retrieveResponseMatchParameters(logger, body, CommandSearchSnippetGet, "1", &snippetResponse) if err != nil { - logger.Error().Err(err) - return EmailSnippetQueryResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailSnippetQueryResult{}, err } return EmailSnippetQueryResult{ @@ -301,27 +293,25 @@ func (j *Client) QueryEmails(accountId string, filter EmailFilterElement, sessio MaxBodyValueBytes: maxBodyValueBytes, } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailQuery, query, "0"), invocation(CommandEmailGet, mails, "1"), ) - if err != nil { - logger.Error().Err(err) - return EmailQueryResult{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return EmailQueryResult{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (EmailQueryResult, Error) { var queryResponse EmailQueryResponse - err = retrieveResponseMatchParameters(body, CommandEmailQuery, "0", &queryResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailQuery, "0", &queryResponse) if err != nil { - return EmailQueryResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailQueryResult{}, err } var emailsResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "1", &emailsResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &emailsResponse) if err != nil { - return EmailQueryResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailQueryResult{}, err } return EmailQueryResult{ @@ -388,34 +378,34 @@ func (j *Client) QueryEmailsWithSnippets(accountId string, filter EmailFilterEle MaxBodyValueBytes: maxBodyValueBytes, } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailQuery, query, "0"), invocation(CommandSearchSnippetGet, snippet, "1"), invocation(CommandEmailGet, mails, "2"), ) if err != nil { - logger.Error().Err(err) + logger.Error().Err(err).Send() return EmailQueryWithSnippetsResult{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (EmailQueryWithSnippetsResult, Error) { var queryResponse EmailQueryResponse - err = retrieveResponseMatchParameters(body, CommandEmailQuery, "0", &queryResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailQuery, "0", &queryResponse) if err != nil { - return EmailQueryWithSnippetsResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailQueryWithSnippetsResult{}, err } var snippetResponse SearchSnippetGetResponse - err = retrieveResponseMatchParameters(body, CommandSearchSnippetGet, "1", &snippetResponse) + err = retrieveResponseMatchParameters(logger, body, CommandSearchSnippetGet, "1", &snippetResponse) if err != nil { - return EmailQueryWithSnippetsResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailQueryWithSnippetsResult{}, err } var emailsResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "2", &emailsResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "2", &emailsResponse) if err != nil { - return EmailQueryWithSnippetsResult{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return EmailQueryWithSnippetsResult{}, err } snippetsById := map[string][]SearchSnippet{} @@ -482,7 +472,7 @@ func (j *Client) ImportEmail(accountId string, session *Session, ctx context.Con Properties: []string{BlobPropertyDigestSha512}, } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandBlobUpload, upload, "0"), invocation(CommandBlobGet, getHash, "1"), ) @@ -492,17 +482,16 @@ func (j *Client) ImportEmail(accountId string, session *Session, ctx context.Con return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (UploadedEmail, Error) { var uploadResponse BlobUploadResponse - err = retrieveResponseMatchParameters(body, CommandBlobUpload, "0", &uploadResponse) + err = retrieveResponseMatchParameters(logger, body, CommandBlobUpload, "0", &uploadResponse) if err != nil { - logger.Error().Err(err) - return UploadedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return UploadedEmail{}, err } var getResponse BlobGetResponse - err = retrieveResponseMatchParameters(body, CommandBlobGet, "1", &getResponse) + err = retrieveResponseMatchParameters(logger, body, CommandBlobGet, "1", &getResponse) if err != nil { - logger.Error().Err(err) - return UploadedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return UploadedEmail{}, err } if len(uploadResponse.Created) != 1 { @@ -537,7 +526,7 @@ type CreatedEmail struct { } func (j *Client) CreateEmail(accountId string, email EmailCreate, session *Session, ctx context.Context, logger *log.Logger) (CreatedEmail, SessionState, Error) { - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailSubmissionSet, EmailSetCommand{ AccountId: accountId, Create: map[string]EmailCreate{ @@ -546,16 +535,14 @@ func (j *Client) CreateEmail(accountId string, email EmailCreate, session *Sessi }, "0"), ) if err != nil { - logger.Error().Err(err) - return CreatedEmail{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return CreatedEmail{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (CreatedEmail, Error) { var setResponse EmailSetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSet, "0", &setResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSet, "0", &setResponse) if err != nil { - logger.Error().Err(err) - return CreatedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return CreatedEmail{}, err } if len(setResponse.NotCreated) > 0 { @@ -571,9 +558,9 @@ func (j *Client) CreateEmail(accountId string, email EmailCreate, session *Sessi created, ok := setResponse.Created["c"] if !ok { - err = fmt.Errorf("failed to find %s in %s response", string(EmailType), string(CommandEmailSet)) - logger.Error().Err(err) - return CreatedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + berr := fmt.Errorf("failed to find %s in %s response", string(EmailType), string(CommandEmailSet)) + logger.Error().Err(berr) + return CreatedEmail{}, simpleError(berr, JmapErrorInvalidJmapResponsePayload) } return CreatedEmail{ @@ -597,23 +584,21 @@ type UpdatedEmails struct { // // To delete mails, use the DeleteEmails function instead. func (j *Client) UpdateEmails(accountId string, updates map[string]EmailUpdate, session *Session, ctx context.Context, logger *log.Logger) (UpdatedEmails, SessionState, Error) { - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailSet, EmailSetCommand{ AccountId: accountId, Update: updates, }, "0"), ) if err != nil { - logger.Error().Err(err) - return UpdatedEmails{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return UpdatedEmails{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (UpdatedEmails, Error) { var setResponse EmailSetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSet, "0", &setResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSet, "0", &setResponse) if err != nil { - logger.Error().Err(err) - return UpdatedEmails{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return UpdatedEmails{}, err } if len(setResponse.NotUpdated) != len(updates) { // error occured @@ -631,23 +616,21 @@ type DeletedEmails struct { } func (j *Client) DeleteEmails(accountId string, destroy []string, session *Session, ctx context.Context, logger *log.Logger) (DeletedEmails, SessionState, Error) { - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailSet, EmailSetCommand{ AccountId: accountId, Destroy: destroy, }, "0"), ) if err != nil { - logger.Error().Err(err) - return DeletedEmails{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return DeletedEmails{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (DeletedEmails, Error) { var setResponse EmailSetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSet, "0", &setResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSet, "0", &setResponse) if err != nil { - logger.Error().Err(err) - return DeletedEmails{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return DeletedEmails{}, err } if len(setResponse.NotDestroyed) != len(destroy) { // error occured @@ -707,21 +690,19 @@ func (j *Client) SubmitEmail(accountId string, identityId string, emailId string }, } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandEmailSubmissionSet, set, "0"), invocation(CommandEmailSubmissionGet, get, "1"), ) if err != nil { - logger.Error().Err(err) - return SubmittedEmail{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return SubmittedEmail{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (SubmittedEmail, Error) { var submissionResponse EmailSubmissionSetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSubmissionSet, "0", &submissionResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSubmissionSet, "0", &submissionResponse) if err != nil { - logger.Error().Err(err) - return SubmittedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return SubmittedEmail{}, err } if len(submissionResponse.NotCreated) > 0 { @@ -735,17 +716,15 @@ func (j *Client) SubmitEmail(accountId string, identityId string, emailId string // The response to this MUST be returned after the EmailSubmission/set response." // from an example in the spec, it has the same tag as the EmailSubmission/set command ("0" in this case) var setResponse EmailSetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSet, "0", &setResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSet, "0", &setResponse) if err != nil { - logger.Error().Err(err) - return SubmittedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return SubmittedEmail{}, err } var getResponse EmailSubmissionGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailSubmissionGet, "1", &getResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailSubmissionGet, "1", &getResponse) if err != nil { - logger.Error().Err(err) - return SubmittedEmail{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return SubmittedEmail{}, err } if len(getResponse.List) != 1 { @@ -773,7 +752,7 @@ func (j *Client) EmailsInThread(accountId string, threadId string, session *Sess return z.Bool(logFetchBodies, fetchBodies).Str("threadId", log.SafeString(threadId)) }) - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandThreadGet, ThreadGetCommand{ AccountId: accountId, Ids: []string{threadId}, @@ -789,17 +768,15 @@ func (j *Client) EmailsInThread(accountId string, threadId string, session *Sess MaxBodyValueBytes: maxBodyValueBytes, }, "1"), ) - if err != nil { - logger.Error().Err(err) - return []Email{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return []Email{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) ([]Email, Error) { var emailsResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "1", &emailsResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &emailsResponse) if err != nil { - return []Email{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return []Email{}, err } return emailsResponse.List, nil }) diff --git a/pkg/jmap/jmap_api_identity.go b/pkg/jmap/jmap_api_identity.go index 3b3c5357d8..1c613efa98 100644 --- a/pkg/jmap/jmap_api_identity.go +++ b/pkg/jmap/jmap_api_identity.go @@ -16,17 +16,15 @@ type Identities struct { // https://jmap.io/spec-mail.html#identityget func (j *Client) GetIdentity(accountId string, session *Session, ctx context.Context, logger *log.Logger) (Identities, SessionState, Error) { logger = j.logger("GetIdentity", session, logger) - cmd, err := request(invocation(CommandIdentityGet, IdentityGetCommand{AccountId: accountId}, "0")) + cmd, err := j.request(session, logger, invocation(CommandIdentityGet, IdentityGetCommand{AccountId: accountId}, "0")) if err != nil { - logger.Error().Err(err) - return Identities{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return Identities{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (Identities, Error) { var response IdentityGetResponse - err = retrieveResponseMatchParameters(body, CommandIdentityGet, "0", &response) + err = retrieveResponseMatchParameters(logger, body, CommandIdentityGet, "0", &response) if err != nil { - logger.Error().Err(err) - return Identities{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return Identities{}, err } return Identities{ Identities: response.List, @@ -51,10 +49,9 @@ func (j *Client) GetIdentities(accountIds []string, session *Session, ctx contex calls[i] = invocation(CommandIdentityGet, IdentityGetCommand{AccountId: accountId}, strconv.Itoa(i)) } - cmd, err := request(calls...) + cmd, err := j.request(session, logger, calls...) if err != nil { - logger.Error().Err(err) - return IdentitiesGetResponse{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return IdentitiesGetResponse{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (IdentitiesGetResponse, Error) { identities := make(map[string][]Identity, len(uniqueAccountIds)) @@ -62,10 +59,9 @@ func (j *Client) GetIdentities(accountIds []string, session *Session, ctx contex notFound := []string{} for i, accountId := range uniqueAccountIds { var response IdentityGetResponse - err = retrieveResponseMatchParameters(body, CommandIdentityGet, strconv.Itoa(i), &response) + err = retrieveResponseMatchParameters(logger, body, CommandIdentityGet, strconv.Itoa(i), &response) if err != nil { - logger.Error().Err(err) - return IdentitiesGetResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return IdentitiesGetResponse{}, err } else { identities[accountId] = response.List } @@ -99,10 +95,9 @@ func (j *Client) GetIdentitiesAndMailboxes(mailboxAccountId string, accountIds [ calls[i+1] = invocation(CommandIdentityGet, IdentityGetCommand{AccountId: accountId}, strconv.Itoa(i+1)) } - cmd, err := request(calls...) + cmd, err := j.request(session, logger, calls...) if err != nil { - logger.Error().Err(err) - return IdentitiesAndMailboxesGetResponse{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return IdentitiesAndMailboxesGetResponse{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (IdentitiesAndMailboxesGetResponse, Error) { identities := make(map[string][]Identity, len(uniqueAccountIds)) @@ -110,10 +105,9 @@ func (j *Client) GetIdentitiesAndMailboxes(mailboxAccountId string, accountIds [ notFound := []string{} for i, accountId := range uniqueAccountIds { var response IdentityGetResponse - err = retrieveResponseMatchParameters(body, CommandIdentityGet, strconv.Itoa(i+1), &response) + err = retrieveResponseMatchParameters(logger, body, CommandIdentityGet, strconv.Itoa(i+1), &response) if err != nil { - logger.Error().Err(err) - return IdentitiesAndMailboxesGetResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return IdentitiesAndMailboxesGetResponse{}, err } else { identities[accountId] = response.List } @@ -122,10 +116,9 @@ func (j *Client) GetIdentitiesAndMailboxes(mailboxAccountId string, accountIds [ } var mailboxResponse MailboxGetResponse - err = retrieveResponseMatchParameters(body, CommandMailboxGet, "0", &mailboxResponse) + err = retrieveResponseMatchParameters(logger, body, CommandMailboxGet, "0", &mailboxResponse) if err != nil { - logger.Error().Err(err) - return IdentitiesAndMailboxesGetResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return IdentitiesAndMailboxesGetResponse{}, err } return IdentitiesAndMailboxesGetResponse{ diff --git a/pkg/jmap/jmap_api_mailbox.go b/pkg/jmap/jmap_api_mailbox.go index 30b1fec17e..529a6d1ee9 100644 --- a/pkg/jmap/jmap_api_mailbox.go +++ b/pkg/jmap/jmap_api_mailbox.go @@ -19,29 +19,31 @@ func (j *Client) GetMailbox(accountIds []string, session *Session, ctx context.C logger = j.logger("GetMailbox", session, logger) uniqueAccountIds := structs.Uniq(accountIds) - if len(uniqueAccountIds) < 1 { + n := len(uniqueAccountIds) + if n < 1 { return map[string]MailboxesResponse{}, "", nil } - invocations := make([]Invocation, len(uniqueAccountIds)) + invocations := make([]Invocation, n) for i, accountId := range uniqueAccountIds { - invocations[i] = invocation(CommandMailboxGet, MailboxGetCommand{AccountId: accountId, Ids: ids}, accountId) + baseId := accountId + ":" + invocations[i] = invocation(CommandMailboxGet, MailboxGetCommand{AccountId: accountId, Ids: ids}, baseId+"0") } - cmd, err := request(invocations...) + cmd, err := j.request(session, logger, invocations...) if err != nil { - logger.Error().Err(err) - return map[string]MailboxesResponse{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return map[string]MailboxesResponse{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (map[string]MailboxesResponse, Error) { resp := map[string]MailboxesResponse{} for _, accountId := range uniqueAccountIds { + baseId := accountId + ":" + var response MailboxGetResponse - err = retrieveResponseMatchParameters(body, CommandMailboxGet, "0", &response) + err = retrieveResponseMatchParameters(logger, body, CommandMailboxGet, baseId+"0", &response) if err != nil { - logger.Error().Err(err) - return map[string]MailboxesResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return map[string]MailboxesResponse{}, err } resp[accountId] = MailboxesResponse{ @@ -97,10 +99,9 @@ func (j *Client) SearchMailboxes(accountIds []string, session *Session, ctx cont IdRef: &ResultReference{Name: CommandMailboxQuery, Path: "/ids/*", ResultOf: baseId + "0"}, }, baseId+"1") } - cmd, err := request(invocations...) + cmd, err := j.request(session, logger, invocations...) if err != nil { - logger.Error().Err(err) - return map[string]Mailboxes{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return map[string]Mailboxes{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (map[string]Mailboxes, Error) { @@ -109,10 +110,9 @@ func (j *Client) SearchMailboxes(accountIds []string, session *Session, ctx cont baseId := accountId + ":" var response MailboxGetResponse - err = retrieveResponseMatchParameters(body, CommandMailboxGet, baseId+"1", &response) + err = retrieveResponseMatchParameters(logger, body, CommandMailboxGet, baseId+"1", &response) if err != nil { - logger.Error().Err(err) - return map[string]Mailboxes{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return map[string]Mailboxes{}, err } resp[accountId] = Mailboxes{Mailboxes: response.List, State: response.State} @@ -161,36 +161,34 @@ func (j *Client) GetMailboxChanges(accountId string, session *Session, ctx conte getUpdated.MaxBodyValueBytes = maxBodyValueBytes } - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandMailboxChanges, changes, "0"), invocation(CommandEmailGet, getCreated, "1"), invocation(CommandEmailGet, getUpdated, "2"), ) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return MailboxChanges{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (MailboxChanges, Error) { var mailboxResponse MailboxChangesResponse - err = retrieveResponseMatchParameters(body, CommandMailboxChanges, "0", &mailboxResponse) + err = retrieveResponseMatchParameters(logger, body, CommandMailboxChanges, "0", &mailboxResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return MailboxChanges{}, err } var createdResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "1", &createdResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "1", &createdResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return MailboxChanges{}, err } var updatedResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, "2", &updatedResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, "2", &updatedResponse) if err != nil { - logger.Error().Err(err) - return MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + logger.Error().Err(err).Send() + return MailboxChanges{}, err } return MailboxChanges{ @@ -259,10 +257,9 @@ func (j *Client) GetMailboxChangesForMultipleAccounts(accountIds []string, sessi invocations[i*3+2] = invocation(CommandEmailGet, getUpdated, baseId+"2") } - cmd, err := request(invocations...) + cmd, err := j.request(session, logger, invocations...) if err != nil { - logger.Error().Err(err) - return map[string]MailboxChanges{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return map[string]MailboxChanges{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (map[string]MailboxChanges, Error) { @@ -271,24 +268,21 @@ func (j *Client) GetMailboxChangesForMultipleAccounts(accountIds []string, sessi baseId := accountId + ":" var mailboxResponse MailboxChangesResponse - err = retrieveResponseMatchParameters(body, CommandMailboxChanges, baseId+"0", &mailboxResponse) + err = retrieveResponseMatchParameters(logger, body, CommandMailboxChanges, baseId+"0", &mailboxResponse) if err != nil { - logger.Error().Err(err) - return map[string]MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return map[string]MailboxChanges{}, err } var createdResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, baseId+"1", &createdResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, baseId+"1", &createdResponse) if err != nil { - logger.Error().Err(err) - return map[string]MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return map[string]MailboxChanges{}, err } var updatedResponse EmailGetResponse - err = retrieveResponseMatchParameters(body, CommandEmailGet, baseId+"2", &updatedResponse) + err = retrieveResponseMatchParameters(logger, body, CommandEmailGet, baseId+"2", &updatedResponse) if err != nil { - logger.Error().Err(err) - return map[string]MailboxChanges{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return map[string]MailboxChanges{}, err } resp[accountId] = MailboxChanges{ diff --git a/pkg/jmap/jmap_api_vacation.go b/pkg/jmap/jmap_api_vacation.go index f9639b296e..ca8824d7a7 100644 --- a/pkg/jmap/jmap_api_vacation.go +++ b/pkg/jmap/jmap_api_vacation.go @@ -15,17 +15,15 @@ const ( // https://jmap.io/spec-mail.html#vacationresponseget func (j *Client) GetVacationResponse(accountId string, session *Session, ctx context.Context, logger *log.Logger) (VacationResponseGetResponse, SessionState, Error) { logger = j.logger("GetVacationResponse", session, logger) - cmd, err := request(invocation(CommandVacationResponseGet, VacationResponseGetCommand{AccountId: accountId}, "0")) + cmd, err := j.request(session, logger, invocation(CommandVacationResponseGet, VacationResponseGetCommand{AccountId: accountId}, "0")) if err != nil { - logger.Error().Err(err) - return VacationResponseGetResponse{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return VacationResponseGetResponse{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (VacationResponseGetResponse, Error) { var response VacationResponseGetResponse - err = retrieveResponseMatchParameters(body, CommandVacationResponseGet, "0", &response) + err = retrieveResponseMatchParameters(logger, body, CommandVacationResponseGet, "0", &response) if err != nil { - logger.Error().Err(err) - return VacationResponseGetResponse{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return VacationResponseGetResponse{}, err } return response, nil }) @@ -63,7 +61,7 @@ type VacationResponseChange struct { func (j *Client) SetVacationResponse(accountId string, vacation VacationResponsePayload, session *Session, ctx context.Context, logger *log.Logger) (VacationResponseChange, SessionState, Error) { logger = j.logger("SetVacationResponse", session, logger) - cmd, err := request( + cmd, err := j.request(session, logger, invocation(CommandVacationResponseSet, VacationResponseSetCommand{ AccountId: accountId, Create: map[string]VacationResponse{ @@ -82,15 +80,13 @@ func (j *Client) SetVacationResponse(accountId string, vacation VacationResponse invocation(CommandVacationResponseGet, VacationResponseGetCommand{AccountId: accountId}, "1"), ) if err != nil { - logger.Error().Err(err) - return VacationResponseChange{}, "", simpleError(err, JmapErrorInvalidJmapRequestPayload) + return VacationResponseChange{}, "", err } return command(j.api, logger, ctx, session, j.onSessionOutdated, cmd, func(body *Response) (VacationResponseChange, Error) { var setResponse VacationResponseSetResponse - err = retrieveResponseMatchParameters(body, CommandVacationResponseSet, "0", &setResponse) + err = retrieveResponseMatchParameters(logger, body, CommandVacationResponseSet, "0", &setResponse) if err != nil { - logger.Error().Err(err) - return VacationResponseChange{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return VacationResponseChange{}, err } setErr, notok := setResponse.NotCreated[vacationResponseId] @@ -101,16 +97,15 @@ func (j *Client) SetVacationResponse(accountId string, vacation VacationResponse } var getResponse VacationResponseGetResponse - err = retrieveResponseMatchParameters(body, CommandVacationResponseGet, "1", &getResponse) + err = retrieveResponseMatchParameters(logger, body, CommandVacationResponseGet, "1", &getResponse) if err != nil { - logger.Error().Err(err) - return VacationResponseChange{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + return VacationResponseChange{}, err } if len(getResponse.List) != 1 { - err = fmt.Errorf("failed to find %s in %s response", string(VacationResponseType), string(CommandVacationResponseGet)) - logger.Error().Err(err) - return VacationResponseChange{}, simpleError(err, JmapErrorInvalidJmapResponsePayload) + berr := fmt.Errorf("failed to find %s in %s response", string(VacationResponseType), string(CommandVacationResponseGet)) + logger.Error().Msg(berr.Error()) + return VacationResponseChange{}, simpleError(berr, JmapErrorInvalidJmapResponsePayload) } return VacationResponseChange{ diff --git a/pkg/jmap/jmap_client.go b/pkg/jmap/jmap_client.go index 1a1aa489a2..e8337e2d18 100644 --- a/pkg/jmap/jmap_client.go +++ b/pkg/jmap/jmap_client.go @@ -62,3 +62,29 @@ func (j *Client) loggerParams(operation string, _ *Session, logger *log.Logger, } return log.From(l) } + +func (j *Client) maxCallsCheck(calls int, session *Session, logger *log.Logger) Error { + if calls > session.Capabilities.Core.MaxCallsInRequest { + logger.Warn(). + Int("max-calls-in-request", session.Capabilities.Core.MaxCallsInRequest). + Int("calls-in-request", calls). + Msgf("number of calls in request payload (%d) would exceed the allowed maximum (%d)", session.Capabilities.Core.MaxCallsInRequest, calls) + return simpleError(errTooManyMethodCalls, JmapErrorTooManyMethodCalls) + } + return nil +} + +// Construct a Request from the given list of Invocation objects. +// +// If an issue occurs, then it is logged prior to returning it. +func (j *Client) request(session *Session, logger *log.Logger, methodCalls ...Invocation) (Request, Error) { + err := j.maxCallsCheck(len(methodCalls), session, logger) + if err != nil { + return Request{}, err + } + return Request{ + Using: []string{JmapCore, JmapMail}, + MethodCalls: methodCalls, + CreatedIds: nil, + }, nil +} diff --git a/pkg/jmap/jmap_error.go b/pkg/jmap/jmap_error.go index a21c09fe3b..bbb49b520d 100644 --- a/pkg/jmap/jmap_error.go +++ b/pkg/jmap/jmap_error.go @@ -1,6 +1,7 @@ package jmap import ( + "errors" "fmt" "strings" ) @@ -19,6 +20,11 @@ const ( JmapErrorInvalidJmapResponsePayload JmapErrorMethodLevel JmapErrorSetError + JmapErrorTooManyMethodCalls +) + +var ( + errTooManyMethodCalls = errors.New("the amount of methodCalls in the request body would exceed the maximum that is configured in the session") ) type Error interface { diff --git a/pkg/jmap/jmap_model.go b/pkg/jmap/jmap_model.go index ff277cc395..9ce4552005 100644 --- a/pkg/jmap/jmap_model.go +++ b/pkg/jmap/jmap_model.go @@ -1909,14 +1909,6 @@ type Request struct { CreatedIds map[string]string `json:"createdIds,omitempty"` } -func request(methodCalls ...Invocation) (Request, error) { - return Request{ - Using: []string{JmapCore, JmapMail}, - MethodCalls: methodCalls, - CreatedIds: nil, - }, nil -} - type Response struct { // An array of responses, in the same format as the methodCalls on the Request object. // The output of the methods MUST be added to the methodResponses array in the same order that the methods are processed. @@ -2380,7 +2372,7 @@ type Identity struct { Email string `json:"email,omitempty"` // The Reply-To value the client SHOULD set when creating a new Email from this Identity. - ReplyTo string `json:"replyTo:omitempty"` + ReplyTo string `json:"replyTo,omitempty"` // The Bcc value the client SHOULD set when creating a new Email from this Identity. Bcc []EmailAddress `json:"bcc,omitempty"` diff --git a/pkg/jmap/jmap_test.go b/pkg/jmap/jmap_test.go index 5db86e73f4..c927bbf97a 100644 --- a/pkg/jmap/jmap_test.go +++ b/pkg/jmap/jmap_test.go @@ -47,6 +47,11 @@ func (t *TestJmapWellKnownClient) GetSession(sessionUrl *url.URL, username strin Quota: pa, Websocket: pa, }, + Capabilities: SessionCapabilities{ + Core: SessionCoreCapabilities{ + MaxCallsInRequest: 64, + }, + }, }, nil } @@ -149,7 +154,17 @@ func TestRequests(t *testing.T) { jmapUrl, err := url.Parse("http://localhost/jmap") require.NoError(err) - session := Session{Username: "user123", JmapUrl: *jmapUrl} + session := Session{ + Username: "user123", + JmapUrl: *jmapUrl, + SessionResponse: SessionResponse{ + Capabilities: SessionCapabilities{ + Core: SessionCoreCapabilities{ + MaxCallsInRequest: 10, + }, + }, + }, + } foldersByAccountId, sessionState, err := client.GetAllMailboxes([]string{"a"}, &session, ctx, &logger) require.NoError(err) diff --git a/pkg/jmap/jmap_tools.go b/pkg/jmap/jmap_tools.go index 34bb25b46b..d7cc30521b 100644 --- a/pkg/jmap/jmap_tools.go +++ b/pkg/jmap/jmap_tools.go @@ -140,17 +140,19 @@ func retrieveResponseMatch(data *Response, command Command, tag string) (Invocat return Invocation{}, false } -func retrieveResponseMatchParameters[T any](data *Response, command Command, tag string, target *T) error { +func retrieveResponseMatchParameters[T any](logger *log.Logger, data *Response, command Command, tag string, target *T) Error { match, ok := retrieveResponseMatch(data, command, tag) if !ok { - return fmt.Errorf("failed to find JMAP response invocation match for command '%v' and tag '%v'", command, tag) + err := fmt.Errorf("failed to find JMAP response invocation match for command '%v' and tag '%v'", command, tag) + logger.Error().Msg(err.Error()) + return simpleError(err, JmapErrorInvalidJmapResponsePayload) } params := match.Parameters typedParams, ok := params.(T) if !ok { - actualType := reflect.TypeOf(params) - expectedType := reflect.TypeOf(*target) - return fmt.Errorf("JMAP response invocation matches command '%v' and tag '%v' but the type %v does not match the expected %v", command, tag, actualType, expectedType) + err := fmt.Errorf("JMAP response invocation matches command '%v' and tag '%v' but the type %T does not match the expected %T", command, tag, params, *target) + logger.Error().Msg(err.Error()) + return simpleError(err, JmapErrorInvalidJmapResponsePayload) } *target = typedParams return nil diff --git a/pkg/jmap/testdata/mailboxes1.json b/pkg/jmap/testdata/mailboxes1.json index 4ffb5c317f..28437a7636 100644 --- a/pkg/jmap/testdata/mailboxes1.json +++ b/pkg/jmap/testdata/mailboxes1.json @@ -116,6 +116,6 @@ } ], "notFound":[] - },"0"] + },"a:0"] ], "sessionState":"3e25b2a0" }