Issue 303 - Don't load element hiding rules

src/org/adblockplus/android/AndroidWebRequest.java

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 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.ServerResponse.NsStatus;
 import org.adblockplus.libadblockplus.WebRequest;
-import org.adblockplus.libadblockplus.ServerResponse.NsStatus;
+import org.apache.commons.lang.StringUtils;
 
 import android.util.Log;
 
 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)
   {
     try
     {
       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();
 
       final ServerResponse response = new ServerResponse();
       response.setResponseStatus(connection.getResponseCode());
 
       if (response.getResponseStatus() == 200)
       {
-        final InputStream in = connection.getInputStream();
+        final BufferedReader reader = new BufferedReader(new InputStreamReader(connection.getInputStream(), "UTF-8"));
+        final StringBuilder sb = new StringBuilder();
 
-        final byte[] buffer = new byte[4096];
-
-        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)
+          // We're only appending non-element-hiding filters here.
+          // See: https://issues.adblockplus.org/ticket/303
+          if (!mightBeFilterList || line.indexOf('#') == -1)
           {
-            break;
+            sb.append(line);
+            sb.append('\n');
           }
-          if (pos + read > out.length)
-          {
-            final byte[] old = out;
-            out = new byte[out.length + BUFFER_GROWTH_DELTA];
-            System.arraycopy(old, 0, out, 0, pos);
-          }
-          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
       {
         response.setStatus(NsStatus.ERROR_FAILURE);
       }
       return response;
     }
     catch (final Throwable t)
     {
       throw new AdblockPlusException("WebRequest failed", t);
     }
   }
 }