You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by agrieve <gi...@git.apache.org> on 2014/10/29 15:18:10 UTC

[GitHub] cordova-android pull request: Unplug whitelist

GitHub user agrieve opened a pull request:

    https://github.com/apache/cordova-android/pull/132

    Unplug whitelist

    Creating for code review.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/cordova-android unplug-whitelist

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-android/pull/132.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #132
    
----
commit b56c12ff4ce8c904738f754138dc65f519bdf3bc
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-22T19:52:09Z

    Add hooks in CordovaPlugin and PluginManager for whitelist plugins
    
    This adds three hooks to CordovaPlugin objects. In each case, a null
    value can be returned to indicate "I don't care". This null value is
    the default.
    
        public Boolean shouldAllowRequest(String url)
        public Boolean shouldAllowNavigation(String url)
        public Boolean shouldOpenExternalUrl(String url)

commit 972540f4434b0b72f7d062e12652e9b1ac31c09b
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-22T20:16:52Z

    Defer whitelist decisions to plugins
    
    There is a default policy, which is implemented in the case where no plugins override any of the whitelist methods:
     * Error URLs must start with file://
     * Navigation is allowed to file:// and data: URLs which do not contain "app_webview"
     * External URLs do not launch intents
     * XHRs are allowed to file:// and data: URLs which do not contain "app_webview"

commit 19563c6c813ccacb1c60793b5a0c944e7c713556
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-22T20:26:41Z

    Refactor ConfigXmlParser to allow subclasses

commit 9293484dd33a2d44f3aa37822481c25eb9a2595e
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-09T18:44:09Z

    Remove unused Config methods (Breaking Change)

commit a8e67d4ab43bf72ec5092be2d395edfb5065f0c5
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-09T19:28:29Z

    Remove whitelists from WebView classes

commit 7ff71731549709740282e28d5a1027ad77cbee01
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-22T20:27:28Z

    Remove whitelist config.xml parsing

