diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java index b29a98721..422b92b69 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/WhisperServerService.java @@ -722,7 +722,7 @@ public class WhisperServerService extends Application maybeClientVersionTag = UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager); validateIndividualMessageBundle(destination, destinationIdentifier, messagesByDeviceId, registrationIdsByDeviceId, syncMessageSenderDeviceId, - platformTag); + platformTag, + maybeClientVersionTag); messagesManager.insert(destination.getIdentifier(IdentityType.ACI), messagesByDeviceId) .forEach((deviceId, destinationPresent) -> { @@ -173,8 +181,9 @@ public class MessageSender { @Nullable final String userAgent) throws MultiRecipientMismatchedDevicesException, MessageTooLargeException { final Tag platformTag = UserAgentTagUtil.getPlatformTag(userAgent); + final Optional maybeClientVersionTag = UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager); - validateMultiRecipientMessageContentLength(multiRecipientMessage, isStory, platformTag); + validateMultiRecipientMessageContentLength(multiRecipientMessage, isStory, platformTag, maybeClientVersionTag); final Map mismatchedDevicesByServiceIdentifier = new HashMap<>(); @@ -253,14 +262,16 @@ public class MessageSender { final Map messagesByDeviceId, final Map registrationIdsByDeviceId, @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional syncMessageSenderDeviceId, - @Nullable final String userAgent) throws MessageTooLargeException, MismatchedDevicesException { + @Nullable final String userAgent, + final ClientReleaseManager clientReleaseManager) throws MessageTooLargeException, MismatchedDevicesException { validateIndividualMessageBundle(destination, destinationIdentifier, messagesByDeviceId, registrationIdsByDeviceId, syncMessageSenderDeviceId, - UserAgentTagUtil.getPlatformTag(userAgent)); + UserAgentTagUtil.getPlatformTag(userAgent), + UserAgentTagUtil.getClientVersionTag(userAgent, clientReleaseManager)); } private static void validateIndividualMessageBundle(final Account destination, @@ -268,7 +279,8 @@ public class MessageSender { final Map messagesByDeviceId, final Map registrationIdsByDeviceId, @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional syncMessageSenderDeviceId, - final Tag platformTag) throws MismatchedDevicesException, MessageTooLargeException { + final Tag platformTag, + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional maybeClientVersionTag) throws MismatchedDevicesException, MessageTooLargeException { if (!destination.isIdentifiedBy(destinationIdentifier)) { throw new IllegalArgumentException("Destination account not identified by destination service identifier"); @@ -305,7 +317,10 @@ public class MessageSender { throw new MismatchedDevicesException(maybeMismatchedDevices.get()); } - validateIndividualMessageContentLength(messagesByDeviceId.values(), syncMessageSenderDeviceId.isPresent(), platformTag); + validateIndividualMessageContentLength(messagesByDeviceId.values(), + syncMessageSenderDeviceId.isPresent(), + platformTag, + maybeClientVersionTag); } @VisibleForTesting @@ -313,7 +328,8 @@ public class MessageSender { final boolean isMultiRecipientMessage, final boolean isSyncMessage, final boolean isStory, - final Tag platformTag) throws MessageTooLargeException { + final Tag platformTag, + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional maybeClientVersionTag) throws MessageTooLargeException { final boolean oversize = contentLength > MAX_MESSAGE_SIZE; @@ -328,11 +344,14 @@ public class MessageSender { .record(contentLength); if (contentLength > OVERSIZE_MESSAGE_WARNING_THRESHOLD) { - Metrics.counter(OVERSIZE_MESSAGE_WARNING_COUNTER_NAME, Tags.of(platformTag, - Tag.of("multiRecipientMessage", String.valueOf(isMultiRecipientMessage)), - Tag.of("syncMessage", String.valueOf(isSyncMessage)), - Tag.of("story", String.valueOf(isStory)))) - .increment(); + Tags tags = Tags.of(platformTag, + Tag.of("multiRecipientMessage", String.valueOf(isMultiRecipientMessage)), + Tag.of("syncMessage", String.valueOf(isSyncMessage)), + Tag.of("story", String.valueOf(isStory))); + + tags = maybeClientVersionTag.map(tags::and).orElse(tags); + + Metrics.counter(OVERSIZE_MESSAGE_WARNING_COUNTER_NAME, tags).increment(); } if (oversize) { @@ -387,27 +406,31 @@ public class MessageSender { private static void validateIndividualMessageContentLength(final Iterable messages, final boolean isSyncMessage, - final Tag platformTag) throws MessageTooLargeException { + final Tag platformTag, + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional maybeClientVersionTag) throws MessageTooLargeException { for (final Envelope message : messages) { MessageSender.validateContentLength(message.getContent().size(), false, isSyncMessage, message.getStory(), - platformTag); + platformTag, + maybeClientVersionTag); } } private static void validateMultiRecipientMessageContentLength(final SealedSenderMultiRecipientMessage multiRecipientMessage, final boolean isStory, - final Tag platformTag) throws MessageTooLargeException { + final Tag platformTag, + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") final Optional maybeClientVersionTag) throws MessageTooLargeException { for (final SealedSenderMultiRecipientMessage.Recipient recipient : multiRecipientMessage.getRecipients().values()) { MessageSender.validateContentLength(multiRecipientMessage.messageSizeForRecipient(recipient), true, false, isStory, - platformTag); + platformTag, + maybeClientVersionTag); } } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java index 4cf04b624..662d5fd3d 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManager.java @@ -27,15 +27,18 @@ public class ChangeNumberManager { private static final Logger logger = LoggerFactory.getLogger(ChangeNumberManager.class); private final MessageSender messageSender; private final AccountsManager accountsManager; + private final ClientReleaseManager clientReleaseManager; private final Clock clock; public ChangeNumberManager( final MessageSender messageSender, final AccountsManager accountsManager, + final ClientReleaseManager clientReleaseManager, final Clock clock) { this.messageSender = messageSender; this.accountsManager = accountsManager; + this.clientReleaseManager = clientReleaseManager; this.clock = clock; } @@ -76,7 +79,8 @@ public class ChangeNumberManager { messagesByDeviceId, registrationIdsByDeviceId, Optional.of(Device.PRIMARY_ID), - senderUserAgent); + senderUserAgent, + clientReleaseManager); } final Account updatedAccount = accountsManager.changeNumber( diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java index fefa12981..5c4b81b6f 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/push/MessageSenderTest.java @@ -48,6 +48,7 @@ import org.whispersystems.textsecuregcm.identity.PniServiceIdentifier; import org.whispersystems.textsecuregcm.identity.ServiceIdentifier; import org.whispersystems.textsecuregcm.metrics.UserAgentTagUtil; import org.whispersystems.textsecuregcm.storage.Account; +import org.whispersystems.textsecuregcm.storage.ClientReleaseManager; import org.whispersystems.textsecuregcm.storage.Device; import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.tests.util.MultiRecipientMessageHelper; @@ -65,7 +66,7 @@ class MessageSenderTest { messagesManager = mock(MessagesManager.class); pushNotificationManager = mock(PushNotificationManager.class); - messageSender = new MessageSender(messagesManager, pushNotificationManager); + messageSender = new MessageSender(messagesManager, pushNotificationManager, mock(ClientReleaseManager.class)); } @@ -275,7 +276,8 @@ class MessageSenderTest { messagesByDeviceId, registrationIdsByDeviceId, syncMessageSenderDeviceId, - "Signal/Test"); + "Signal/Test", + mock(ClientReleaseManager.class)); if (expectedExceptionClass != null) { assertThrows(expectedExceptionClass, validateIndividualMessageBundle); @@ -434,10 +436,10 @@ class MessageSenderTest { @Test void validateContentLength() { assertThrows(MessageTooLargeException.class, () -> - MessageSender.validateContentLength(MessageSender.MAX_MESSAGE_SIZE + 1, false, false, false, Tag.of(UserAgentTagUtil.PLATFORM_TAG, "test"))); + MessageSender.validateContentLength(MessageSender.MAX_MESSAGE_SIZE + 1, false, false, false, Tag.of(UserAgentTagUtil.PLATFORM_TAG, "test"), Optional.empty())); assertDoesNotThrow(() -> - MessageSender.validateContentLength(MessageSender.MAX_MESSAGE_SIZE, false, false, false, Tag.of(UserAgentTagUtil.PLATFORM_TAG, "test"))); + MessageSender.validateContentLength(MessageSender.MAX_MESSAGE_SIZE, false, false, false, Tag.of(UserAgentTagUtil.PLATFORM_TAG, "test"), Optional.empty())); } @ParameterizedTest diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java index 6704af13c..4d21c22e2 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/storage/ChangeNumberManagerTest.java @@ -51,7 +51,7 @@ public class ChangeNumberManagerTest { void setUp() throws Exception { accountsManager = mock(AccountsManager.class); messageSender = mock(MessageSender.class); - changeNumberManager = new ChangeNumberManager(messageSender, accountsManager, CLOCK); + changeNumberManager = new ChangeNumberManager(messageSender, accountsManager, mock(ClientReleaseManager.class), CLOCK); updatedPhoneNumberIdentifiersByAccount = new HashMap<>();