This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit ee4510bde05e0a83ad4c669820fb70f32c6d4cce Author: Benoit Tellier <[email protected]> AuthorDate: Sat Mar 28 19:17:37 2020 +0700 JAMES-3105 Additional sanitizing for mailbox counters --- .../james/mailbox/model/MailboxCounters.java | 32 +++++++++++ .../james/mailbox/model/MailboxCountersTest.java | 64 ++++++++++++++++++++++ .../james/jmap/draft/model/MailboxFactory.java | 7 ++- 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxCounters.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxCounters.java index d05eb65..3cceed5 100644 --- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxCounters.java +++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/MailboxCounters.java @@ -24,6 +24,7 @@ import org.slf4j.LoggerFactory; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; +import com.google.common.base.Preconditions; public class MailboxCounters { private static final Logger LOGGER = LoggerFactory.getLogger(MailboxCounters.class); @@ -61,6 +62,20 @@ public class MailboxCounters { } } + public static class Sanitized extends MailboxCounters { + static Sanitized of(MailboxId mailboxId, long count, long unseen) { + Preconditions.checkArgument(count >= 0, "'count' need to be strictly positive"); + Preconditions.checkArgument(unseen >= 0, "'count' need to be strictly positive"); + Preconditions.checkArgument(count >= unseen, "'unseen' cannot exceed 'count'"); + + return new Sanitized(mailboxId, count, unseen); + } + + private Sanitized(MailboxId mailboxId, long count, long unseen) { + super(mailboxId, count, unseen); + } + } + public static Builder.RequireMailboxId builder() { return mailboxId -> count -> unseen -> new Builder.FinalStage(count, unseen, mailboxId); } @@ -87,6 +102,23 @@ public class MailboxCounters { return unseen; } + public MailboxCounters.Sanitized sanitize() { + if (!isValid()) { + LOGGER.warn("Invalid mailbox counters for {} : {} / {}", mailboxId, unseen, count); + } + long sanitizedCount = Math.max(count, 0); + long positiveUnseen = Math.max(unseen, 0); + long sanitizedUnseen = Math.min(positiveUnseen, sanitizedCount); + + return Sanitized.of(mailboxId, sanitizedCount, sanitizedUnseen); + } + + private boolean isValid() { + return count >= 0 + && unseen >= 0 + && count >= unseen; + } + @Override public final boolean equals(Object o) { if (o instanceof MailboxCounters) { diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxCountersTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxCountersTest.java index b7efa25..d08b988 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxCountersTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/model/MailboxCountersTest.java @@ -20,14 +20,78 @@ package org.apache.james.mailbox.model; +import static org.assertj.core.api.Assertions.assertThat; + import org.junit.jupiter.api.Test; import nl.jqno.equalsverifier.EqualsVerifier; class MailboxCountersTest { + public static final TestId MAILBOX_ID = TestId.of(36); + @Test void mailboxCountersShouldRespectBeanContract() { EqualsVerifier.forClass(MailboxCounters.class).verify(); } + + @Test + void sanitizeShouldCorrectNegativeCount() { + assertThat(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(-1) + .unseen(0) + .build() + .sanitize()) + .isEqualTo(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(0) + .unseen(0) + .build()); + } + + @Test + void sanitizeShouldCorrectNegativeUnseen() { + assertThat(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(12) + .unseen(-1) + .build() + .sanitize()) + .isEqualTo(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(12) + .unseen(0) + .build()); + } + + @Test + void sanitizeShouldCorrectUnseenExceedingCount() { + assertThat(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(12) + .unseen(36) + .build() + .sanitize()) + .isEqualTo(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(12) + .unseen(12) + .build()); + } + + @Test + void sanitizeShouldNoopWhenValid() { + assertThat(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(36) + .unseen(12) + .build() + .sanitize()) + .isEqualTo(MailboxCounters.builder() + .mailboxId(MAILBOX_ID) + .count(36) + .unseen(12) + .build()); + } } diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java index c0ea2b8..456f58a 100644 --- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java +++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/model/MailboxFactory.java @@ -110,8 +110,9 @@ public class MailboxFactory { MailboxPath mailboxPath = mailboxMetaData.map(MailboxMetaData::getPath) .orElseGet(Throwing.supplier(() -> retrieveCachedMailbox(mailboxId, mailbox).getMailboxPath()).sneakyThrow()); - MailboxCounters mailboxCounters = mailboxMetaData.map(MailboxMetaData::getCounters) - .orElseGet(Throwing.supplier(() -> retrieveCachedMailbox(mailboxId, mailbox).getMailboxCounters(session)).sneakyThrow()); + MailboxCounters.Sanitized mailboxCounters = mailboxMetaData.map(MailboxMetaData::getCounters) + .orElseGet(Throwing.supplier(() -> retrieveCachedMailbox(mailboxId, mailbox).getMailboxCounters(session)).sneakyThrow()) + .sanitize(); return Optional.of(mailboxFactory.from( mailboxId, @@ -163,7 +164,7 @@ public class MailboxFactory { private Mailbox from(MailboxId mailboxId, MailboxPath mailboxPath, - MailboxCounters mailboxCounters, + MailboxCounters.Sanitized mailboxCounters, MailboxACL resolvedAcl, Optional<List<MailboxMetaData>> userMailboxesMetadata, QuotaLoader quotaLoader, --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
