From 96006426c97d4e4a3b4378585a847b50cd8b63d7 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sun, 20 Oct 2013 11:47:04 -0700 Subject: [PATCH 1/3] Reduce the odds of a collision in image keys Fixes #27 --- .../bumptech/glide/resize/ImageManager.java | 91 ++++++++++++++++++- .../resize/cache/DiskLruCacheWrapper.java | 4 +- library/src/com/bumptech/glide/util/Util.java | 21 +++-- 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/library/src/com/bumptech/glide/resize/ImageManager.java b/library/src/com/bumptech/glide/resize/ImageManager.java index 2b124872c5..5636025bd5 100644 --- a/library/src/com/bumptech/glide/resize/ImageManager.java +++ b/library/src/com/bumptech/glide/resize/ImageManager.java @@ -36,6 +36,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.UnsupportedEncodingException; +import java.nio.ByteBuffer; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -68,6 +74,7 @@ public class ImageManager { private final MemoryCache memoryCache; private final ImageResizer resizer; private final DiskCache diskCache; + private final SafeKeyGenerator safeKeyGenerator = new SafeKeyGenerator(); //special downsampler that doesn't check exif, and assumes inWidth and inHeight == outWidth and outHeight so it //doesn't need to read the image header for size information @@ -389,7 +396,7 @@ public void onImageRemoved(Bitmap removed) { public ImageManagerJob getImage(String id, StreamLoader streamLoader, Transformation transformation, Downsampler downsampler, int width, int height, LoadedCallback cb) { if (shutdown) return null; - final String key = getKey(id, transformation.getId(), downsampler, width, height); + final String key = safeKeyGenerator.getSafeKey(id, transformation, downsampler, width, height); ImageManagerJob job = null; if (!returnFromCache(key, cb)) { @@ -631,8 +638,84 @@ private void putInMemoryCache(String key, final Bitmap bitmap) { } } - private static String getKey(String id, String transformationId, Downsampler downsampler, int width, int height) { - return String.valueOf(Util.hash(id.hashCode(), downsampler.getId().hashCode(), - transformationId.hashCode(), width, height)); + private static class SafeKeyGenerator { + private final Map loadIdToSafeHash = new HashMap(); + private final ByteBuffer byteBuffer = ByteBuffer.allocate(8); + private MessageDigest messageDigest; + + public SafeKeyGenerator() { + try { + messageDigest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + e.printStackTrace(); + } + } + + public String getSafeKey(String id, Transformation transformation, Downsampler downsampler, int width, int height) { + LoadId loadId = new LoadId(id, transformation.getId(), downsampler.getId(), width, height); + String safeKey = loadIdToSafeHash.get(loadId); + if (safeKey == null) { + try { + safeKey = loadId.generateSafeKey(); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + loadIdToSafeHash.put(loadId, safeKey); + } + return safeKey; + } + + private class LoadId { + private final String id; + private final String transformationId; + private final String downsamplerId; + private final int width; + private final int height; + + public LoadId(String id, String transformationId, String downsamplerId, int width, int height) { + this.id = id; + this.transformationId = transformationId; + this.downsamplerId = downsamplerId; + this.width = width; + this.height = height; + } + + public String generateSafeKey() throws UnsupportedEncodingException { + messageDigest.update(id.getBytes("UTF-8")); + messageDigest.update(transformationId.getBytes("UTF-8")); + messageDigest.update(downsamplerId.getBytes("UTF-8")); + byteBuffer.position(0); + byteBuffer.putInt(width); + byteBuffer.putInt(height); + messageDigest.update(byteBuffer.array()); + return Util.sha256BytesToHex(messageDigest.digest()); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + LoadId loadId = (LoadId) o; + + if (height != loadId.height) return false; + if (width != loadId.width) return false; + if (!downsamplerId.equals(loadId.downsamplerId)) return false; + if (!id.equals(loadId.id)) return false; + if (!transformationId.equals(loadId.transformationId)) return false; + + return true; + } + + @Override + public int hashCode() { + int result = id.hashCode(); + result = 31 * result + transformationId.hashCode(); + result = 31 * result + downsamplerId.hashCode(); + result = 31 * result + width; + result = 31 * result + height; + return result; + } + } } } diff --git a/library/src/com/bumptech/glide/resize/cache/DiskLruCacheWrapper.java b/library/src/com/bumptech/glide/resize/cache/DiskLruCacheWrapper.java index f28f6c4514..9673934b1d 100644 --- a/library/src/com/bumptech/glide/resize/cache/DiskLruCacheWrapper.java +++ b/library/src/com/bumptech/glide/resize/cache/DiskLruCacheWrapper.java @@ -18,12 +18,14 @@ */ public class DiskLruCacheWrapper implements DiskCache { + private static final int APP_VERSION = 1; + private static final int VALUE_COUNT = 1; private static DiskLruCache CACHE = null; private static DiskLruCacheWrapper WRAPPER = null; private synchronized static DiskLruCache getDiskLruCache(File directory, int maxSize) throws IOException { if (CACHE == null) { - CACHE = DiskLruCache.open(directory, 0, 1, maxSize); + CACHE = DiskLruCache.open(directory, APP_VERSION, VALUE_COUNT, maxSize); } return CACHE; } diff --git a/library/src/com/bumptech/glide/util/Util.java b/library/src/com/bumptech/glide/util/Util.java index 44d64d5f00..999707e510 100644 --- a/library/src/com/bumptech/glide/util/Util.java +++ b/library/src/com/bumptech/glide/util/Util.java @@ -1,13 +1,22 @@ package com.bumptech.glide.util; public class Util { - private static final int PRIME = 31; + private static final char[] hexArray = "0123456789abcdef".toCharArray(); + private static final char[] sha256Chars = new char[64]; //32 bytes from sha-256 -> 64 hex chars - public static int hash(int... hashes) { - int result = 1; - for (int hash : hashes) { - result *= PRIME * hash; + public static String sha256BytesToHex(byte[] bytes) { + return bytesToHex(bytes, sha256Chars); + } + + // Taken from: + // http://stackoverflow.com/questions/9655181/convert-from-byte-array-to-hex-string-in-java/9655275#9655275 + private static String bytesToHex(byte[] bytes, char[] hexChars) { + int v; + for ( int j = 0; j < bytes.length; j++ ) { + v = bytes[j] & 0xFF; + hexChars[j * 2] = hexArray[v >>> 4]; + hexChars[j * 2 + 1] = hexArray[v & 0x0F]; } - return result; + return new String(hexChars); } } From f59163dea0fd9b71ad8b3bce80f6852f38e6eab8 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Tue, 5 Nov 2013 07:47:59 -0800 Subject: [PATCH 2/3] move key generator to its own class and add test --- .../bumptech/glide/resize/ImageManager.java | 81 ----------- .../glide/resize/SafeKeyGenerator.java | 128 ++++++++++++++++++ .../com/bumptech/glide/KeyGeneratorTest.java | 71 ++++++++++ 3 files changed, 199 insertions(+), 81 deletions(-) create mode 100644 library/src/com/bumptech/glide/resize/SafeKeyGenerator.java create mode 100644 library/tests/src/com/bumptech/glide/KeyGeneratorTest.java diff --git a/library/src/com/bumptech/glide/resize/ImageManager.java b/library/src/com/bumptech/glide/resize/ImageManager.java index 5636025bd5..50749cd948 100644 --- a/library/src/com/bumptech/glide/resize/ImageManager.java +++ b/library/src/com/bumptech/glide/resize/ImageManager.java @@ -637,85 +637,4 @@ private void putInMemoryCache(String key, final Bitmap bitmap) { memoryCache.put(key, bitmap); } } - - private static class SafeKeyGenerator { - private final Map loadIdToSafeHash = new HashMap(); - private final ByteBuffer byteBuffer = ByteBuffer.allocate(8); - private MessageDigest messageDigest; - - public SafeKeyGenerator() { - try { - messageDigest = MessageDigest.getInstance("SHA-256"); - } catch (NoSuchAlgorithmException e) { - e.printStackTrace(); - } - } - - public String getSafeKey(String id, Transformation transformation, Downsampler downsampler, int width, int height) { - LoadId loadId = new LoadId(id, transformation.getId(), downsampler.getId(), width, height); - String safeKey = loadIdToSafeHash.get(loadId); - if (safeKey == null) { - try { - safeKey = loadId.generateSafeKey(); - } catch (UnsupportedEncodingException e) { - e.printStackTrace(); - } - loadIdToSafeHash.put(loadId, safeKey); - } - return safeKey; - } - - private class LoadId { - private final String id; - private final String transformationId; - private final String downsamplerId; - private final int width; - private final int height; - - public LoadId(String id, String transformationId, String downsamplerId, int width, int height) { - this.id = id; - this.transformationId = transformationId; - this.downsamplerId = downsamplerId; - this.width = width; - this.height = height; - } - - public String generateSafeKey() throws UnsupportedEncodingException { - messageDigest.update(id.getBytes("UTF-8")); - messageDigest.update(transformationId.getBytes("UTF-8")); - messageDigest.update(downsamplerId.getBytes("UTF-8")); - byteBuffer.position(0); - byteBuffer.putInt(width); - byteBuffer.putInt(height); - messageDigest.update(byteBuffer.array()); - return Util.sha256BytesToHex(messageDigest.digest()); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - LoadId loadId = (LoadId) o; - - if (height != loadId.height) return false; - if (width != loadId.width) return false; - if (!downsamplerId.equals(loadId.downsamplerId)) return false; - if (!id.equals(loadId.id)) return false; - if (!transformationId.equals(loadId.transformationId)) return false; - - return true; - } - - @Override - public int hashCode() { - int result = id.hashCode(); - result = 31 * result + transformationId.hashCode(); - result = 31 * result + downsamplerId.hashCode(); - result = 31 * result + width; - result = 31 * result + height; - return result; - } - } - } } diff --git a/library/src/com/bumptech/glide/resize/SafeKeyGenerator.java b/library/src/com/bumptech/glide/resize/SafeKeyGenerator.java new file mode 100644 index 0000000000..a9ea864795 --- /dev/null +++ b/library/src/com/bumptech/glide/resize/SafeKeyGenerator.java @@ -0,0 +1,128 @@ +package com.bumptech.glide.resize; + +import android.os.Build; +import com.bumptech.glide.resize.load.Downsampler; +import com.bumptech.glide.resize.load.Transformation; +import com.bumptech.glide.util.Util; + +import java.io.UnsupportedEncodingException; +import java.nio.ByteBuffer; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayDeque; +import java.util.HashMap; +import java.util.LinkedList; +import java.util.Map; +import java.util.Queue; + +public class SafeKeyGenerator { + private final Map loadIdToSafeHash = new HashMap(); + private final ByteBuffer byteBuffer = ByteBuffer.allocate(8); + private final LoadIdPool loadIdPool = new LoadIdPool(); + private MessageDigest messageDigest; + + public SafeKeyGenerator() { + try { + messageDigest = MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + e.printStackTrace(); + } + } + + public String getSafeKey(String id, Transformation transformation, Downsampler downsampler, int width, int height) { + LoadId loadId = loadIdPool.get(id, transformation.getId(), downsampler.getId(), width, height); + String safeKey = loadIdToSafeHash.get(loadId); + if (safeKey == null) { + try { + safeKey = loadId.generateSafeKey(); + } catch (UnsupportedEncodingException e) { + e.printStackTrace(); + } + loadIdToSafeHash.put(loadId, safeKey); + } else { + loadIdPool.offer(loadId); + } + return safeKey; + } + + private class LoadIdPool { + private static final int MAX_SIZE = 20; + private Queue loadIdQueue; + + public LoadIdPool() { + if (Build.VERSION.SDK_INT >= 9) { + loadIdQueue = new ArrayDeque(MAX_SIZE); + } else { + loadIdQueue = new LinkedList(); + } + } + + public LoadId get(String id, String transformationId, String downsamplerId, int width, int height) { + LoadId loadId = loadIdQueue.poll(); + if (loadId == null) { + loadId = new LoadId(); + } + loadId.init(id, transformationId, downsamplerId, width, height); + return loadId; + } + + public void offer(LoadId loadId) { + if (loadIdQueue.size() < MAX_SIZE) { + loadIdQueue.offer(loadId); + } + } + } + + private class LoadId { + private String id; + private String transformationId; + private String downsamplerId; + private int width; + private int height; + + public void init(String id, String transformationId, String downsamplerId, int width, int height) { + this.id = id; + this.transformationId = transformationId; + this.downsamplerId = downsamplerId; + this.width = width; + this.height = height; + } + + public String generateSafeKey() throws UnsupportedEncodingException { + messageDigest.update(id.getBytes("UTF-8")); + messageDigest.update(transformationId.getBytes("UTF-8")); + messageDigest.update(downsamplerId.getBytes("UTF-8")); + byteBuffer.position(0); + byteBuffer.putInt(width); + byteBuffer.putInt(height); + messageDigest.update(byteBuffer.array()); + return Util.sha256BytesToHex(messageDigest.digest()); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + + LoadId loadId = (LoadId) o; + + if (height != loadId.height) return false; + if (width != loadId.width) return false; + if (!downsamplerId.equals(loadId.downsamplerId)) return false; + if (!id.equals(loadId.id)) return false; + if (!transformationId.equals(loadId.transformationId)) return false; + + return true; + } + + @Override + public int hashCode() { + int result = id.hashCode(); + result = 31 * result + transformationId.hashCode(); + result = 31 * result + downsamplerId.hashCode(); + result = 31 * result + width; + result = 31 * result + height; + return result; + } + } +} diff --git a/library/tests/src/com/bumptech/glide/KeyGeneratorTest.java b/library/tests/src/com/bumptech/glide/KeyGeneratorTest.java new file mode 100644 index 0000000000..4c12fffcdf --- /dev/null +++ b/library/tests/src/com/bumptech/glide/KeyGeneratorTest.java @@ -0,0 +1,71 @@ +package com.bumptech.glide; + +import android.graphics.Bitmap; +import android.test.AndroidTestCase; +import com.bumptech.glide.resize.SafeKeyGenerator; +import com.bumptech.glide.resize.bitmap_recycle.BitmapPool; +import com.bumptech.glide.resize.load.Downsampler; +import com.bumptech.glide.resize.load.Transformation; + +import java.util.UUID; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class KeyGeneratorTest extends AndroidTestCase { + private SafeKeyGenerator keyGenerator; + + @Override + protected void setUp() throws Exception { + super.setUp(); + keyGenerator = new SafeKeyGenerator(); + } + + public void testKeysAreValidForDiskCache() { + String key; + final Pattern diskCacheRegex = Pattern.compile("[a-z0-9_-]{64}"); + for (int i = 0; i < 1000; i++) { + key = getRandomKeyFromGenerator(); + final Matcher matcher = diskCacheRegex.matcher(key); + assertTrue(matcher.matches()); + } + } + + private String getRandomKeyFromGenerator() { + return keyGenerator.getSafeKey(getRandomId(), new RandomTransformation(), new RandomDownsampler(), + getRandomDimen(), getRandomDimen()); + } + + private static int getRandomDimen() { + return (int) Math.round(Math.random() * 1000); + } + + private static String getRandomId() { + return UUID.randomUUID().toString(); + } + + private static class RandomDownsampler extends Downsampler { + + @Override + protected int getSampleSize(int inWidth, int inHeight, int outWidth, int outHeight) { + return 0; + } + + @Override + public String getId() { + return getRandomId(); + } + } + + private static class RandomTransformation extends Transformation { + + @Override + public Bitmap transform(Bitmap bitmap, BitmapPool pool, int outWidth, int outHeight) { + return null; + } + + @Override + public String getId() { + return getRandomId(); + } + } +} From 90ee177aea615192ad71cc87cdcee76cbe05f606 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Tue, 5 Nov 2013 07:50:36 -0800 Subject: [PATCH 3/3] remove unused imports --- library/src/com/bumptech/glide/resize/ImageManager.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/library/src/com/bumptech/glide/resize/ImageManager.java b/library/src/com/bumptech/glide/resize/ImageManager.java index 50749cd948..13833ff17a 100644 --- a/library/src/com/bumptech/glide/resize/ImageManager.java +++ b/library/src/com/bumptech/glide/resize/ImageManager.java @@ -30,18 +30,11 @@ import com.bumptech.glide.resize.load.ImageResizer; import com.bumptech.glide.resize.load.Transformation; import com.bumptech.glide.util.Log; -import com.bumptech.glide.util.Util; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.UnsupportedEncodingException; -import java.nio.ByteBuffer; -import java.security.MessageDigest; -import java.security.NoSuchAlgorithmException; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future;