Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GODT-2683): Reduce validation checks when appending into Drafts #363

Merged
merged 1 commit into from
Jun 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions internal/session/handle_append.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,22 @@ func (s *Session) handleAppend(ctx context.Context, tag string, cmd *command.App
return response.Bad(tag).WithError(err)
}

if err := rfc5322.ValidateMessageHeaderFields(cmd.Literal); err != nil {
return response.Bad(tag).WithError(err)
}

if err := s.state.AppendOnlyMailbox(ctx, nameUTF8, func(mailbox state.AppendOnlyMailbox, isSameMBox bool) error {
isDrafts, err := mailbox.IsDrafts(ctx)
if err != nil {
return err
}

if !isDrafts {
if err := rfc5322.ValidateMessageHeaderFields(cmd.Literal); err != nil {
return response.Bad(tag).WithError(err)
}
} else {
if err := rfc5322.ValidateMessageHeaderFieldsDrafts(cmd.Literal); err != nil {
return response.Bad(tag).WithError(err)
}
}

messageUID, err := mailbox.Append(ctx, cmd.Literal, flags, cmd.DateTime)
if err != nil {
if shouldReportIMAPCommandError(err) {
Expand Down
10 changes: 10 additions & 0 deletions internal/state/mailbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type AppendOnlyMailbox interface {
Append(ctx context.Context, literal []byte, flags imap.FlagSet, date time.Time) (imap.UID, error)
Flush(ctx context.Context, permitExpunge bool) ([]response.Response, error)
UIDValidity() imap.UID
IsDrafts(ctx context.Context) (bool, error)
}

func newMailbox(mbox *db.Mailbox, state *State, snap *snapshot) *Mailbox {
Expand Down Expand Up @@ -251,6 +252,15 @@ func (m *Mailbox) Append(ctx context.Context, literal []byte, flags imap.FlagSet
return uid, err
}

func (m *Mailbox) IsDrafts(ctx context.Context) (bool, error) {
attrs, err := m.Attributes(ctx)
if err != nil {
return false, err
}

return attrs.Contains(imap.AttrDrafts), nil
}

// Copy copies the messages represented by the given sequence set into the mailbox with the given name.
// If the context is a UID context, the sequence set refers to message UIDs.
// If no items are copied the response object will be nil.
Expand Down
24 changes: 24 additions & 0 deletions rfc5322/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,27 @@ func ValidateMessageHeaderFields(literal []byte) error {

return nil
}

// ValidateMessageHeaderFieldsDrafts checks the headers of message to verify that at least a valid From header is
// present.
func ValidateMessageHeaderFieldsDrafts(literal []byte) error {
headerBytes, _ := rfc822.Split(literal)

header, err := rfc822.NewHeader(headerBytes)
if err != nil {
return err
}

// Check for from.
value := header.Get("From")
if len(value) == 0 {
return fmt.Errorf("%w: Required header field 'From' not found or empty", ErrInvalidMessage)
}

// Check if From is a multi address. If so, a sender filed must be present and non-empty.
if _, err := ParseAddressList(value); err != nil {
return fmt.Errorf("%w: failed to parse From header: %v", ErrInvalidMessage, err)
}

return nil
}
34 changes: 34 additions & 0 deletions tests/append_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,3 +449,37 @@ func TestGODT2007AppendInternalIDPresentOnDeletedMessage(t *testing.T) {
}
})
}

func TestAppendIntoDraftsWithFromOnly(t *testing.T) {
const (
literalWithFrom = `From: Foo@bar`
literalWithoutFrom = `To: Foo@bar`
literalValid = `From: Foo@bar
Date: Wed, 26 Apr 2023 08:25:16 +0200
`
)

runOneToOneTestClientWithAuth(t, defaultServerOptions(t), func(client *client.Client, s *testSession) {

s.mailboxCreatedWithAttributes("user", []string{"Drafts"}, imap.NewFlagSet(imap.AttrDrafts))
s.flush("user")

{
require.NoError(t, doAppendWithClient(client, "Drafts", literalWithFrom, time.Now()))
require.NoError(t, doAppendWithClient(client, "INBOX", literalValid, time.Now()))
require.Error(t, doAppendWithClient(client, "Drafts", literalWithoutFrom, time.Now()))
require.Error(t, doAppendWithClient(client, "INBOX", literalWithoutFrom, time.Now()))
}

{
status, err := client.Status("Drafts", []goimap.StatusItem{goimap.StatusMessages})
require.NoError(t, err)
require.Equal(t, uint32(1), status.Messages, "Expected message count does not match")
}
{
status, err := client.Status("INBOX", []goimap.StatusItem{goimap.StatusMessages})
require.NoError(t, err)
require.Equal(t, uint32(1), status.Messages, "Expected message count does not match")
}
})
}