You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ia...@apache.org on 2013/07/11 20:12:28 UTC

[2/2] android commit: [CB-4151] Extract whitelist from Config class for testability

[CB-4151] Extract whitelist from Config class for testability


Project: http://git-wip-us.apache.org/repos/asf/cordova-android/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-android/commit/3ae28b30
Tree: http://git-wip-us.apache.org/repos/asf/cordova-android/tree/3ae28b30
Diff: http://git-wip-us.apache.org/repos/asf/cordova-android/diff/3ae28b30

Branch: refs/heads/master
Commit: 3ae28b3085cbea91bd93105a54a9218012278924
Parents: b5df9dd
Author: Ian Clelland <ic...@chromium.org>
Authored: Thu Jul 11 14:01:23 2013 -0400
Committer: Ian Clelland <ic...@chromium.org>
Committed: Thu Jul 11 14:11:37 2013 -0400

----------------------------------------------------------------------
 framework/src/org/apache/cordova/Config.java    | 100 +---------------
 framework/src/org/apache/cordova/Whitelist.java | 119 +++++++++++++++++++
 2 files changed, 123 insertions(+), 96 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-android/blob/3ae28b30/framework/src/org/apache/cordova/Config.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/Config.java b/framework/src/org/apache/cordova/Config.java
index 1a522de..6e0c147 100644
--- a/framework/src/org/apache/cordova/Config.java
+++ b/framework/src/org/apache/cordova/Config.java
@@ -21,9 +21,6 @@ package org.apache.cordova;
 
 import java.io.IOException;
 
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.Iterator;
 import java.util.Locale;
 
 import java.util.regex.Matcher;
