Escape unsafe characters in urls.
Fixes #133
diff --git a/integration/volley/src/androidTest/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java b/integration/volley/src/androidTest/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java
index 8d50837..e70c616 100644
--- a/integration/volley/src/androidTest/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java
+++ b/integration/volley/src/androidTest/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java
@@ -8,6 +8,7 @@
import com.android.volley.toolbox.Volley;
import com.bumptech.glide.Priority;
import com.bumptech.glide.load.data.DataFetcher;
+import com.bumptech.glide.load.model.GlideUrl;
import com.squareup.okhttp.mockwebserver.MockResponse;
import com.squareup.okhttp.mockwebserver.MockWebServer;
import org.junit.After;
@@ -196,7 +197,7 @@
return super.get();
}
};
- return new VolleyStreamFetcher(requestQueue, url.toString(), requestFuture);
+ return new VolleyStreamFetcher(requestQueue, new GlideUrl(url.toString()), requestFuture);
}
private static String isToString(InputStream is) throws IOException {
diff --git a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
index 8b9e5f9..d78723d 100644
--- a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
+++ b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyStreamFetcher.java
@@ -7,24 +7,27 @@
import com.android.volley.toolbox.HttpHeaderParser;
import com.bumptech.glide.Priority;
import com.bumptech.glide.load.data.DataFetcher;
+import com.bumptech.glide.load.model.GlideUrl;
import java.io.ByteArrayInputStream;
import java.io.InputStream;
+import java.lang.reflect.Method;
/**
* A DataFetcher backed by volley for fetching images via http.
*/
public class VolleyStreamFetcher implements DataFetcher<InputStream> {
private final RequestQueue requestQueue;
- private final String url;
+ private final GlideUrl url;
private VolleyRequestFuture<InputStream> requestFuture;
@SuppressWarnings("unused")
- public VolleyStreamFetcher(RequestQueue requestQueue, String url) {
+ public VolleyStreamFetcher(RequestQueue requestQueue, GlideUrl url) {
this(requestQueue, url, null);
}
- public VolleyStreamFetcher(RequestQueue requestQueue, String url, VolleyRequestFuture<InputStream> requestFuture) {
+ public VolleyStreamFetcher(RequestQueue requestQueue, GlideUrl url,
+ VolleyRequestFuture<InputStream> requestFuture) {
this.requestQueue = requestQueue;
this.url = url;
this.requestFuture = requestFuture;
@@ -35,7 +38,9 @@
@Override
public InputStream loadData(Priority priority) throws Exception {
- GlideRequest request = new GlideRequest(url, requestFuture, glideToVolleyPriority(priority));
+ // Make sure the string url safely encodes non ascii characters.
+ String stringUrl = url.toURL().toString();
+ GlideRequest request = new GlideRequest(stringUrl, requestFuture, glideToVolleyPriority(priority));
requestFuture.setRequest(requestQueue.add(request));
@@ -49,7 +54,7 @@
@Override
public String getId() {
- return url;
+ return url.toString();
}
@Override
diff --git a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyUrlLoader.java b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyUrlLoader.java
index b08b022..b2093ef 100644
--- a/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyUrlLoader.java
+++ b/integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyUrlLoader.java
@@ -67,6 +67,6 @@
@Override
public DataFetcher<InputStream> getResourceFetcher(GlideUrl url, int width, int height) {
- return new VolleyStreamFetcher(requestQueue, url.toString(), new VolleyRequestFuture<InputStream>());
+ return new VolleyStreamFetcher(requestQueue, url, new VolleyRequestFuture<InputStream>());
}
}
diff --git a/library/src/androidTest/java/com/bumptech/glide/load/model/GlideUrlTest.java b/library/src/androidTest/java/com/bumptech/glide/load/model/GlideUrlTest.java
index ed8559a..5d0071b 100644
--- a/library/src/androidTest/java/com/bumptech/glide/load/model/GlideUrlTest.java
+++ b/library/src/androidTest/java/com/bumptech/glide/load/model/GlideUrlTest.java
@@ -60,4 +60,26 @@
assertEquals(expected, glideUrl.toString());
}
+
+ @Test
+ public void testIssue133() throws MalformedURLException {
+ // u00e0=à
+ final String original = "http://www.commitstrip.com/wp-content/uploads/2014/07/"
+ + "Excel-\u00E0-toutes-les-sauces-650-finalenglish.jpg";
+
+ final String escaped = "http://www.commitstrip.com/wp-content/uploads/2014/07/"
+ + "Excel-%C3%A0-toutes-les-sauces-650-finalenglish.jpg";
+
+ GlideUrl glideUrlFromString = new GlideUrl(original);
+ assertEquals(escaped, glideUrlFromString.toURL().toString());
+
+ GlideUrl glideUrlFromEscapedString = new GlideUrl(escaped);
+ assertEquals(escaped, glideUrlFromEscapedString.toURL().toString());
+
+ GlideUrl glideUrlFromUrl = new GlideUrl(new URL(original));
+ assertEquals(escaped, glideUrlFromUrl.toURL().toString());
+
+ GlideUrl glideUrlFromEscapedUrl = new GlideUrl(new URL(escaped));
+ assertEquals(escaped, glideUrlFromEscapedUrl.toURL().toString());
+ }
}
\ No newline at end of file
diff --git a/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java b/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java
index ddb8dcb..b87b3cf 100644
--- a/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java
+++ b/library/src/main/java/com/bumptech/glide/load/model/GlideUrl.java
@@ -1,19 +1,30 @@
package com.bumptech.glide.load.model;
+import android.net.Uri;
import android.text.TextUtils;
import java.net.MalformedURLException;
import java.net.URL;
/**
- * This is a simple wrapper for strings representing http/https URLs. new URL() is an excessively expensive operation
- * that may be unnecessary if the class loading the image from the URL doesn't actually require a URL object.
+ * A wrapper for strings representing http/https URLs responsible for ensuring URLs are properly escaped and avoiding
+ * unnecessary URL instantiations for loaders that require only string urls rather than URL objects.
*
- * Users wishing to replace the class for handling URLs must register a factory using GlideUrl.
+ * <p>
+ * Users wishing to replace the class for handling URLs must register a factory using GlideUrl.
+ * </p>
+ *
+ * <p>
+ * To obtain a properly escaped URL, call {@link #toURL()}. To obtain a properly escaped string URL, call
+ * {@link #toURL()} and then {@link java.net.URL#toString()}.
+ * </p>
*/
public class GlideUrl {
private String stringUrl;
+ private static final String ALLOWED_URI_CHARS = "@#&=*+-_.,:!?()/~'%";
+
private URL url;
+ private URL safeUrl;
public GlideUrl(URL url) {
if (url == null) {
@@ -32,10 +43,21 @@
}
public URL toURL() throws MalformedURLException {
- if (url == null) {
- url = new URL(stringUrl);
+ return getSafeUrl();
+ }
+
+ // See http://stackoverflow.com/questions/3286067/url-encoding-in-android. Although the answer using URI would work,
+ // using it would require both decoding and encoding each string which is more complicated, slower and generates
+ // more objects than the solution below. See also issue #133.
+ private URL getSafeUrl() throws MalformedURLException {
+ if (safeUrl != null) {
+ return safeUrl;
}
- return url;
+ String unsafe = toString();
+ String safe = Uri.encode(unsafe, ALLOWED_URI_CHARS);
+
+ safeUrl = new URL(safe);
+ return safeUrl;
}
@Override
diff --git a/library/src/main/java/com/bumptech/glide/load/model/stream/HttpUrlGlideUrlLoader.java b/library/src/main/java/com/bumptech/glide/load/model/stream/HttpUrlGlideUrlLoader.java
index 086a55b..573f709 100644
--- a/library/src/main/java/com/bumptech/glide/load/model/stream/HttpUrlGlideUrlLoader.java
+++ b/library/src/main/java/com/bumptech/glide/load/model/stream/HttpUrlGlideUrlLoader.java
@@ -5,6 +5,7 @@
import com.bumptech.glide.load.data.HttpUrlFetcher;
import com.bumptech.glide.load.model.GenericLoaderFactory;
import com.bumptech.glide.load.model.GlideUrl;
+import com.bumptech.glide.load.model.ModelCache;
import com.bumptech.glide.load.model.ModelLoader;
import com.bumptech.glide.load.model.ModelLoaderFactory;
@@ -16,13 +17,17 @@
*/
public class HttpUrlGlideUrlLoader implements ModelLoader<GlideUrl, InputStream> {
+ private ModelCache<GlideUrl, GlideUrl> modelCache;
+
/**
* The default factory for {@link com.bumptech.glide.load.model.stream.HttpUrlGlideUrlLoader}s.
*/
public static class Factory implements ModelLoaderFactory<GlideUrl, InputStream> {
+ private ModelCache<GlideUrl, GlideUrl> modelCache = new ModelCache<GlideUrl, GlideUrl>(500);
+
@Override
public ModelLoader<GlideUrl, InputStream> build(Context context, GenericLoaderFactory factories) {
- return new HttpUrlGlideUrlLoader();
+ return new HttpUrlGlideUrlLoader(modelCache);
}
@Override
@@ -31,8 +36,25 @@
}
}
+ public HttpUrlGlideUrlLoader() {
+ // Empty.
+ }
+
+ public HttpUrlGlideUrlLoader(ModelCache<GlideUrl, GlideUrl> modelCache) {
+ this.modelCache = modelCache;
+ }
+
@Override
public DataFetcher<InputStream> getResourceFetcher(GlideUrl model, int width, int height) {
- return new HttpUrlFetcher(model);
+ // GlideUrls memoize parsed URLs so caching them saves a few object instantiations and time spent parsing urls.
+ GlideUrl url = model;
+ if (modelCache != null) {
+ url = modelCache.get(model, 0, 0);
+ if (url == null) {
+ modelCache.put(model, 0, 0, model);
+ url = model;
+ }
+ }
+ return new HttpUrlFetcher(url);
}
}
