Issue 303 - Don't load element hiding rules

src/org/adblockplus/android/AndroidWebRequest.java

Index: src/org/adblockplus/android/AndroidWebRequest.java
diff --git a/src/org/adblockplus/android/AndroidWebRequest.java b/src/org/adblockplus/android/AndroidWebRequest.java
index 35102acc610982d0bc7b5aad471bd9cf1431628a..223cbfbca7f07a2a0625dd014b9ccc76f76b3889 100644
--- a/src/org/adblockplus/android/AndroidWebRequest.java
+++ b/src/org/adblockplus/android/AndroidWebRequest.java
@@ -17,16 +17,19 @@
 
 package org.adblockplus.android;
 
-import java.io.InputStream;
+import java.io.BufferedReader;
+import java.io.InputStreamReader;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.util.List;
+import java.util.Locale;
 
 import org.adblockplus.libadblockplus.AdblockPlusException;
 import org.adblockplus.libadblockplus.HeaderEntry;
 import org.adblockplus.libadblockplus.ServerResponse;
-import org.adblockplus.libadblockplus.WebRequest;
 import org.adblockplus.libadblockplus.ServerResponse.NsStatus;
+import org.adblockplus.libadblockplus.WebRequest;
+import org.apache.commons.lang.StringUtils;
 
 import android.util.Log;
 
@@ -34,8 +37,24 @@ public class AndroidWebRequest extends WebRequest
 {
   public final String TAG = Utils.getTag(WebRequest.class);
 
-  private static final int INITIAL_BUFFER_SIZE = 65536;
-  private static final int BUFFER_GROWTH_DELTA = 65536;
+  private static boolean mightBeFilterList(URL url)
+  {
+    String filename = url.getFile();
+
+    if (StringUtils.isEmpty(filename))
+    {
+      return false;
+    }
+
+    final int params = filename.indexOf('?');
+
+    if (params != -1)
+    {
+      filename = filename.substring(0, params);

[Felix Dahlke, 2014/11/11 09:15:37]

Why not just use url.getPath() instead of manually cutting off the query string?
According to the docs, url.getFile() is the same as url.getPath() +
url.getQuery().

[René Jeschke, 2014/11/11 11:55:55]

Right, done that.

+    }
+
+    return filename.toLowerCase(Locale.US).trim().endsWith(".txt");

[Felix Dahlke, 2014/11/11 09:15:37]

I think I might have thought of a better way to determine whether the request is
for a filter list: We can go through all active subscriptions and check their
URL. We'd have to ensure that the subscription is first added to the list of
known subscriptions before the request happens on first start.

That'd allow us to give this method name a bit more conviction, e.g.
"isSubscriptionUrl" or something.

[René Jeschke, 2014/11/11 11:55:55]

I think that this is a bit much of an overhead for a hack. The Android app does
not load anything else than filter lists or update JSONs, so a check for .txt
should really be Ok for now.

[Felix Dahlke, 2014/11/11 16:18:30]

It currently doesn't, but I don't think hard coding such assumptions here is a
good idea if we can easily avoid it. We might make requests for *.txt files that
aren't filter lists. And, more likely, we might make requests for filter lists
that don't match *.txt. I believe we should try to be more robust here, if we
can.

Going over all subscription URLs here should be really negligible overhead-wise
- we're only making about one request per day per filter list, slowing that one
down by maybe 10ms shouldn't be noticeable at all. On the other hand, we could
be 100% sure that it's a filter list request, no matter what.

+  }
 
   @Override
   public ServerResponse httpGET(final String urlStr, final List<HeaderEntry> headers)
@@ -45,6 +64,8 @@ public class AndroidWebRequest extends WebRequest
       final URL url = new URL(urlStr);
       Log.d(this.TAG, "Downloading from: " + url);
 
+      final boolean mightBeFilterList = mightBeFilterList(url);
+
       final HttpURLConnection connection = (HttpURLConnection) url.openConnection();
       connection.setRequestMethod("GET");
       connection.connect();
@@ -54,34 +75,24 @@ public class AndroidWebRequest extends WebRequest
 
       if (response.getResponseStatus() == 200)
       {
-        final InputStream in = connection.getInputStream();
-
-        final byte[] buffer = new byte[4096];
+        final BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream(), "UTF-8"));
+        final StringBuilder sb = new StringBuilder();
 
-        byte[] out = new byte[INITIAL_BUFFER_SIZE];
-
-        int pos = 0;
-        for (;;)
+        String line;
+        while ((line = reader.readLine()) != null)

[Felix Dahlke, 2014/11/11 09:15:37]

Using BufferedReader instead of System.arraycopy seems fine for all requests we
make here, but I think we shouldn't read anything but filter lists line-by-line.

[René Jeschke, 2014/11/11 11:55:55]

Ok, we'll read the rest char-by-char.

         {
-          final int read = in.read(buffer);
-          if (read < 0)
-          {
-            break;
-          }
-          if (pos + read > out.length)
+          // We're only appending non-element-hiding filters here.
+          // See: https://issues.adblockplus.org/ticket/303
+          if (!mightBeFilterList || line.indexOf('#') == -1)
           {
-            final byte[] old = out;
-            out = new byte[out.length + BUFFER_GROWTH_DELTA];
-            System.arraycopy(old, 0, out, 0, pos);
+            sb.append(line);
+            sb.append('\n');
           }
-          System.arraycopy(buffer, 0, out, pos, read);
-          pos += read;
         }
-
         connection.disconnect();
 
         response.setStatus(NsStatus.OK);
-        response.setResponse(new String(out, 0, pos, "utf-8"));
+        response.setResponse(sb.toString());
       }
       else
       {