@@ -44,8 +41,7 @@ public class Config {
 
     public static final String TAG = "Config";
 
-    private ArrayList<Pattern> whiteList = new ArrayList<Pattern>();
-    private HashMap<String, Boolean> whiteListCache = new HashMap<String, Boolean>();
+    private Whitelist whitelist = new Whitelist();
     private String startUrl;
 
     private static Config self = null;
@@ -92,7 +88,7 @@ public class Config {
                     String origin = xml.getAttributeValue(null, "origin");
                     String subdomains = xml.getAttributeValue(null, "subdomains");
                     if (origin != null) {
-                        this._addWhiteListEntry(origin, (subdomains != null) && (subdomains.compareToIgnoreCase("true") == 0));
+                        whitelist.addWhiteListEntry(origin, (subdomains != null) && (subdomains.compareToIgnoreCase("true") == 0));
                     }
                 }
                 else if (strNode.equals("log")) {
@@ -200,79 +196,9 @@ public class Config {
         if (self == null) {
             return;
         }
-
-        self._addWhiteListEntry(origin, subdomains);
+        self.whitelist.addWhiteListEntry(origin, subdomains);
     }
 
-    /*
-     * Trying to figure out how to match * is a pain
-     * So, we don't use a regex here
-     */
-    
-    private boolean originHasWildcard(String origin){
-        //First, check for a protocol, then split it if it has one.
-        if(origin.contains("//"))
-        {
-            origin = origin.split("//")[1];
-        }
-        return origin.startsWith("*");
-    }
-
-    private void _addWhiteListEntry(String origin, boolean subdomains) {
-        try {
-            // Unlimited access to network resources
-            if (origin.compareTo("*") == 0) {
-                LOG.d(TAG, "Unlimited access to network resources");
-                this.whiteList.add(Pattern.compile(".*"));
-            }
-            else { // specific access
-                // check if subdomains should be included
-                if(originHasWildcard(origin))
-                {
-                    subdomains = true;
-                    //Remove the wildcard so this works properly
-                    origin = origin.replace("*.", "");
-                }
-                
-                // TODO: we should not add more domains if * has already been added
-                Pattern schemeRegex = Pattern.compile("^[a-z-]+://");
-                Matcher matcher = schemeRegex.matcher(origin);
-                if (subdomains) {
-                    // Check for http or https protocols
-                    if (origin.startsWith("http")) {
-                        this.whiteList.add(Pattern.compile(origin.replaceFirst("https?://", "^https?://(.*\\.)?")));
-                    }
-                    // Check for other protocols
-                    else if(matcher.find()){
-                        this.whiteList.add(Pattern.compile("^" + origin.replaceFirst("//", "//(.*\\.)?")));
-                    }
-                    // XXX making it stupid friendly for people who forget to include protocol/SSL
-                    else {
-                        this.whiteList.add(Pattern.compile("^https?://(.*\\.)?" + origin));
-                    }
-                    LOG.d(TAG, "Origin to allow with subdomains: %s", origin);
-                } else {
-                    // Check for http or https protocols
-                    if (origin.startsWith("http")) {
-                        this.whiteList.add(Pattern.compile(origin.replaceFirst("https?://", "^https?://")));
-                    }
-                    // Check for other protocols
-                    else if(matcher.find()){
-                        this.whiteList.add(Pattern.compile("^" + origin));
-                    }
-                    // XXX making it stupid friendly for people who forget to include protocol/SSL
-                    else {
-                        this.whiteList.add(Pattern.compile("^https?://" + origin));
-                    }
-                    LOG.d(TAG, "Origin to allow: %s", origin);
-                }
-            }
-        } catch (Exception e) {
-            LOG.d(TAG, "Failed to add origin %s", origin);
-        }
-    }
-
-
     /**
      * Determine if URL is in approved list of URLs to load.
      *
@@ -283,25 +209,7 @@ public class Config {
         if (self == null) {
             return false;
         }
-
-        // Check to see if we have matched url previously
-        if (self.whiteListCache.get(url) != null) {
-            return true;
-        }
-
-        // Look for match in white list
-        Iterator<Pattern> pit = self.whiteList.iterator();
-        while (pit.hasNext()) {
-            Pattern p = pit.next();
-            Matcher m = p.matcher(url);
-
-            // If match found, then cache it to speed up subsequent comparisons
-            if (m.find()) {
-                self.whiteListCache.put(url, true);
-                return true;
-            }
-        }
-        return false;
+        return self.whitelist.isUrlWhiteListed(url);
     }
 
     public static String getStartUrl() {

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/3ae28b30/framework/src/org/apache/cordova/Whitelist.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/Whitelist.java b/framework/src/org/apache/cordova/Whitelist.java
new file mode 100644
index 0000000..736e5a7
--- /dev/null
+++ b/framework/src/org/apache/cordova/Whitelist.java
@@ -0,0 +1,119 @@
+package org.apache.cordova;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.cordova.LOG;
+
+public class Whitelist {
+    private ArrayList<Pattern> whiteList;
+    private HashMap<String, Boolean> whiteListCache;
+
+    public static final String TAG = "Whitelist";
+
+	public Whitelist() {
+		this.whiteList = new ArrayList<Pattern>();
+		this.whiteListCache = new HashMap<String, Boolean>();
+	}
+
+    /*
+     * Trying to figure out how to match * is a pain
+     * So, we don't use a regex here
+     */
+    
+    private boolean originHasWildcard(String origin){
+        //First, check for a protocol, then split it if it has one.
+        if(origin.contains("//"))
+        {
+            origin = origin.split("//")[1];
+        }
+        return origin.startsWith("*");
+    }
+
+    public void addWhiteListEntry(String origin, boolean subdomains) {
+        try {
+            // Unlimited access to network resources
+            if (origin.compareTo("*") == 0) {
+                LOG.d(TAG, "Unlimited access to network resources");
+                whiteList.add(Pattern.compile(".*"));
+            }
+            else { // specific access
+                // check if subdomains should be included
+                if(originHasWildcard(origin))
+                {
+                    subdomains = true;
+                    //Remove the wildcard so this works properly
+                    origin = origin.replace("*.", "");
+                }
+                
+                // TODO: we should not add more domains if * has already been added
+                Pattern schemeRegex = Pattern.compile("^[a-z-]+://");
+                Matcher matcher = schemeRegex.matcher(origin);
+                if (subdomains) {
+                    // Check for http or https protocols
+                    if (origin.startsWith("http")) {
+                        whiteList.add(Pattern.compile(origin.replaceFirst("https?://", "^https?://(.*\\.)?")));
+                    }
+                    // Check for other protocols
+                    else if(matcher.find()){
+                        whiteList.add(Pattern.compile("^" + origin.replaceFirst("//", "//(.*\\.)?")));
+                    }
+                    // XXX making it stupid friendly for people who forget to include protocol/SSL
+                    else {
+                        whiteList.add(Pattern.compile("^https?://(.*\\.)?" + origin));
+                    }
+                    LOG.d(TAG, "Origin to allow with subdomains: %s", origin);
+                } else {
+                    // Check for http or https protocols
+                    if (origin.startsWith("http")) {
+                        whiteList.add(Pattern.compile(origin.replaceFirst("https?://", "^https?://")));
+                    }
+                    // Check for other protocols
+                    else if(matcher.find()){
+                        whiteList.add(Pattern.compile("^" + origin));
+                    }
+                    // XXX making it stupid friendly for people who forget to include protocol/SSL
+                    else {
+                        whiteList.add(Pattern.compile("^https?://" + origin));
+                    }
+                    LOG.d(TAG, "Origin to allow: %s", origin);
+                }
+            }
+        } catch (Exception e) {
+            LOG.d(TAG, "Failed to add origin %s", origin);
+        }
+    }
+
+
+    /**
+     * Determine if URL is in approved list of URLs to load.
+     *
+     * @param url
+     * @return
+     */
+    public boolean isUrlWhiteListed(String url) {
+
+        // Check to see if we have matched url previously
+        if (whiteListCache.get(url) != null) {
+            return true;
+        }
+
+        // Look for match in white list
+        Iterator<Pattern> pit = whiteList.iterator();
+        while (pit.hasNext()) {
+            Pattern p = pit.next();
+            Matcher m = p.matcher(url);
+
+            // If match found, then cache it to speed up subsequent comparisons
+            if (m.find()) {
+                whiteListCache.put(url, true);
+                return true;
+            }
+        }
+        return false;
+    }
+
+}