diff --git a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java index 37ccde7e9..c5440f25f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java +++ b/app/src/main/java/org/thoughtcrime/securesms/groups/v2/processing/GroupsV2StateProcessor.java @@ -372,6 +372,7 @@ public final class GroupsV2StateProcessor { Log.d(TAG, "Skipping profile key changes only update message"); } else { storeMessage(GroupProtoUtil.createDecryptedGroupV2Context(masterKey, new GroupMutation(previousGroupState, entry.getChange(), entry.getGroup()), null), timestamp); + timestamp++; } previousGroupState = entry.getGroup(); } @@ -476,7 +477,9 @@ public final class GroupsV2StateProcessor { IncomingTextMessage incoming = new IncomingTextMessage(sender, -1, timestamp, timestamp, "", Optional.of(groupId), 0, false); IncomingGroupUpdateMessage groupMessage = new IncomingGroupUpdateMessage(incoming, decryptedGroupV2Context); - smsDatabase.insertMessageInbox(groupMessage); + if (!smsDatabase.insertMessageInbox(groupMessage).isPresent()) { + Log.w(TAG, "Could not insert update message"); + } } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java index a01c04aaf..2fa821bdd 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/groupsv2/GroupsV2Api.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.security.SecureRandom; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.UUID; @@ -97,9 +98,19 @@ public final class GroupsV2Api { GroupsV2AuthorizationString authorization) throws IOException, InvalidGroupStateException, VerificationFailedException { - GroupChanges group = socket.getGroupsV2GroupHistory(fromRevision, authorization); + List changesList = new LinkedList<>(); + PushServiceSocket.GroupHistory group; + + do { + group = socket.getGroupsV2GroupHistory(fromRevision, authorization); + + changesList.addAll(group.getGroupChanges().getGroupChangesList()); + + if (group.hasMore()) { + fromRevision = group.getNextPageStartGroupRevision(); + } + } while (group.hasMore()); - List changesList = group.getGroupChangesList(); ArrayList result = new ArrayList<>(changesList.size()); GroupsV2Operations.GroupOperations groupOperations = groupsOperations.forGroup(groupSecretParams); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/ContentRange.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/ContentRange.java new file mode 100644 index 000000000..b8bded21f --- /dev/null +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/ContentRange.java @@ -0,0 +1,49 @@ +package org.whispersystems.signalservice.internal.push; + +import org.whispersystems.libsignal.util.guava.Optional; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public final class ContentRange { + private static final Pattern PATTERN = Pattern.compile("versions (\\d+)-(\\d+)\\/(\\d+)"); + + private final int rangeStart; + private final int rangeEnd; + private final int totalSize; + + /** + * Parses a content range header. + */ + public static Optional parse(String header) { + if (header != null) { + Matcher matcher = PATTERN.matcher(header); + + if (matcher.matches()) { + return Optional.of(new ContentRange(Integer.parseInt(matcher.group(1)), + Integer.parseInt(matcher.group(2)), + Integer.parseInt(matcher.group(3)))); + } + } + + return Optional.absent(); + } + + private ContentRange(int rangeStart, int rangeEnd, int totalSize) { + this.rangeStart = rangeStart; + this.rangeEnd = rangeEnd; + this.totalSize = totalSize; + } + + public int getRangeStart() { + return rangeStart; + } + + public int getRangeEnd() { + return rangeEnd; + } + + public int getTotalSize() { + return totalSize; + } +} diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java index e7323016f..0d598fe73 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/PushServiceSocket.java @@ -1585,6 +1585,12 @@ public class PushServiceSocket { private ResponseBody makeStorageRequest(String authorization, String path, String method, RequestBody body, ResponseCodeHandler responseCodeHandler) throws PushNetworkException, NonSuccessfulResponseCodeException + { + return makeStorageRequestResponse(authorization, path, method, body, responseCodeHandler).body(); + } + + private Response makeStorageRequestResponse(String authorization, String path, String method, RequestBody body, ResponseCodeHandler responseCodeHandler) + throws PushNetworkException, NonSuccessfulResponseCodeException { ConnectionHolder connectionHolder = getRandom(storageClients, random); OkHttpClient okHttpClient = connectionHolder.getClient() @@ -1618,7 +1624,7 @@ public class PushServiceSocket { response = call.execute(); if (response.isSuccessful() && response.code() != 204) { - return response.body(); + return response; } } catch (IOException e) { throw new PushNetworkException(e); @@ -1745,6 +1751,9 @@ public class PushServiceSocket { * Converts {@link IOException} on body byte reading to {@link PushNetworkException}. */ private static byte[] readBodyBytes(ResponseBody response) throws PushNetworkException { + if (response == null) { + throw new PushNetworkException("No body!"); + } try { return response.bytes(); } catch (IOException e) { @@ -1977,16 +1986,31 @@ public class PushServiceSocket { return GroupChange.parseFrom(readBodyBytes(response)); } - public GroupChanges getGroupsV2GroupHistory(int fromVersion, GroupsV2AuthorizationString authorization) + public GroupHistory getGroupsV2GroupHistory(int fromVersion, GroupsV2AuthorizationString authorization) throws NonSuccessfulResponseCodeException, PushNetworkException, InvalidProtocolBufferException { - ResponseBody response = makeStorageRequest(authorization.toString(), - String.format(Locale.US, GROUPSV2_GROUP_CHANGES, fromVersion), - "GET", - null, - GROUPS_V2_GET_LOGS_HANDLER); + Response response = makeStorageRequestResponse(authorization.toString(), + String.format(Locale.US, GROUPSV2_GROUP_CHANGES, fromVersion), + "GET", + null, + GROUPS_V2_GET_LOGS_HANDLER); - return GroupChanges.parseFrom(readBodyBytes(response)); + GroupChanges groupChanges = GroupChanges.parseFrom(readBodyBytes(response.body())); + + if (response.code() == 206) { + String contentRangeHeader = response.header("Content-Range"); + Optional contentRange = ContentRange.parse(contentRangeHeader); + + if (contentRange.isPresent()) { + Log.i(TAG, "Additional logs for group: " + contentRangeHeader); + return new GroupHistory(groupChanges, contentRange); + } else { + Log.w(TAG, "Unable to parse Content-Range header: " + contentRangeHeader); + throw new NonSuccessfulResponseCodeException("Unable to parse content range header on 206"); + } + } + + return new GroupHistory(groupChanges, Optional.absent()); } public GroupJoinInfo getGroupJoinInfo(Optional groupLinkPassword, GroupsV2AuthorizationString authorization) @@ -2002,6 +2026,31 @@ public class PushServiceSocket { return GroupJoinInfo.parseFrom(readBodyBytes(response)); } + public static final class GroupHistory { + private final GroupChanges groupChanges; + private final Optional contentRange; + + public GroupHistory(GroupChanges groupChanges, Optional contentRange) { + this.groupChanges = groupChanges; + this.contentRange = contentRange; + } + + public GroupChanges getGroupChanges() { + return groupChanges; + } + + public boolean hasMore() { + return contentRange.isPresent(); + } + + /** + * Valid iff {@link #hasMore()}. + */ + public int getNextPageStartGroupRevision() { + return contentRange.get().getRangeEnd() + 1; + } + } + private final class ResumeInfo { private final String contentRange; private final long contentStart; diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_Test.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_Test.java new file mode 100644 index 000000000..496835147 --- /dev/null +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_Test.java @@ -0,0 +1,49 @@ +package org.whispersystems.signalservice.internal.push; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.Arrays; +import java.util.Collection; + +import static org.junit.Assert.assertEquals; + +@RunWith(Parameterized.class) +public final class ContentRange_parse_Test { + + @Parameterized.Parameter(0) + public String input; + + @Parameterized.Parameter(1) + public int expectedRangeStart; + + @Parameterized.Parameter(2) + public int expectedRangeEnd; + + @Parameterized.Parameter(3) + public int expectedSize; + + @Parameterized.Parameters(name = "Content-Range: \"{0}\"") + public static Collection data() { + return Arrays.asList(new Object[][]{ + { "versions 1-2/3", 1, 2, 3 }, + { "versions 23-45/67", 23, 45, 67 }, + }); + } + + @Test + public void rangeStart() { + assertEquals(expectedRangeStart, ContentRange.parse(input).get().getRangeStart()); + } + + @Test + public void rangeEnd() { + assertEquals(expectedRangeEnd, ContentRange.parse(input).get().getRangeEnd()); + } + + @Test + public void totalSize() { + assertEquals(expectedSize, ContentRange.parse(input).get().getTotalSize()); + } +} diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_withInvalidStrings_Test.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_withInvalidStrings_Test.java new file mode 100644 index 000000000..9abffba9d --- /dev/null +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/ContentRange_parse_withInvalidStrings_Test.java @@ -0,0 +1,34 @@ +package org.whispersystems.signalservice.internal.push; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import java.util.Arrays; +import java.util.Collection; + +import static org.junit.Assert.assertFalse; + +@RunWith(Parameterized.class) +public final class ContentRange_parse_withInvalidStrings_Test { + + @Parameterized.Parameter(0) + public String input; + + @Parameterized.Parameters(name = "Content-Range: \"{0}\"") + public static Collection data() { + return Arrays.asList(new Object[][]{ + { null }, + { "" }, + { "23-45/67" }, + { "ersions 23-45/67" }, + { "versions 23-45" }, + { "versions a-b/c" } + }); + } + + @Test + public void parse_should_be_absent() { + assertFalse(ContentRange.parse(input).isPresent()); + } +}