From dc903e49af94de4339b90e1e757d5f2948536435 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Mon, 18 May 2015 08:16:06 -0700 Subject: [PATCH] Fix image cache bugs. 1) Index SlideDeckListener cache by MMS (id, timestamp) tuple. 2) Index parts by (id, content_id) tuples. Fixes #840 Closes #3183 // FREEBIE --- .../securesms/database/MmsDatabase.java | 19 ++++---- .../securesms/database/PartDatabase.java | 43 +++++++++++-------- .../securesms/mms/ImageSlide.java | 2 +- .../securesms/mms/IncomingMediaMessage.java | 4 ++ .../securesms/mms/PartAuthority.java | 33 +++++++++----- .../thoughtcrime/securesms/mms/PartUri.java | 30 +++++++++++++ .../securesms/providers/PartProvider.java | 26 +++++++---- .../com/google/android/mms/pdu/PduPart.java | 1 + 8 files changed, 113 insertions(+), 45 deletions(-) create mode 100644 src/org/thoughtcrime/securesms/mms/PartUri.java diff --git a/src/org/thoughtcrime/securesms/database/MmsDatabase.java b/src/org/thoughtcrime/securesms/database/MmsDatabase.java index ec504b9730..e74d71325b 100644 --- a/src/org/thoughtcrime/securesms/database/MmsDatabase.java +++ b/src/org/thoughtcrime/securesms/database/MmsDatabase.java @@ -165,8 +165,8 @@ public class MmsDatabase extends MessagingDatabase { }; public static final ExecutorService slideResolver = org.thoughtcrime.securesms.util.Util.newSingleThreadedLifoExecutor(); - private static final Map> slideCache = - Collections.synchronizedMap(new LRUCache>(20)); + private static final Map> slideCache = + Collections.synchronizedMap(new LRUCache>(20)); private final JobManager jobManager; @@ -1045,7 +1045,7 @@ public class MmsDatabase extends MessagingDatabase { List mismatches = getMismatchedIdentities(mismatchDocument); List networkFailures = getFailures(networkDocument); - ListenableFutureTask slideDeck = getSlideDeck(masterSecret, id); + ListenableFutureTask slideDeck = getSlideDeck(masterSecret, dateReceived, id); return new MediaMmsMessageRecord(context, id, recipients, recipients.getPrimaryRecipient(), addressDeviceId, dateSent, dateReceived, receiptCount, @@ -1109,9 +1109,10 @@ public class MmsDatabase extends MessagingDatabase { } private ListenableFutureTask getSlideDeck(final MasterSecret masterSecret, + final long timestamp, final long id) { - ListenableFutureTask future = getCachedSlideDeck(id); + ListenableFutureTask future = getCachedSlideDeck(timestamp, id); if (future != null) { return future; @@ -1128,21 +1129,21 @@ public class MmsDatabase extends MessagingDatabase { SlideDeck slideDeck = new SlideDeck(context, masterSecret, body); if (!body.containsPushInProgress()) { - slideCache.put(id, new SoftReference(slideDeck)); + slideCache.put(timestamp + "::" + id, new SoftReference<>(slideDeck)); } return slideDeck; } }; - future = new ListenableFutureTask(task); + future = new ListenableFutureTask<>(task); slideResolver.execute(future); return future; } - private ListenableFutureTask getCachedSlideDeck(final long id) { - SoftReference reference = slideCache.get(id); + private ListenableFutureTask getCachedSlideDeck(final long timestamp, final long id) { + SoftReference reference = slideCache.get(timestamp + "::" + id); if (reference != null) { final SlideDeck slideDeck = reference.get(); @@ -1155,7 +1156,7 @@ public class MmsDatabase extends MessagingDatabase { } }; - ListenableFutureTask future = new ListenableFutureTask(task); + ListenableFutureTask future = new ListenableFutureTask<>(task); future.run(); return future; diff --git a/src/org/thoughtcrime/securesms/database/PartDatabase.java b/src/org/thoughtcrime/securesms/database/PartDatabase.java index 9fcffdeb8f..f89ef49f99 100644 --- a/src/org/thoughtcrime/securesms/database/PartDatabase.java +++ b/src/org/thoughtcrime/securesms/database/PartDatabase.java @@ -79,6 +79,8 @@ public class PartDatabase extends Database { private static final String THUMBNAIL = "thumbnail"; private static final String ASPECT_RATIO = "aspect_ratio"; + private static final String ID_CONTENT_WHERE = ID + " = ? AND " + CONTENT_ID + " = ?"; + public static final String CREATE_TABLE = "CREATE TABLE " + TABLE_NAME + " (" + ID + " INTEGER PRIMARY KEY, " + MMS_ID + " INTEGER, " + SEQUENCE + " INTEGER DEFAULT 0, " + CONTENT_TYPE + " TEXT, " + NAME + " TEXT, " + CHARSET + " INTEGER, " + @@ -113,10 +115,10 @@ public class PartDatabase extends Database { super(context, databaseHelper); } - public InputStream getPartStream(MasterSecret masterSecret, long partId) + public InputStream getPartStream(MasterSecret masterSecret, long partId, byte[] contentId) throws FileNotFoundException { - return getDataStream(masterSecret, partId, DATA); + return getDataStream(masterSecret, partId, contentId, DATA); } public void updateFailedDownloadedPart(long messageId, long partId, PduPart part) @@ -206,7 +208,7 @@ public class PartDatabase extends Database { cursor.close(); } - database.delete(TABLE_NAME, MMS_ID + " = ?", new String[] {mmsId+""}); + database.delete(TABLE_NAME, MMS_ID + " = ?", new String[] {mmsId + ""}); } @SuppressWarnings("ResultOfMethodCallIgnored") @@ -343,15 +345,17 @@ public class PartDatabase extends Database { return new EncryptingPartOutputStream(path, masterSecret); } - @VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, String dataType) + @VisibleForTesting InputStream getDataStream(MasterSecret masterSecret, long partId, byte[] contentId, String dataType) throws FileNotFoundException { SQLiteDatabase database = databaseHelper.getReadableDatabase(); Cursor cursor = null; try { - cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_WHERE, - new String[] {partId+""}, null, null, null); + cursor = database.query(TABLE_NAME, new String[]{dataType}, ID_CONTENT_WHERE, + new String[] {String.valueOf(partId), + Util.toIsoString(contentId)}, + null, null, null); if (cursor != null && cursor.moveToFirst()) { if (cursor.isNull(0)) { @@ -402,15 +406,15 @@ public class PartDatabase extends Database { } } - public InputStream getThumbnailStream(final MasterSecret masterSecret, final long partId) throws IOException { + public InputStream getThumbnailStream(MasterSecret masterSecret, long partId, byte[] contentId) throws IOException { Log.w(TAG, "getThumbnailStream(" + partId + ")"); - final InputStream dataStream = getDataStream(masterSecret, partId, THUMBNAIL); + final InputStream dataStream = getDataStream(masterSecret, partId, contentId, THUMBNAIL); if (dataStream != null) { return dataStream; } try { - return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)).get(); + return thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, contentId)).get(); } catch (InterruptedException ie) { throw new AssertionError("interrupted"); } catch (ExecutionException ee) { @@ -424,7 +428,7 @@ public class PartDatabase extends Database { getPartValues(part, cursor); - part.setDataUri(ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, part.getId())); + part.setDataUri(PartAuthority.getPartUri(part.getId(), part.getContentId())); return part; } @@ -454,7 +458,7 @@ public class PartDatabase extends Database { ThumbnailData data = new ThumbnailData(thumbnail); updatePartThumbnail(masterSecret, partId, part, data.toDataStream(), data.getAspectRatio()); } else if (!part.isPendingPush()) { - thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)); + thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId())); } return partId; @@ -479,7 +483,7 @@ public class PartDatabase extends Database { database.update(TABLE_NAME, values, ID_WHERE, new String[]{partId+""}); - thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId)); + thumbnailExecutor.submit(new ThumbnailFetchCallable(masterSecret, partId, part.getContentId())); notifyConversationListeners(DatabaseFactory.getMmsDatabase(context).getThreadIdForMessage(messageId)); } @@ -534,11 +538,12 @@ public class PartDatabase extends Database { public static class ImageRecord { private long partId; + private byte[] contentId; private String contentType; private String address; private long date; - private ImageRecord(long partId, String contentType, String address, long date) { + private ImageRecord(long partId, byte[] contentId, String contentType, String address, long date) { this.partId = partId; this.contentType = contentType; this.address = address; @@ -547,6 +552,7 @@ public class PartDatabase extends Database { public static ImageRecord from(Cursor cursor) { return new ImageRecord(cursor.getLong(cursor.getColumnIndexOrThrow(ID)), + Util.toIsoBytes(cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_ID))), cursor.getString(cursor.getColumnIndexOrThrow(CONTENT_TYPE)), cursor.getString(cursor.getColumnIndexOrThrow(MmsDatabase.ADDRESS)), cursor.getLong(cursor.getColumnIndexOrThrow(MmsDatabase.NORMALIZED_DATE_RECEIVED)) * 1000); @@ -569,22 +575,24 @@ public class PartDatabase extends Database { } public Uri getUri() { - return ContentUris.withAppendedId(PartAuthority.PART_CONTENT_URI, getPartId()); + return PartAuthority.getPartUri(partId, contentId); } } @VisibleForTesting class ThumbnailFetchCallable implements Callable { private final MasterSecret masterSecret; private final long partId; + private final byte[] contentId; - public ThumbnailFetchCallable(MasterSecret masterSecret, long partId) { + public ThumbnailFetchCallable(MasterSecret masterSecret, long partId, byte[] contentId) { this.masterSecret = masterSecret; this.partId = partId; + this.contentId = contentId; } @Override public InputStream call() throws Exception { - final InputStream stream = getDataStream(masterSecret, partId, THUMBNAIL); + final InputStream stream = getDataStream(masterSecret, partId, contentId, THUMBNAIL); if (stream != null) { return stream; } @@ -599,7 +607,8 @@ public class PartDatabase extends Database { } catch (BitmapDecodingException bde) { throw new IOException(bde); } - return getDataStream(masterSecret, partId, THUMBNAIL); + + return getDataStream(masterSecret, partId, contentId, THUMBNAIL); } } } diff --git a/src/org/thoughtcrime/securesms/mms/ImageSlide.java b/src/org/thoughtcrime/securesms/mms/ImageSlide.java index 63e3a652c7..faf51a7b2a 100644 --- a/src/org/thoughtcrime/securesms/mms/ImageSlide.java +++ b/src/org/thoughtcrime/securesms/mms/ImageSlide.java @@ -46,7 +46,7 @@ public class ImageSlide extends Slide { if (!getPart().isPendingPush() && getPart().getDataUri() != null) { return isDraft() ? getPart().getDataUri() - : PartAuthority.getThumbnailUri(getPart().getId()); + : PartAuthority.getThumbnailUri(getPart().getId(), part.getContentId()); } return null; diff --git a/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java b/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java index bf92437727..4b8f13e22b 100644 --- a/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java +++ b/src/org/thoughtcrime/securesms/mms/IncomingMediaMessage.java @@ -32,6 +32,10 @@ public class IncomingMediaMessage { this.body = retreived.getBody(); this.groupId = null; this.push = false; + + for (int i=0;i(); + setContentId(String.valueOf(System.currentTimeMillis()).getBytes()); } public void setEncrypted(boolean isEncrypted) {