Issue 5254 - Improve URL verification when whitelisting domains

adblockplussbrowser/build.gradle

Index: adblockplussbrowser/build.gradle
===================================================================
--- a/adblockplussbrowser/build.gradle
+++ b/adblockplussbrowser/build.gradle
@@ -1,8 +1,9 @@
+

[anton, 2017/07/06 06:51:10]

this change (new line) is not required

 apply plugin: 'com.android.application'
 
 android {
     compileSdkVersion rootProject.ext.compileSdkVersion
     buildToolsVersion rootProject.ext.buildToolsVersion
 
     compileOptions {
         sourceCompatibility = rootProject.ext.sourceCompatibilityVersion
@@ -23,19 +24,22 @@ android {
             java.srcDirs = ['src']
             res.srcDirs = ['res']
         }
     }
 
     buildTypes {
         release {
             minifyEnabled true
-            proguardFiles getDefaultProguardFile('proguard-android.txt')
+            proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'
             lintOptions {
                 disable 'MissingTranslation'
             }
         }
     }
 }
 
 dependencies {
     compile "com.android.support:support-v4:$rootProject.ext.androidSupportLibraryVersion"
+    compile (group: 'commons-validator', name: 'commons-validator', version: '1.6') {

[anton, 2017/07/06 06:51:10]

should we really use the whole Apache commons module for just URL validation?
In libadblockplus-android we originally copied Apache Strings right into app
source code and then reimplemented it with our own code.

Can't we really implement (or copy the code which is not desired but possible)
this on our own?
I'd prefer to cover it by tests too.

[jens, 2017/07/06 08:49:06]

I think it's okay to use commons validator, as the list of valid TLDs is updated
regularly. We just need to check if there is a new version available or
alternatively use "version: '+'" to always use the lastest release. Even if the
latter one is not ideal, it shouldn't be a problem if we cover the validity
check by tests. With proguard activated the APK is just slightly bigger than
before: 1.666.715 Byte vs. 1.709.685 Byte

Copying the code and keeping the TLDs up to date would be a bigger effort than
to just use the lib. In view of the fact that the apk is only slightly bigger,
this solution would be fine for me. But of course we can talk about this with
the others.

[diegocarloslima, 2017/07/06 18:25:16]

IHMO, for this project, we don't have a high size requirement as we do have in
libadblockplus-android, so the size increase in this case sounds fine to me. But
also, a extremely high quality TLD validation is currently just a 'nice to have'
feature, since this is currently just used when the user whitelist a website. So
I'm quite fine with either keeping the dependency or extracting the relevant
piece of code to our project.

[anton, 2017/07/07 13:03:19]

okay, let's just revert adding new empty lines and then it LGTM

+        exclude group: 'commons-logging', module: 'commons-logging'
+    }
 }