commit c439565d1bd705e65e8fdcc36383b54835e4d42e
Author: Ian Clelland <ic...@chromium.org>
Date:   2014-10-28T15:00:49Z

    Fix reversed condition for isUrlHarmful

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19541119
  
    --- Diff: framework/src/org/apache/cordova/PluginManager.java ---
    @@ -254,6 +254,110 @@ public void onNewIntent(Intent intent) {
         }
     
         /**
    +     * Called when the webview is going to request an external resource.
    +     *
    +     * This delegates to the installed plugins, which must all return true for
    +     * this method to return true.
    +     *
    +     * @param url       The URL that is being requested.
    +     * @return          Tri-State:
    +     *                    null: All plugins returned null (the default). This
    +     *                          indicates that the default policy should be
    +     *                          followed.
    +     *                    true: All plugins returned true (allow the resource
    --- End diff --
    
    Why not just return the first non-null value?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540154
  
    --- Diff: framework/src/org/apache/cordova/AndroidWebView.java ---
    @@ -429,7 +426,7 @@ public void showWebPage(String url, boolean openExternal, boolean clearHistory,
             if (!openExternal) {
     
                 // Make sure url is in whitelist
    -            if (url.startsWith("file://") || internalWhitelist.isUrlWhiteListed(url)) {
    +            if (url.startsWith("file://") || pluginManager.shouldAllowNavigation(url)) {
    --- End diff --
    
    ditto - also - should we check for null here? also also - make sense to make a helper for combining default logic with pluginManager result?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540282
  
    --- Diff: framework/src/org/apache/cordova/CordovaPlugin.java ---
    @@ -162,19 +162,53 @@ public Object onMessage(String id, Object data) {
          * Called when an activity you launched exits, giving you the requestCode you started it with,
          * the resultCode it returned, and any additional data from it.
          *
    -     * @param requestCode		The request code originally supplied to startActivityForResult(),
    -     * 							allowing you to identify who this result came from.
    -     * @param resultCode		The integer result code returned by the child activity through its setResult().
    -     * @param intent				An Intent, which can return result data to the caller (various data can be attached to Intent "extras").
    +     * @param requestCode   The request code originally supplied to startActivityForResult(),
    +     *                      allowing you to identify who this result came from.
    +     * @param resultCode    The integer result code returned by the child activity through its setResult().
    +     * @param intent        An Intent, which can return result data to the caller (various data can be
    +     *                      attached to Intent "extras").
          */
         public void onActivityResult(int requestCode, int resultCode, Intent intent) {
         }
     
         /**
    +     * Hook for blocking the loading of external resources.
    +     *
    +     * This will be called when the WebView's shouldInterceptRequest wants to know whether to
    +     * open a connection to an external resource. Return false to block the request. Only if
    +     * all plugins return true, then the request will proceed.
    +     */
    +    public Boolean shouldAllowRequest(String url) {
    +        return true;
    --- End diff --
    
    this is tri-state, so returning null should be the default, no?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19553788
  
    --- Diff: framework/src/org/apache/cordova/AndroidWebView.java ---
    @@ -354,7 +349,9 @@ private void loadUrlNow(String url) {
             if (LOG.isLoggable(LOG.DEBUG) && !url.startsWith("javascript:")) {
                 LOG.d(TAG, ">>> loadUrlNow()");
             }
    -        if (url.startsWith("file://") || url.startsWith("javascript:") || internalWhitelist.isUrlWhiteListed(url)) {
    +        Boolean shouldAllowNavigation = pluginManager.shouldAllowNavigation(url);
    +        if (url.startsWith("file://") || url.startsWith("javascript:") ||
    --- End diff --
    
    The only question I see here is what to do when the plugins return `false`.
    On `true`: Allow the page to load
    On `null`: Allow `file` and `javascript` pages to load
    On `false`: ?
    
    I can disallow `file` urls in that case (which is probably a good idea; otherwise, how do you ever block them?) But what about `javascript:`? Should we always allow that? Is it part of the bridge mechanism that could never ever be dropped?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540735
  
    --- Diff: framework/src/org/apache/cordova/AndroidWebView.java ---
    @@ -429,7 +426,7 @@ public void showWebPage(String url, boolean openExternal, boolean clearHistory,
             if (!openExternal) {
     
                 // Make sure url is in whitelist
    -            if (url.startsWith("file://") || internalWhitelist.isUrlWhiteListed(url)) {
    +            if (url.startsWith("file://") || pluginManager.shouldAllowNavigation(url)) {
    --- End diff --
    
    Shoudl definitely check for null there -- to avoid NPE at the very least


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19541080
  
    --- Diff: framework/src/org/apache/cordova/CordovaPlugin.java ---
    @@ -162,19 +162,53 @@ public Object onMessage(String id, Object data) {
          * Called when an activity you launched exits, giving you the requestCode you started it with,
          * the resultCode it returned, and any additional data from it.
          *
    -     * @param requestCode		The request code originally supplied to startActivityForResult(),
    -     * 							allowing you to identify who this result came from.
    -     * @param resultCode		The integer result code returned by the child activity through its setResult().
    -     * @param intent				An Intent, which can return result data to the caller (various data can be attached to Intent "extras").
    +     * @param requestCode   The request code originally supplied to startActivityForResult(),
    +     *                      allowing you to identify who this result came from.
    +     * @param resultCode    The integer result code returned by the child activity through its setResult().
    +     * @param intent        An Intent, which can return result data to the caller (various data can be
    +     *                      attached to Intent "extras").
          */
         public void onActivityResult(int requestCode, int resultCode, Intent intent) {
         }
     
         /**
    +     * Hook for blocking the loading of external resources.
    +     *
    +     * This will be called when the WebView's shouldInterceptRequest wants to know whether to
    +     * open a connection to an external resource. Return false to block the request. Only if
    +     * all plugins return true, then the request will proceed.
    --- End diff --
    
    Were there html tags in that comment? I'm thinking `<audio>`/`<video>`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540508
  
    --- Diff: framework/src/org/apache/cordova/AndroidWebView.java ---
    @@ -429,7 +426,7 @@ public void showWebPage(String url, boolean openExternal, boolean clearHistory,
             if (!openExternal) {
     
                 // Make sure url is in whitelist
    -            if (url.startsWith("file://") || internalWhitelist.isUrlWhiteListed(url)) {
    +            if (url.startsWith("file://") || pluginManager.shouldAllowNavigation(url)) {
    --- End diff --
    
    Ah - I see there's a helper in CordovaUriHelper - can that be used?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540420
  
    --- Diff: framework/src/org/apache/cordova/CordovaPlugin.java ---
    @@ -162,19 +162,53 @@ public Object onMessage(String id, Object data) {
          * Called when an activity you launched exits, giving you the requestCode you started it with,
          * the resultCode it returned, and any additional data from it.
          *
    -     * @param requestCode		The request code originally supplied to startActivityForResult(),
    -     * 							allowing you to identify who this result came from.
    -     * @param resultCode		The integer result code returned by the child activity through its setResult().
    -     * @param intent				An Intent, which can return result data to the caller (various data can be attached to Intent "extras").
    +     * @param requestCode   The request code originally supplied to startActivityForResult(),
    +     *                      allowing you to identify who this result came from.
    +     * @param resultCode    The integer result code returned by the child activity through its setResult().
    +     * @param intent        An Intent, which can return result data to the caller (various data can be
    +     *                      attached to Intent "extras").
          */
         public void onActivityResult(int requestCode, int resultCode, Intent intent) {
         }
     
         /**
    +     * Hook for blocking the loading of external resources.
    +     *
    +     * This will be called when the WebView's shouldInterceptRequest wants to know whether to
    +     * open a connection to an external resource. Return false to block the request. Only if
    +     * all plugins return true, then the request will proceed.
    --- End diff --
    
    Add the caveat here that WebSocket and <audio> / <video> cannot be blocked.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve closed the pull request at:

    https://github.com/apache/cordova-android/pull/132


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540972
  
    --- Diff: framework/src/org/apache/cordova/CordovaPlugin.java ---
    @@ -162,19 +162,53 @@ public Object onMessage(String id, Object data) {
          * Called when an activity you launched exits, giving you the requestCode you started it with,
          * the resultCode it returned, and any additional data from it.
          *
    -     * @param requestCode		The request code originally supplied to startActivityForResult(),
    -     * 							allowing you to identify who this result came from.
    -     * @param resultCode		The integer result code returned by the child activity through its setResult().
    -     * @param intent				An Intent, which can return result data to the caller (various data can be attached to Intent "extras").
    +     * @param requestCode   The request code originally supplied to startActivityForResult(),
    +     *                      allowing you to identify who this result came from.
    +     * @param resultCode    The integer result code returned by the child activity through its setResult().
    +     * @param intent        An Intent, which can return result data to the caller (various data can be
    +     *                      attached to Intent "extras").
          */
         public void onActivityResult(int requestCode, int resultCode, Intent intent) {
         }
     
         /**
    +     * Hook for blocking the loading of external resources.
    +     *
    +     * This will be called when the WebView's shouldInterceptRequest wants to know whether to
    +     * open a connection to an external resource. Return false to block the request. Only if
    +     * all plugins return true, then the request will proceed.
    +     */
    +    public Boolean shouldAllowRequest(String url) {
    +        return true;
    --- End diff --
    
    Aye; I missed it when I changed that one from a *b* oolean.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by clelland <gi...@git.apache.org>.
Github user clelland commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19542061
  
    --- Diff: framework/src/org/apache/cordova/PluginManager.java ---
    @@ -254,6 +254,110 @@ public void onNewIntent(Intent intent) {
         }
     
         /**
    +     * Called when the webview is going to request an external resource.
    +     *
    +     * This delegates to the installed plugins, which must all return true for
    +     * this method to return true.
    +     *
    +     * @param url       The URL that is being requested.
    +     * @return          Tri-State:
    +     *                    null: All plugins returned null (the default). This
    +     *                          indicates that the default policy should be
    +     *                          followed.
    +     *                    true: All plugins returned true (allow the resource
    --- End diff --
    
    Because it's still explicitly a whitelist mechanism, for security -- if two plugins could disagree, then the one which returns 'false' wins.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540066
  
    --- Diff: framework/src/org/apache/cordova/AndroidWebView.java ---
    @@ -354,7 +349,9 @@ private void loadUrlNow(String url) {
             if (LOG.isLoggable(LOG.DEBUG) && !url.startsWith("javascript:")) {
                 LOG.d(TAG, ">>> loadUrlNow()");
             }
    -        if (url.startsWith("file://") || url.startsWith("javascript:") || internalWhitelist.isUrlWhiteListed(url)) {
    +        Boolean shouldAllowNavigation = pluginManager.shouldAllowNavigation(url);
    +        if (url.startsWith("file://") || url.startsWith("javascript:") ||
    --- End diff --
    
    Since shouldAllowNavigtion is tri-state, I think we should do the default behaviour only when non-null


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540574
  
    --- Diff: framework/src/org/apache/cordova/CordovaUriHelper.java ---
    @@ -37,11 +38,54 @@ public CordovaUriHelper(CordovaInterface cdv, CordovaWebView webView)
             appView = webView;
             cordova = cdv;
         }
    -    
    +
    +    /**
    +     * Determine whether the webview should be allowed to navigate to a given URL.
    +     *
    +     * This method implements the default whitelist policy when no plugins override
    +     * shouldAllowNavigation
    +     */
    +    public boolean shouldAllowNavigation(String url) {
    +        Boolean pluginManagerAllowsNavigation = this.appView.getPluginManager().shouldAllowNavigation(url);
    +        if (pluginManagerAllowsNavigation == null) {
    +            // Default policy:
    +            // Internal urls on file:// or data:// that do not contain "app_webview" are allowed for navigation
    +            if(url.startsWith("file://") || url.startsWith("data:"))
    +            {
    +                //This directory on WebKit/Blink based webviews contains SQLite databases!
    +                //DON'T CHANGE THIS UNLESS YOU KNOW WHAT YOU'RE DOING!
    +                return !url.contains("app_webview");
    --- End diff --
    
    We should change this to at least be: "/app_webview/"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19541046
  
    --- Diff: framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java ---
    @@ -72,8 +72,14 @@ public WebResourceResponse shouldInterceptRequest(WebView view, String url) {
         }
     
         private boolean isUrlHarmful(String url) {
    --- End diff --
    
    Move this function to CordovaUriHelpers. Suggestion: CordovaUriHelpers."shouldAllowRequest"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540361
  
    --- Diff: framework/src/org/apache/cordova/CordovaPlugin.java ---
    @@ -162,19 +162,53 @@ public Object onMessage(String id, Object data) {
          * Called when an activity you launched exits, giving you the requestCode you started it with,
          * the resultCode it returned, and any additional data from it.
          *
    -     * @param requestCode		The request code originally supplied to startActivityForResult(),
    -     * 							allowing you to identify who this result came from.
    -     * @param resultCode		The integer result code returned by the child activity through its setResult().
    -     * @param intent				An Intent, which can return result data to the caller (various data can be attached to Intent "extras").
    +     * @param requestCode   The request code originally supplied to startActivityForResult(),
    +     *                      allowing you to identify who this result came from.
    +     * @param resultCode    The integer result code returned by the child activity through its setResult().
    +     * @param intent        An Intent, which can return result data to the caller (various data can be
    +     *                      attached to Intent "extras").
          */
         public void onActivityResult(int requestCode, int resultCode, Intent intent) {
         }
     
         /**
    +     * Hook for blocking the loading of external resources.
    +     *
    +     * This will be called when the WebView's shouldInterceptRequest wants to know whether to
    +     * open a connection to an external resource. Return false to block the request. Only if
    +     * all plugins return true, then the request will proceed.
    +     */
    +    public Boolean shouldAllowRequest(String url) {
    +        return true;
    +    }
    +
    +    /**
    +     * Hook for blocking navigation by the Cordova WebView
    +     *
    +     * This will be called when the WebView's needs to know whether to navigate to a new page.
    +     * Return false to block the navigation. Only if all plugins return true, then the navigation
    +     * will proceed.
    +     */
    +    public Boolean shouldAllowNavigation(String url) {
    +        return null;
    +    }
    +
    +    /**
    +     * Hook for blocking the launching of Intents by the Cordova application
    +     *
    +     * This will be called when the WebView will not navigate to a page, but could launch an intent
    +     * to handle the URL. Return false to block the navigation. Only if all plugins return true,
    --- End diff --
    
    Don't think this description is right - plugins can return null and not effect it right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19541221
  
    --- Diff: framework/src/org/apache/cordova/CordovaUriHelper.java ---
    @@ -37,11 +38,54 @@ public CordovaUriHelper(CordovaInterface cdv, CordovaWebView webView)
             appView = webView;
             cordova = cdv;
         }
    -    
    +
    +    /**
    +     * Determine whether the webview should be allowed to navigate to a given URL.
    +     *
    +     * This method implements the default whitelist policy when no plugins override
    +     * shouldAllowNavigation
    +     */
    +    public boolean shouldAllowNavigation(String url) {
    --- End diff --
    
    Should call this from AndroidBridge as well I think.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-android pull request: Unplug whitelist

Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:

    https://github.com/apache/cordova-android/pull/132#discussion_r19540914
  
    --- Diff: framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java ---
    @@ -72,8 +72,14 @@ public WebResourceResponse shouldInterceptRequest(WebView view, String url) {
         }
     
         private boolean isUrlHarmful(String url) {
    -        return ((url.startsWith("http:") || url.startsWith("https:")) && !appView.getWhitelist().isUrlWhiteListed(url))
    -            || url.contains("app_webview");
    +        Boolean shouldAllowRequest = appView.getPluginManager().shouldAllowRequest(url);
    +        if (shouldAllowRequest == null || shouldAllowRequest == true) {
    +            // This is the behaviour when all plugins return true, and is also the default behaviour
    +            // if no plugins override shouldAllowRequest()
    +            return ((url.startsWith("file://") || url.startsWith("data:")) && url.contains("app_webview"));
    --- End diff --
    
    same nit about app_webview


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org