Skip to content

Commit

Permalink
Respect the 206 paged response from the server group logs endpoint.
Browse files Browse the repository at this point in the history
Prevent the deduplicate message logic firing and log it if it does.
  • Loading branch information
alan-signal committed Oct 9, 2020
1 parent 07b0d8c commit 054c705
Show file tree
Hide file tree
Showing 6 changed files with 206 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ private void insertUpdateMessages(long timestamp,
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();
}
Expand Down Expand Up @@ -476,7 +477,9 @@ private void storeMessage(@NonNull DecryptedGroupV2Context decryptedGroupV2Conte
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");
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.UUID;

Expand Down Expand Up @@ -97,9 +98,19 @@ public List<DecryptedGroupHistoryEntry> getGroupHistory(GroupSecretParams groupS
GroupsV2AuthorizationString authorization)
throws IOException, InvalidGroupStateException, VerificationFailedException
{
GroupChanges group = socket.getGroupsV2GroupHistory(fromRevision, authorization);
List<GroupChanges.GroupChangeState> 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<GroupChanges.GroupChangeState> changesList = group.getGroupChangesList();
ArrayList<DecryptedGroupHistoryEntry> result = new ArrayList<>(changesList.size());
GroupsV2Operations.GroupOperations groupOperations = groupsOperations.forGroup(groupSecretParams);

Expand Down
Original file line number Diff line number Diff line change
@@ -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<ContentRange> 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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1585,6 +1585,12 @@ private ResponseBody makeStorageRequest(String authorization, String path, Strin

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()
Expand Down Expand Up @@ -1618,7 +1624,7 @@ private ResponseBody makeStorageRequest(String authorization, String path, Strin
response = call.execute();

if (response.isSuccessful() && response.code() != 204) {
return response.body();
return response;
}
} catch (IOException e) {
throw new PushNetworkException(e);
Expand Down Expand Up @@ -1745,6 +1751,9 @@ public ProfileKeyCredential parseResponse(UUID uuid, ProfileKey profileKey, Prof
* 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) {
Expand Down Expand Up @@ -1977,16 +1986,31 @@ public GroupChange patchGroupsV2Group(GroupChange.Actions groupChange, String au
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);

GroupChanges groupChanges = GroupChanges.parseFrom(readBodyBytes(response.body()));

if (response.code() == 206) {
String contentRangeHeader = response.header("Content-Range");
Optional<ContentRange> contentRange = ContentRange.parse(contentRangeHeader);

return GroupChanges.parseFrom(readBodyBytes(response));
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<byte[]> groupLinkPassword, GroupsV2AuthorizationString authorization)
Expand All @@ -2002,6 +2026,31 @@ public GroupJoinInfo getGroupJoinInfo(Optional<byte[]> groupLinkPassword, Groups
return GroupJoinInfo.parseFrom(readBodyBytes(response));
}

public static final class GroupHistory {
private final GroupChanges groupChanges;
private final Optional<ContentRange> contentRange;

public GroupHistory(GroupChanges groupChanges, Optional<ContentRange> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Object[]> 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());
}
}
Original file line number Diff line number Diff line change
@@ -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<Object[]> 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());
}
}

0 comments on commit 054c705

Please sign in to comment.