diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java index 15a15ffaf..1b729bea8 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/controllers/MessageController.java @@ -194,8 +194,17 @@ public class MessageController { validateCompleteDeviceList(destination.get(), messages.getMessages(), isSyncMessage); validateRegistrationIds(destination.get(), messages.getMessages()); + // iOS versions prior to 5.5.0.7 send `online` on IncomingMessageList.message, rather on the top-level entity. + // This causes some odd client behaviors, such as persisted typing indicators, so we have a temporary + // server-side adaptation. + final boolean online = messages.getMessages() + .stream() + .findFirst() + .map(IncomingMessage::isOnline) + .orElse(messages.isOnline()); + final List tags = List.of(UserAgentTagUtil.getPlatformTag(userAgent), - Tag.of(EPHEMERAL_TAG_NAME, String.valueOf(messages.isOnline())), + Tag.of(EPHEMERAL_TAG_NAME, String.valueOf(online)), Tag.of(SENDER_TYPE_TAG_NAME, senderType)); for (IncomingMessage incomingMessage : messages.getMessages()) { @@ -203,7 +212,7 @@ public class MessageController { if (destinationDevice.isPresent()) { Metrics.counter(SENT_MESSAGE_COUNTER_NAME, tags).increment(); - sendMessage(source, destination.get(), destinationDevice.get(), messages.getTimestamp(), messages.isOnline(), incomingMessage); + sendMessage(source, destination.get(), destinationDevice.get(), messages.getTimestamp(), online, incomingMessage); } } diff --git a/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessage.java b/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessage.java index c4fd2ceda..d8aca7297 100644 --- a/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessage.java +++ b/service/src/main/java/org/whispersystems/textsecuregcm/entities/IncomingMessage.java @@ -32,6 +32,9 @@ public class IncomingMessage { @JsonProperty private long timestamp; // deprecated + @JsonProperty + private Boolean online; // use IncomingMessageList.online - this is a temporary adaptation for older clients + public String getDestination() { return destination; } @@ -60,4 +63,7 @@ public class IncomingMessage { return content; } + public Boolean isOnline() { + return online; + } } diff --git a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java index 3fc39070e..57643750a 100644 --- a/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java +++ b/service/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/MessageControllerTest.java @@ -29,6 +29,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableSet; import io.dropwizard.auth.PolymorphicAuthValueFactoryProvider; import io.dropwizard.testing.junit.ResourceTestRule; +import java.io.IOException; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -39,11 +40,14 @@ import java.util.concurrent.TimeUnit; import javax.ws.rs.client.Entity; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; import org.glassfish.jersey.test.grizzly.GrizzlyWebTestContainerFactory; import org.junit.Before; import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.whispersystems.textsecuregcm.auth.AmbiguousIdentifier; @@ -71,6 +75,7 @@ import org.whispersystems.textsecuregcm.storage.MessagesManager; import org.whispersystems.textsecuregcm.tests.util.AuthHelper; import org.whispersystems.textsecuregcm.util.Base64; +@RunWith(JUnitParamsRunner.class) public class MessageControllerTest { private static final String SINGLE_DEVICE_RECIPIENT = "+14151111111"; @@ -381,4 +386,34 @@ public class MessageControllerTest { } + @Test + @Parameters(method = "argumentsForTestOnlineMessage") + public void testOnlineMessage(final String fixture, final boolean expectedOnline) throws Exception { + + final Response response = + resources.getJerseyTest() + .target(String.format("/v1/messages/%s", SINGLE_DEVICE_RECIPIENT)) + .request() + .header("Authorization", AuthHelper.getAuthHeader(AuthHelper.VALID_NUMBER, AuthHelper.VALID_PASSWORD)) + .put(Entity.entity(mapper.readValue(jsonFixture(fixture), IncomingMessageList.class), + MediaType.APPLICATION_JSON_TYPE)); + + assertThat("Good Response", response.getStatus(), is(equalTo(200))); + + verify(messageSender, times(1)).sendMessage(any(Account.class), any(Device.class), any(Envelope.class), eq(expectedOnline)); + } + + private static Object argumentsForTestOnlineMessage() { + return new Object[] { + new Object[] { "fixtures/current_message_single_device.json", false }, // default to `false` when absent + new Object[] { "fixtures/online_message_true.json", true }, + new Object[] { "fixtures/online_message_false.json", false }, + // iOS versions prior to 5.5.0.7 send `online` on IncomingMessageList.message, rather on the top-level entity. + // This causes some odd client behaviors, such as persisted typing indicators, so we have a temporary + // server-side adaptation. + new Object[] { "fixtures/online_message_true_nested_property.json", true }, + new Object[] { "fixtures/online_message_false_nested_property.json", false }, + }; + } + } diff --git a/service/src/test/resources/fixtures/online_message_false.json b/service/src/test/resources/fixtures/online_message_false.json new file mode 100644 index 000000000..265e10feb --- /dev/null +++ b/service/src/test/resources/fixtures/online_message_false.json @@ -0,0 +1,11 @@ +{ + "online": false, + "messages": [ + { + "type": 1, + "destinationDeviceId": 1, + "body": "Zm9vYmFyego", + "timestamp": 1234 + } + ] +} diff --git a/service/src/test/resources/fixtures/online_message_false_nested_property.json b/service/src/test/resources/fixtures/online_message_false_nested_property.json new file mode 100644 index 000000000..77111f71d --- /dev/null +++ b/service/src/test/resources/fixtures/online_message_false_nested_property.json @@ -0,0 +1,11 @@ +{ + "messages": [ + { + "type": 1, + "destinationDeviceId": 1, + "body": "Zm9vYmFyego", + "timestamp": 1234, + "online": false + } + ] +} diff --git a/service/src/test/resources/fixtures/online_message_true.json b/service/src/test/resources/fixtures/online_message_true.json new file mode 100644 index 000000000..6fd0a6a61 --- /dev/null +++ b/service/src/test/resources/fixtures/online_message_true.json @@ -0,0 +1,11 @@ +{ + "online": true, + "messages": [ + { + "type": 1, + "destinationDeviceId": 1, + "body": "Zm9vYmFyego", + "timestamp": 1234 + } + ] +} diff --git a/service/src/test/resources/fixtures/online_message_true_nested_property.json b/service/src/test/resources/fixtures/online_message_true_nested_property.json new file mode 100644 index 000000000..3437cc17f --- /dev/null +++ b/service/src/test/resources/fixtures/online_message_true_nested_property.json @@ -0,0 +1,11 @@ +{ + "messages": [ + { + "type": 1, + "destinationDeviceId": 1, + "body": "Zm9vYmFyego", + "timestamp": 1234, + "online": true + } + ] +}