Escape unsafe characters in urls.

Fixes #133

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 @@ public InputStream get() throws InterruptedException, ExecutionException {
                 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 {

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 @@ public VolleyStreamFetcher(RequestQueue requestQueue, String url, VolleyRequestF
 
     @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 @@ public void cleanup() {
 
     @Override
     public String getId() {
-        return url;
+        return url.toString();
     }
 
     @Override

integration/volley/src/main/java/com/bumptech/glide/integration/volley/VolleyUrlLoader.java

@@ -67,6 +67,6 @@ public VolleyUrlLoader(RequestQueue requestQueue) {
 
     @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>());
     }
 }

library/src/androidTest/java/com/bumptech/glide/load/model/GlideUrlTest.java

@@ -60,4 +60,26 @@ public void testProducesEquivalentStringFromURL() throws MalformedURLException {
 
         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());
+    }
 }

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 GlideUrl(String url) {
     }
 
     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

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 void teardown() {
         }
     }
 
+    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);
     }
 }