You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by mbillau <gi...@git.apache.org> on 2014/03/19 19:44:07 UTC

[GitHub] cordova-android pull request: Allow subdomains flag in Whitelist x...

GitHub user mbillau opened a pull request:

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

    Allow subdomains flag in Whitelist xml for backcompat

    With the new Whitelist implementation, the `subdomains="true"` flag for the whitelist is being ignored even though the attribute is still parsed from XML and passed around our code. I think that we might as well allow this flag for backwards compatibility. 
    https://issues.apache.org/jira/browse/CB-4096

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

    $ git pull https://github.com/mbillau/cordova-android CB-4096

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

    https://github.com/apache/cordova-android/pull/95.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 #95
    
----
commit b95d729312ef00f59c75e43d73a73818cadd8761
Author: mbillau <mi...@gmail.com>
Date:   2014-03-19T18:21:08Z

    Allow subdomains flag in Whitelist xml for backcompat

----


---
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.
---

[GitHub] cordova-android pull request: Allow subdomains flag in Whitelist x...

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/95#discussion_r10768516
  
    --- Diff: framework/src/org/apache/cordova/Whitelist.java ---
    @@ -136,6 +136,14 @@ public void addWhiteListEntry(String origin, boolean subdomains) {
                             } else {
                                 whiteList.add(new URLPattern(scheme, host, port, path));
                             }
    +
    +                        if( subdomains ){
    --- End diff --
    
    This should probably also check `!host.startsWith("*.")`, to avoid accidentally generating patterns like `*.*.apache.org`


---
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.
---

[GitHub] cordova-android pull request: Allow subdomains flag in Whitelist x...

Posted by mbillau <gi...@git.apache.org>.
Github user mbillau commented on the pull request:

    https://github.com/apache/cordova-android/pull/95#issuecomment-38181676
  
    Going to close this as we decided to axe the subdomain attribute, but thanks for the comments. I agree with them.


---
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.
---

[GitHub] cordova-android pull request: Allow subdomains flag in Whitelist x...

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

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


---
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.
---

[GitHub] cordova-android pull request: Allow subdomains flag in Whitelist x...

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/95#discussion_r10768518
  
    --- Diff: framework/src/org/apache/cordova/Whitelist.java ---
    @@ -136,6 +136,14 @@ public void addWhiteListEntry(String origin, boolean subdomains) {
                             } else {
                                 whiteList.add(new URLPattern(scheme, host, port, path));
                             }
    +
    +                        if( subdomains ){
    +                            // Support the `subdomains="true"` XML attribute in the whitelist
    +                            if( scheme == null) scheme = "";
    +                            String sub = scheme + "*." + host;
    +                            addWhiteListEntry(sub, false);
    --- End diff --
    
    Why the recursive call here? I think this drops the port (which might be important) and path (which probably isn't, but you never know)
    
    Couldn't this just call whiteList.add with a modified host?


---
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.
---