Skip to content

Commit 7d07d56

Browse files
committed
Fix for 'bad encrypted message' errors.
1) There was a regression in the outgoing multipart transport logic, such that the same 'identifier' byte would be used for all messages (0). This now works correctly. 2) Added some additional heuristics on the receiving side. Now mutlipart containers are only valid for 1hr, and are considered invalid if the container size is different from the multipart message size.
1 parent 4281df7 commit 7d07d56

File tree

3 files changed

+59
-21
lines changed

3 files changed

+59
-21
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package org.thoughtcrime.securesms.sms;
2+
3+
4+
import java.util.HashMap;
5+
6+
public class MultipartSmsIdentifier {
7+
8+
private static final MultipartSmsIdentifier instance = new MultipartSmsIdentifier();
9+
10+
public static MultipartSmsIdentifier getInstance() {
11+
return instance;
12+
}
13+
14+
private final HashMap<String, Integer> idMap = new HashMap<String, Integer>();
15+
16+
public synchronized byte getIdForRecipient(String recipient) {
17+
Integer currentId;
18+
19+
if (idMap.containsKey(recipient)) {
20+
currentId = idMap.get(recipient);
21+
idMap.remove(recipient);
22+
} else {
23+
currentId = 0;
24+
}
25+
26+
byte id = currentId.byteValue();
27+
idMap.put(recipient, (currentId + 1) % 255);
28+
29+
return id;
30+
}
31+
32+
}

src/org/thoughtcrime/securesms/sms/MultipartSmsMessageHandler.java

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ public class MultipartSmsMessageHandler {
2929
private final HashMap<String, MultipartSmsTransportMessageFragments> partialMessages =
3030
new HashMap<String, MultipartSmsTransportMessageFragments>();
3131

32-
private final HashMap<String, Integer> idMap = new HashMap<String, Integer>();
33-
3432
private IncomingTextMessage processMultipartMessage(MultipartSmsTransportMessage message) {
3533
Log.w("MultipartSmsMessageHandler", "Processing multipart message...");
34+
Log.w("MultipartSmsMessageHandler", "Multipart Count: " + message.getMultipartCount());
35+
Log.w("MultipartSmsMessageHandler", "Multipart ID: " + message.getIdentifier());
36+
Log.w("MultipartSmsMessageHandler", "Multipart Key: " + message.getKey());
3637
MultipartSmsTransportMessageFragments container = partialMessages.get(message.getKey());
3738

38-
if (container == null) {
39+
Log.w("MultipartSmsMessageHandler", "Found multipart container: " + container);
40+
41+
if (container == null || container.getSize() != message.getMultipartCount() || container.isExpired()) {
42+
Log.w("MultipartSmsMessageHandler", "Constructing new container...");
3943
container = new MultipartSmsTransportMessageFragments(message.getMultipartCount());
4044
partialMessages.put(message.getKey(), container);
4145
}
@@ -81,24 +85,9 @@ public IncomingTextMessage processPotentialMultipartMessage(IncomingTextMessage
8185
}
8286
}
8387

84-
private byte getIdForRecipient(String recipient) {
85-
Integer currentId;
86-
87-
if (idMap.containsKey(recipient)) {
88-
currentId = idMap.get(recipient);
89-
idMap.remove(recipient);
90-
} else {
91-
currentId = 0;
92-
}
93-
94-
byte id = currentId.byteValue();
95-
idMap.put(recipient, (currentId + 1) % 255);
96-
97-
return id;
98-
}
99-
10088
public ArrayList<String> divideMessage(OutgoingTextMessage message) {
101-
byte identifier = getIdForRecipient(message.getRecipients().getPrimaryRecipient().getNumber());
89+
String number = message.getRecipients().getPrimaryRecipient().getNumber();
90+
byte identifier = MultipartSmsIdentifier.getInstance().getIdForRecipient(number);
10291
return MultipartSmsTransportMessage.getEncoded(message, identifier);
10392
}
10493
}

src/org/thoughtcrime/securesms/sms/MultipartSmsTransportMessageFragments.java

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,28 @@
22

33
public class MultipartSmsTransportMessageFragments {
44

5+
private static final long VALID_TIME = 60 * 60 * 1000; // 1 Hour
6+
57
private final byte[][] fragments;
8+
private final long initializedTime;
69

710
public MultipartSmsTransportMessageFragments(int count) {
8-
this.fragments = new byte[count][];
11+
this.fragments = new byte[count][];
12+
this.initializedTime = System.currentTimeMillis();
913
}
1014

1115
public void add(MultipartSmsTransportMessage fragment) {
1216
this.fragments[fragment.getMultipartIndex()] = fragment.getStrippedMessage();
1317
}
1418

19+
public int getSize() {
20+
return this.fragments.length;
21+
}
22+
23+
public boolean isExpired() {
24+
return (System.currentTimeMillis() - initializedTime) >= VALID_TIME;
25+
}
26+
1527
public boolean isComplete() {
1628
for (int i=0;i<fragments.length;i++)
1729
if (fragments[i] == null) return false;
@@ -37,4 +49,9 @@ public byte[] getJoined() {
3749
return totalMessage;
3850
}
3951

52+
@Override
53+
public String toString() {
54+
return String.format("[Size: %d, Initialized: %d, Exipired: %s, Complete: %s]",
55+
fragments.length, initializedTime, isExpired()+"", isComplete()+"");
56+
}
4057
}

0 commit comments

Comments
 (0)