You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ag...@apache.org on 2014/07/04 05:02:46 UTC

[5/8] android commit: CB-5988 Allow exec() only from file: or start-up URL's domain

CB-5988 Allow exec() only from file: or start-up URL's domain

Uses prompt() to validate the origin of the calling JS.
This change also simplifies the start-up logic by explicitly disabling
the bridge during page transitions and explictly enabling it when the
JS asks for the bridgeSecret.

We now wait to fire onNativeReady in JS until the bridge is initialized.
It is therefore safe to delete the queue-clear/new exec race condition
code that was in PluginManager.


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

Branch: refs/heads/4.0.x
Commit: aab47bd4532bfe8707d745638eb5695ac543c681
Parents: 445ddd8
Author: Andrew Grieve <ag...@chromium.org>
Authored: Thu Jul 3 21:58:35 2014 -0400
Committer: Andrew Grieve <ag...@chromium.org>
Committed: Thu Jul 3 22:06:09 2014 -0400

----------------------------------------------------------------------
 .../org/apache/cordova/CordovaChromeClient.java | 134 ++++++++++---------
 .../apache/cordova/CordovaWebViewClient.java    |   5 +-
 .../src/org/apache/cordova/ExposedJsApi.java    |  28 +++-
 .../apache/cordova/NativeToJsMessageQueue.java  |  63 ++++-----
 .../src/org/apache/cordova/PluginManager.java   |  44 ------
 5 files changed, 132 insertions(+), 142 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-android/blob/aab47bd4/framework/src/org/apache/cordova/CordovaChromeClient.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/CordovaChromeClient.java b/framework/src/org/apache/cordova/CordovaChromeClient.java
index 2edabf1..f2c3350 100755
--- a/framework/src/org/apache/cordova/CordovaChromeClient.java
+++ b/framework/src/org/apache/cordova/CordovaChromeClient.java
@@ -28,6 +28,7 @@ import android.app.AlertDialog;
 import android.content.DialogInterface;
 import android.content.Intent;
 import android.net.Uri;
+import android.util.Log;
 import android.view.Gravity;
 import android.view.KeyEvent;
 import android.view.View;
@@ -201,64 +202,75 @@ public class CordovaChromeClient extends WebChromeClient {
      * Since we are hacking prompts for our own purposes, we should not be using them for
      * this purpose, perhaps we should hack console.log to do this instead!
      *
-     * @param view
-     * @param url
-     * @param message
-     * @param defaultValue
-     * @param result
      * @see Other implementation in the Dialogs plugin.
      */
     @Override
-    public boolean onJsPrompt(WebView view, String url, String message, String defaultValue, JsPromptResult result) {
-
-        // Security check to make sure any requests are coming from the page initially
-        // loaded in webview and not another loaded in an iframe.
-        boolean reqOk = false;
-        if (url.startsWith("file://") || Config.isUrlWhiteListed(url)) {
-            reqOk = true;
-        }
-
-        // Calling PluginManager.exec() to call a native service using 
-        // prompt(this.stringify(args), "gap:"+this.stringify([service, action, callbackId, true]));
-        if (reqOk && defaultValue != null && defaultValue.length() > 3 && defaultValue.substring(0, 4).equals("gap:")) {
+    public boolean onJsPrompt(WebView view, String origin, String message, String defaultValue, JsPromptResult result) {
+        // Unlike the @JavascriptInterface bridge, this method is always called on the UI thread.
+        if (defaultValue != null && defaultValue.length() > 3 && defaultValue.startsWith("gap:")) {
             JSONArray array;
             try {
                 array = new JSONArray(defaultValue.substring(4));
-                String service = array.getString(0);
-                String action = array.getString(1);
-                String callbackId = array.getString(2);
-                String r = this.appView.exposedJsApi.exec(service, action, callbackId, message);
+                int bridgeSecret = array.getInt(0);
+                String service = array.getString(1);
+                String action = array.getString(2);
+                String callbackId = array.getString(3);
+                String r = appView.exposedJsApi.exec(bridgeSecret, service, action, callbackId, message);
                 result.confirm(r == null ? "" : r);
             } catch (JSONException e) {
                 e.printStackTrace();
-                return false;
+                result.cancel();
+            } catch (IllegalAccessException e) {
+                e.printStackTrace();
+                result.cancel();
             }
         }
 
         // Sets the native->JS bridge mode. 
-        else if (reqOk && defaultValue != null && defaultValue.equals("gap_bridge_mode:")) {
-        	try {
-                this.appView.exposedJsApi.setNativeToJsBridgeMode(Integer.parseInt(message));
-                result.confirm("");
-        	} catch (NumberFormatException e){
-                result.confirm("");
+        else if (defaultValue != null && defaultValue.startsWith("gap_bridge_mode:")) {
+            try {
+                int bridgeSecret = Integer.parseInt(defaultValue.substring(16));
+                appView.exposedJsApi.setNativeToJsBridgeMode(bridgeSecret, Integer.parseInt(message));
+                result.cancel();
+            } catch (NumberFormatException e){
                 e.printStackTrace();
-        	}
+                result.cancel();
+            } catch (IllegalAccessException e) {
+                e.printStackTrace();
+                result.cancel();
+            }
         }
 
         // Polling for JavaScript messages 
-        else if (reqOk && defaultValue != null && defaultValue.equals("gap_poll:")) {
-            String r = this.appView.exposedJsApi.retrieveJsMessages("1".equals(message));
-            result.confirm(r == null ? "" : r);
-        }
-
-        // Do NO-OP so older code doesn't display dialog
-        else if (defaultValue != null && defaultValue.equals("gap_init:")) {
-            result.confirm("OK");
+        else if (defaultValue != null && defaultValue.startsWith("gap_poll:")) {
+            int bridgeSecret = Integer.parseInt(defaultValue.substring(9));
+            try {
+                String r = appView.exposedJsApi.retrieveJsMessages(bridgeSecret, "1".equals(message));
+                result.confirm(r == null ? "" : r);
+            } catch (IllegalAccessException e) {
+                e.printStackTrace();
+                result.cancel();
+            }
         }
 
-        // Show dialog
-        else {
+        else if (defaultValue != null && defaultValue.startsWith("gap_init:")) {
+            String startUrl = Config.getStartUrl();
+            // Protect against random iframes being able to talk through the bridge.
+            // Trust only file URLs and the start URL's domain.
+            // The extra origin.startsWith("http") is to protect against iframes with data: having "" as origin.
+            if (origin.startsWith("file:") || (origin.startsWith("http") && startUrl.startsWith(origin))) {
+                // Enable the bridge
+                int bridgeMode = Integer.parseInt(defaultValue.substring(9));
+                appView.jsMessageQueue.setBridgeMode(bridgeMode);
+                // Tell JS the bridge secret.
+                int secret = appView.exposedJsApi.generateBridgeSecret();
+                result.confirm(""+secret);
+            } else {
+                Log.e(LOG_TAG, "gap_init called from restricted origin: " + origin);
+                result.cancel();
+            }
+        } else {
+            // Returning false would also show a dialog, but the default one shows the origin (ugly).
             final JsPromptResult res = result;
             AlertDialog.Builder dlg = new AlertDialog.Builder(this.cordova.getActivity());
             dlg.setMessage(message);
@@ -338,10 +350,10 @@ public class CordovaChromeClient extends WebChromeClient {
         this.appView.showCustomView(view, callback);
     }
 
-	@Override
-	public void onHideCustomView() {
-    	this.appView.hideCustomView();
-	}
+    @Override
+    public void onHideCustomView() {
+        this.appView.hideCustomView();
+    }
     
     @Override
     /**
@@ -351,24 +363,24 @@ public class CordovaChromeClient extends WebChromeClient {
      */
     public View getVideoLoadingProgressView() {
 
-	    if (mVideoProgressView == null) {	        
-	    	// Create a new Loading view programmatically.
-	    	
-	    	// create the linear layout
-	    	LinearLayout layout = new LinearLayout(this.appView.getContext());
-	        layout.setOrientation(LinearLayout.VERTICAL);
-	        RelativeLayout.LayoutParams layoutParams = new RelativeLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT);
-	        layoutParams.addRule(RelativeLayout.CENTER_IN_PARENT);
-	        layout.setLayoutParams(layoutParams);
-	        // the proress bar
-	        ProgressBar bar = new ProgressBar(this.appView.getContext());
-	        LinearLayout.LayoutParams barLayoutParams = new LinearLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT);
-	        barLayoutParams.gravity = Gravity.CENTER;
-	        bar.setLayoutParams(barLayoutParams);   
-	        layout.addView(bar);
-	        
-	        mVideoProgressView = layout;
-	    }
+        if (mVideoProgressView == null) {            
+            // Create a new Loading view programmatically.
+            
+            // create the linear layout
+            LinearLayout layout = new LinearLayout(this.appView.getContext());
+            layout.setOrientation(LinearLayout.VERTICAL);
+            RelativeLayout.LayoutParams layoutParams = new RelativeLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT);
+            layoutParams.addRule(RelativeLayout.CENTER_IN_PARENT);
+            layout.setLayoutParams(layoutParams);
+            // the proress bar
+            ProgressBar bar = new ProgressBar(this.appView.getContext());
+            LinearLayout.LayoutParams barLayoutParams = new LinearLayout.LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT);
+            barLayoutParams.gravity = Gravity.CENTER;
+            bar.setLayoutParams(barLayoutParams);   
+            layout.addView(bar);
+            
+            mVideoProgressView = layout;
+        }
     return mVideoProgressView; 
     }
     

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/aab47bd4/framework/src/org/apache/cordova/CordovaWebViewClient.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/CordovaWebViewClient.java b/framework/src/org/apache/cordova/CordovaWebViewClient.java
index 4a72fea..9e276d7 100755
--- a/framework/src/org/apache/cordova/CordovaWebViewClient.java
+++ b/framework/src/org/apache/cordova/CordovaWebViewClient.java
@@ -18,7 +18,6 @@
 */
 package org.apache.cordova;
 
-import java.io.ByteArrayInputStream;
 import java.util.Hashtable;
 
 import org.apache.cordova.CordovaInterface;
@@ -28,18 +27,15 @@ import org.json.JSONException;
 import org.json.JSONObject;
 
 import android.annotation.TargetApi;
-import android.content.Intent;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.graphics.Bitmap;
-import android.net.Uri;
 import android.net.http.SslError;
 import android.util.Log;
 import android.view.View;
 import android.webkit.HttpAuthHandler;
 import android.webkit.SslErrorHandler;
-import android.webkit.WebResourceResponse;
 import android.webkit.WebView;
 import android.webkit.WebViewClient;
 
@@ -170,6 +166,7 @@ public class CordovaWebViewClient extends WebViewClient {
         LOG.d(TAG, "onPageStarted(" + url + ")");
         // Flush stale messages.
         this.appView.jsMessageQueue.reset();
+        this.appView.exposedJsApi.clearBridgeSecret();
 
         // Broadcast message that page has loaded
         this.appView.postMessage("onPageStarted", url);

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/aab47bd4/framework/src/org/apache/cordova/ExposedJsApi.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/ExposedJsApi.java b/framework/src/org/apache/cordova/ExposedJsApi.java
index fde5722..97f6038 100755
--- a/framework/src/org/apache/cordova/ExposedJsApi.java
+++ b/framework/src/org/apache/cordova/ExposedJsApi.java
@@ -19,6 +19,7 @@
 package org.apache.cordova;
 
 import android.webkit.JavascriptInterface;
+
 import org.apache.cordova.PluginManager;
 import org.json.JSONException;
 
@@ -31,6 +32,7 @@ import org.json.JSONException;
     
     private PluginManager pluginManager;
     private NativeToJsMessageQueue jsMessageQueue;
+    private volatile int bridgeSecret = -1; // written by UI thread, read by JS thread.
     
     public ExposedJsApi(PluginManager pluginManager, NativeToJsMessageQueue jsMessageQueue) {
         this.pluginManager = pluginManager;
@@ -38,7 +40,8 @@ import org.json.JSONException;
     }
 
     @JavascriptInterface
-    public String exec(String service, String action, String callbackId, String arguments) throws JSONException {
+    public String exec(int bridgeSecret, String service, String action, String callbackId, String arguments) throws JSONException, IllegalAccessException {
+        verifySecret(bridgeSecret);
         // If the arguments weren't received, send a message back to JS.  It will switch bridge modes and try again.  See CB-2666.
         // We send a message meant specifically for this case.  It starts with "@" so no other message can be encoded into the same string.
         if (arguments == null) {
@@ -65,12 +68,31 @@ import org.json.JSONException;
     }
     
     @JavascriptInterface
-    public void setNativeToJsBridgeMode(int value) {
+    public void setNativeToJsBridgeMode(int bridgeSecret, int value) throws IllegalAccessException {
+        verifySecret(bridgeSecret);
         jsMessageQueue.setBridgeMode(value);
     }
     
     @JavascriptInterface
-    public String retrieveJsMessages(boolean fromOnlineEvent) {
+    public String retrieveJsMessages(int bridgeSecret, boolean fromOnlineEvent) throws IllegalAccessException {
+        verifySecret(bridgeSecret);
         return jsMessageQueue.popAndEncode(fromOnlineEvent);
     }
+
+    private void verifySecret(int value) throws IllegalAccessException {
+        if (bridgeSecret < 0 || value != bridgeSecret) {
+            throw new IllegalAccessException();
+        }
+    }
+
+    /** Called on page transitions */
+    void clearBridgeSecret() {
+        bridgeSecret = -1;
+    }
+
+    /** Called by cordova.js to initialize the bridge. */
+    int generateBridgeSecret() {
+        bridgeSecret = (int)(Math.random() * Integer.MAX_VALUE);
+        return bridgeSecret;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/aab47bd4/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
index 9f6f96e..063fc7e 100755
--- a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
+++ b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java
@@ -35,9 +35,6 @@ import android.webkit.WebView;
 public class NativeToJsMessageQueue {
     private static final String LOG_TAG = "JsMessageQueue";
 
-    // This must match the default value in cordova-js/lib/android/exec.js
-    private static final int DEFAULT_BRIDGE_MODE = 2;
-    
     // Set this to true to force plugin results to be encoding as
     // JS instead of the custom format (useful for benchmarking).
     private static final boolean FORCE_ENCODE_USING_EVAL = false;
@@ -49,18 +46,13 @@ public class NativeToJsMessageQueue {
     // Disable sending back native->JS messages during an exec() when the active
     // exec() is asynchronous. Set this to true when running bridge benchmarks.
     static final boolean DISABLE_EXEC_CHAINING = false;
-    
+
     // Arbitrarily chosen upper limit for how much data to send to JS in one shot.
     // This currently only chops up on message boundaries. It may be useful
     // to allow it to break up messages.
     private static int MAX_PAYLOAD_SIZE = 50 * 1024 * 10240;
     
     /**
-     * The index into registeredListeners to treat as active. 
-     */
-    private int activeListenerIndex;
-    
-    /**
      * When true, the active listener is not fired upon enqueue. When set to false,
      * the active listener will be fired if the queue is non-empty. 
      */
@@ -76,6 +68,13 @@ public class NativeToJsMessageQueue {
      */
     private final BridgeMode[] registeredListeners;    
     
+    /**
+     * When null, the bridge is disabled. This occurs during page transitions.
+     * When disabled, all callbacks are dropped since they are assumed to be
+     * relevant to the previous page.
+     */
+    private BridgeMode activeBridgeMode;
+
     private final CordovaInterface cordova;
     private final CordovaWebView webView;
 
@@ -94,17 +93,19 @@ public class NativeToJsMessageQueue {
      * Changes the bridge mode.
      */
     public void setBridgeMode(int value) {
-        if (value < 0 || value >= registeredListeners.length) {
+        if (value < -1 || value >= registeredListeners.length) {
             Log.d(LOG_TAG, "Invalid NativeToJsBridgeMode: " + value);
         } else {
-            if (value != activeListenerIndex) {
-                Log.d(LOG_TAG, "Set native->JS mode to " + value);
+            BridgeMode newMode = value < 0 ? null : registeredListeners[value];
+            if (newMode != activeBridgeMode) {
+                Log.d(LOG_TAG, "Set native->JS mode to " + (newMode == null ? "null" : newMode.getClass().getSimpleName()));
                 synchronized (this) {
-                    activeListenerIndex = value;
-                    BridgeMode activeListener = registeredListeners[value];
-                    activeListener.reset();
-                    if (!paused && !queue.isEmpty()) {
-                        activeListener.onNativeToJsMessageAvailable();
+                    activeBridgeMode = newMode;
+                    if (newMode != null) {
+                        newMode.reset();
+                        if (!paused && !queue.isEmpty()) {
+                            newMode.onNativeToJsMessageAvailable();
+                        }
                     }
                 }
             }
@@ -117,8 +118,7 @@ public class NativeToJsMessageQueue {
     public void reset() {
         synchronized (this) {
             queue.clear();
-            setBridgeMode(DEFAULT_BRIDGE_MODE);
-            registeredListeners[activeListenerIndex].reset();
+            setBridgeMode(-1);
         }
     }
 
@@ -142,7 +142,10 @@ public class NativeToJsMessageQueue {
      */
     public String popAndEncode(boolean fromOnlineEvent) {
         synchronized (this) {
-            registeredListeners[activeListenerIndex].notifyOfFlush(fromOnlineEvent);
+            if (activeBridgeMode == null) {
+                return null;
+            }
+            activeBridgeMode.notifyOfFlush(fromOnlineEvent);
             if (queue.isEmpty()) {
                 return null;
             }
@@ -247,16 +250,20 @@ public class NativeToJsMessageQueue {
 
         enqueueMessage(message);
     }
-    
+
     private void enqueueMessage(JsMessage message) {
         synchronized (this) {
+            if (activeBridgeMode == null) {
+                Log.d(LOG_TAG, "Dropping Native->JS message due to disabled bridge");
+                return;
+            }
             queue.add(message);
             if (!paused) {
-                registeredListeners[activeListenerIndex].onNativeToJsMessageAvailable();
+                activeBridgeMode.onNativeToJsMessageAvailable();
             }
-        }        
+        }
     }
-    
+
     public void setPaused(boolean value) {
         if (paused && value) {
             // This should never happen. If a use-case for it comes up, we should
@@ -266,16 +273,12 @@ public class NativeToJsMessageQueue {
         paused = value;
         if (!value) {
             synchronized (this) {
-                if (!queue.isEmpty()) {
-                    registeredListeners[activeListenerIndex].onNativeToJsMessageAvailable();
+                if (!queue.isEmpty() && activeBridgeMode != null) {
+                    activeBridgeMode.onNativeToJsMessageAvailable();
                 }
             }   
         }
     }
-    
-    public boolean getPaused() {
-        return paused;
-    }
 
     private abstract class BridgeMode {
         abstract void onNativeToJsMessageAvailable();

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/aab47bd4/framework/src/org/apache/cordova/PluginManager.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/PluginManager.java b/framework/src/org/apache/cordova/PluginManager.java
index 1ed0523..02536ba 100755
--- a/framework/src/org/apache/cordova/PluginManager.java
+++ b/framework/src/org/apache/cordova/PluginManager.java
@@ -23,9 +23,7 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
 
-import org.apache.cordova.CordovaArgs;
 import org.apache.cordova.CordovaWebView;
 import org.apache.cordova.CallbackContext;
 import org.apache.cordova.CordovaInterface;
@@ -65,8 +63,6 @@ public class PluginManager {
     // Using <url-filter> is deprecated.
     protected HashMap<String, List<String>> urlMap = new HashMap<String, List<String>>();
 
-    private AtomicInteger numPendingUiExecs;
-
     /**
      * Constructor.
      *
@@ -77,7 +73,6 @@ public class PluginManager {
         this.ctx = ctx;
         this.app = app;
         this.firstRun = true;
-        this.numPendingUiExecs = new AtomicInteger(0);
     }
 
     /**
@@ -99,9 +94,6 @@ public class PluginManager {
             this.clearPluginObjects();
         }
 
-        // Insert PluginManager service
-        this.addService(new PluginEntry("PluginManager", new PluginManagerService()));
-
         // Start up all plugins that have onload specified
         this.startupPlugins();
     }
@@ -216,20 +208,6 @@ public class PluginManager {
      *                      plugin execute method.
      */
     public void exec(final String service, final String action, final String callbackId, final String rawArgs) {
-        if (numPendingUiExecs.get() > 0) {
-            numPendingUiExecs.getAndIncrement();
-            this.ctx.getActivity().runOnUiThread(new Runnable() {
-                public void run() {
-                    execHelper(service, action, callbackId, rawArgs);
-                    numPendingUiExecs.getAndDecrement();
-                }
-            });
-        } else {
-            execHelper(service, action, callbackId, rawArgs);
-        }
-    }
-
-    private void execHelper(final String service, final String action, final String callbackId, final String rawArgs) {
         CordovaPlugin plugin = getPlugin(service);
         if (plugin == null) {
             Log.d(TAG, "exec() call to unknown plugin: " + service);
@@ -437,26 +415,4 @@ public class PluginManager {
         }
         return null;
     }
-
-    private class PluginManagerService extends CordovaPlugin {
-        @Override
-        public boolean execute(String action, CordovaArgs args, final CallbackContext callbackContext) throws JSONException {
-            if ("startup".equals(action)) {
-                // The onPageStarted event of CordovaWebViewClient resets the queue of messages to be returned to javascript in response
-                // to exec calls. Since this event occurs on the UI thread and exec calls happen on the WebCore thread it is possible
-                // that onPageStarted occurs after exec calls have started happening on a new page, which can cause the message queue
-                // to be reset between the queuing of a new message and its retrieval by javascript. To avoid this from happening,
-                // javascript always sends a "startup" exec upon loading a new page which causes all future exec calls to happen on the UI
-                // thread (and hence after onPageStarted) until there are no more pending exec calls remaining.
-                numPendingUiExecs.getAndIncrement();
-                ctx.getActivity().runOnUiThread(new Runnable() {
-                    public void run() {
-                        numPendingUiExecs.getAndDecrement();
-                    }
-                });
-                return true;
-            }
-            return false;
-        }
-    }
 }