Re: Android Plugin API
We won't be breaking plugins. I don't want to move to this for the reasons stated before. This code also is way less maintainable, and looks like it could break easily. I really think that clarity should win out here over an abuse of java reflection. On 10 Jul,2014, at 17:15 , Anis KADRI wrote: > I want to say that this should be default...but we won't change the API > again and won't break plugins again. Both could be supported at the same time, just by using a different base class
Re: Android Plugin API
On 10 Jul,2014, at 17:15 , Anis KADRI wrote: > I want to say that this should be default...but we won't change the API > again and won't break plugins again. Both could be supported at the same time, just by using a different base class
Re: Android Plugin API
I want to say that this should be default...but we won't change the API
again and won't break plugins again.
On Thu, Jul 10, 2014 at 7:48 AM, Erik Jan de Wit wrote:
> Hi all,
>
> Created an plugin that does this:
> https://github.com/edewit/aerogear-reflect-cordova the repository also
> includes a example plugin that uses this. Also have a blog describing this
> a bit more
> http://blog.nerdin.ch/2014/07/improved-cordova-android-plugin-api.html
>
> Cheers,
> Erik Jan
>
> On 2 Jun,2014, at 4:20 , Terence M. Bandoian wrote:
>
> > It does get complicated in a hurry. Using the technique in the Google
> Maps plugin:
> >
> > - getDeclaredMethod is documented to NOT return inherited methods which
> limits exposure
> > - the signature passed to getDeclaredMethod also limits exposure
> > - annotations, as Andrew suggested, could further limit exposure
> >
> > - getDeclaredMethod is documented to return private methods which for me
> raises questions
> > - setAccessible(true) suppresses normal access checking which also
> raises questions
> >
> > - invoke throws InvocationTargetException so exceptions thrown by the
> invoked method could be publicized
> > - invoke applies overriding which is probably a wash
> > - invoke returns an object which would have to be handled correctly
> >
> > Quite a bit to consider.
> >
> > -Terence Bandoian
> >
> >
> > On 5/29/2014 11:44 PM, Joe Bowser wrote:
> >> On Thu, May 29, 2014 at 9:26 PM, Terence M. Bandoian
> wrote:
> >>> Please correct me if I'm wrong but, as I understand it, the
> vulnerability
> >>> stems from injecting a Java object into the WebView which, in API
> levels 16
> >>> and below, exposed all of the public methods of the object (small 'o')
> >>> including the methods inherited from the Object class.
> >>>
> >> Yes, you are correct in this case. You can basically use the object
> >> methods to reflect into whatever you want. Once you have a single
> >> class exposed in this manner, you can then reflect into it, and
> >> basically you're done. The reason we don't use reflection is that
> >> it's very easy to reflect into things you're not supposed to be in.
> >> IMO, It wouldn't be hard to do this if we exposed plugins in the same
> >> way. Right now this is an Android Vulnerability, but if we start
> >> using reflection for plugins, we could very quickly end up making a
> >> similar Cordova exploit if we're not careful.
> >>
> >> This is also why we use the prompt bridge on API levels below 17.
> >>
> >>>
> >>>
> >>> On 5/28/2014 9:54 AM, Joe Bowser wrote:
> In case anyone is curious, here's why we minimize reflection:
>
>
>
> https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
>
> On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve
> wrote:
> > Another reasonable approach would be to use a Map,
> but
> > that can be implemented on top of what is currently exposed. I'm
> quite
> > wary
> > of Reflection as well.
> >
> >
> > On Wed, May 28, 2014 at 10:06 AM, Joe Bowser
> wrote:
> >
> >> The execute command exists for security reasons. We don't want any
> >> methods other than execute exposed to Javascript. I also prefer
> this
> >> approach because it is less prone to less catastrophic bugs than
> using
> >> Java reflection. We try and only use reflection when we have to.
> >>
> >> On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit >
> >> wrote:
> >>> Hi,
> >>>
> >>> When one is writing a plugin for android ATM the api that you have
> to
> >> implement has a execute method that has the action as a string:
> >>> @Override
> >>> public boolean execute(String action, JSONArray args,
> >> CallbackContext callbackContext) throws JSONException {
> >>> if ("beep".equals(action)) {
> >>> this.beep(args.getLong(0));
> >>> callbackContext.success();
> >>> return true;
> >>> }
> >>> return false; // Returning false results in a
> >>> "MethodNotFound"
> >> error.
> >>> }
> >>> When you have multiple actions this method gets very long, if you
> >> compare this with iOS here you don’t need a method like this you
> could
> >> ‘just’ implement the method directly:
> >>> - (void)beep:(CDVInvokedUrlCommand*)command
> >>> {
> >>> CDVPluginResult* pluginResult =;
> >>> NSString* myarg =mmand.arguments objectAtIndex:0];
> >>>
> >>> if (myarg !=) {
> >>> pluginResult �VPluginResult
> >> resultWithStatus:CDVCommandStatus_OK];
> >>> } else {
> >>> pluginResult �VPluginResult
> >> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was
> >> null"];
> >>> }
> >>> [self.commandDelegate sendPluginResult:pluginResult
> >> callbackId
Re: Android Plugin API
Hi all,
Created an plugin that does this:
https://github.com/edewit/aerogear-reflect-cordova the repository also includes
a example plugin that uses this. Also have a blog describing this a bit more
http://blog.nerdin.ch/2014/07/improved-cordova-android-plugin-api.html
Cheers,
Erik Jan
On 2 Jun,2014, at 4:20 , Terence M. Bandoian wrote:
> It does get complicated in a hurry. Using the technique in the Google Maps
> plugin:
>
> - getDeclaredMethod is documented to NOT return inherited methods which
> limits exposure
> - the signature passed to getDeclaredMethod also limits exposure
> - annotations, as Andrew suggested, could further limit exposure
>
> - getDeclaredMethod is documented to return private methods which for me
> raises questions
> - setAccessible(true) suppresses normal access checking which also raises
> questions
>
> - invoke throws InvocationTargetException so exceptions thrown by the invoked
> method could be publicized
> - invoke applies overriding which is probably a wash
> - invoke returns an object which would have to be handled correctly
>
> Quite a bit to consider.
>
> -Terence Bandoian
>
>
> On 5/29/2014 11:44 PM, Joe Bowser wrote:
>> On Thu, May 29, 2014 at 9:26 PM, Terence M. Bandoian
>> wrote:
>>> Please correct me if I'm wrong but, as I understand it, the vulnerability
>>> stems from injecting a Java object into the WebView which, in API levels 16
>>> and below, exposed all of the public methods of the object (small 'o')
>>> including the methods inherited from the Object class.
>>>
>> Yes, you are correct in this case. You can basically use the object
>> methods to reflect into whatever you want. Once you have a single
>> class exposed in this manner, you can then reflect into it, and
>> basically you're done. The reason we don't use reflection is that
>> it's very easy to reflect into things you're not supposed to be in.
>> IMO, It wouldn't be hard to do this if we exposed plugins in the same
>> way. Right now this is an Android Vulnerability, but if we start
>> using reflection for plugins, we could very quickly end up making a
>> similar Cordova exploit if we're not careful.
>>
>> This is also why we use the prompt bridge on API levels below 17.
>>
>>>
>>>
>>> On 5/28/2014 9:54 AM, Joe Bowser wrote:
In case anyone is curious, here's why we minimize reflection:
https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve
wrote:
> Another reasonable approach would be to use a Map, but
> that can be implemented on top of what is currently exposed. I'm quite
> wary
> of Reflection as well.
>
>
> On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
>
>> The execute command exists for security reasons. We don't want any
>> methods other than execute exposed to Javascript. I also prefer this
>> approach because it is less prone to less catastrophic bugs than using
>> Java reflection. We try and only use reflection when we have to.
>>
>> On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
>> wrote:
>>> Hi,
>>>
>>> When one is writing a plugin for android ATM the api that you have to
>> implement has a execute method that has the action as a string:
>>> @Override
>>> public boolean execute(String action, JSONArray args,
>> CallbackContext callbackContext) throws JSONException {
>>> if ("beep".equals(action)) {
>>> this.beep(args.getLong(0));
>>> callbackContext.success();
>>> return true;
>>> }
>>> return false; // Returning false results in a
>>> "MethodNotFound"
>> error.
>>> }
>>> When you have multiple actions this method gets very long, if you
>> compare this with iOS here you don’t need a method like this you could
>> ‘just’ implement the method directly:
>>> - (void)beep:(CDVInvokedUrlCommand*)command
>>> {
>>> CDVPluginResult* pluginResult =;
>>> NSString* myarg =mmand.arguments objectAtIndex:0];
>>>
>>> if (myarg !=) {
>>> pluginResult �VPluginResult
>> resultWithStatus:CDVCommandStatus_OK];
>>> } else {
>>> pluginResult �VPluginResult
>> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was
>> null"];
>>> }
>>> [self.commandDelegate sendPluginResult:pluginResult
>> callbackId:command.callbackId];
>>> }
>>> We could do the same thing for android if we use reflection, making the
>> API more similar and removing all the string test by the user. What do
>> you
>> think?
>>> Cheers,
>>> Erik Jan
Re: Android Plugin API
It does get complicated in a hurry. Using the technique in the Google
Maps plugin:
- getDeclaredMethod is documented to NOT return inherited methods which
limits exposure
- the signature passed to getDeclaredMethod also limits exposure
- annotations, as Andrew suggested, could further limit exposure
- getDeclaredMethod is documented to return private methods which for me
raises questions
- setAccessible(true) suppresses normal access checking which also
raises questions
- invoke throws InvocationTargetException so exceptions thrown by the
invoked method could be publicized
- invoke applies overriding which is probably a wash
- invoke returns an object which would have to be handled correctly
Quite a bit to consider.
-Terence Bandoian
On 5/29/2014 11:44 PM, Joe Bowser wrote:
On Thu, May 29, 2014 at 9:26 PM, Terence M. Bandoian wrote:
Please correct me if I'm wrong but, as I understand it, the vulnerability
stems from injecting a Java object into the WebView which, in API levels 16
and below, exposed all of the public methods of the object (small 'o')
including the methods inherited from the Object class.
Yes, you are correct in this case. You can basically use the object
methods to reflect into whatever you want. Once you have a single
class exposed in this manner, you can then reflect into it, and
basically you're done. The reason we don't use reflection is that
it's very easy to reflect into things you're not supposed to be in.
IMO, It wouldn't be hard to do this if we exposed plugins in the same
way. Right now this is an Android Vulnerability, but if we start
using reflection for plugins, we could very quickly end up making a
similar Cordova exploit if we're not careful.
This is also why we use the prompt bridge on API levels below 17.
On 5/28/2014 9:54 AM, Joe Bowser wrote:
In case anyone is curious, here's why we minimize reflection:
https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve
wrote:
Another reasonable approach would be to use a Map, but
that can be implemented on top of what is currently exposed. I'm quite
wary
of Reflection as well.
On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
The execute command exists for security reasons. We don't want any
methods other than execute exposed to Javascript. I also prefer this
approach because it is less prone to less catastrophic bugs than using
Java reflection. We try and only use reflection when we have to.
On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
wrote:
Hi,
When one is writing a plugin for android ATM the api that you have to
implement has a execute method that has the action as a string:
@Override
public boolean execute(String action, JSONArray args,
CallbackContext callbackContext) throws JSONException {
if ("beep".equals(action)) {
this.beep(args.getLong(0));
callbackContext.success();
return true;
}
return false; // Returning false results in a
"MethodNotFound"
error.
}
When you have multiple actions this method gets very long, if you
compare this with iOS here you don’t need a method like this you could
‘just’ implement the method directly:
- (void)beep:(CDVInvokedUrlCommand*)command
{
CDVPluginResult* pluginResult =;
NSString* myarg =mmand.arguments objectAtIndex:0];
if (myarg !=) {
pluginResult �VPluginResult
resultWithStatus:CDVCommandStatus_OK];
} else {
pluginResult �VPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was
null"];
}
[self.commandDelegate sendPluginResult:pluginResult
callbackId:command.callbackId];
}
We could do the same thing for android if we use reflection, making the
API more similar and removing all the string test by the user. What do
you
think?
Cheers,
Erik Jan
Re: Android Plugin API
On Thu, May 29, 2014 at 9:26 PM, Terence M. Bandoian wrote:
> Please correct me if I'm wrong but, as I understand it, the vulnerability
> stems from injecting a Java object into the WebView which, in API levels 16
> and below, exposed all of the public methods of the object (small 'o')
> including the methods inherited from the Object class.
>
Yes, you are correct in this case. You can basically use the object
methods to reflect into whatever you want. Once you have a single
class exposed in this manner, you can then reflect into it, and
basically you're done. The reason we don't use reflection is that
it's very easy to reflect into things you're not supposed to be in.
IMO, It wouldn't be hard to do this if we exposed plugins in the same
way. Right now this is an Android Vulnerability, but if we start
using reflection for plugins, we could very quickly end up making a
similar Cordova exploit if we're not careful.
This is also why we use the prompt bridge on API levels below 17.
>
>
>
> On 5/28/2014 9:54 AM, Joe Bowser wrote:
>>
>> In case anyone is curious, here's why we minimize reflection:
>>
>>
>> https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
>>
>> On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve
>> wrote:
>>>
>>> Another reasonable approach would be to use a Map, but
>>> that can be implemented on top of what is currently exposed. I'm quite
>>> wary
>>> of Reflection as well.
>>>
>>>
>>> On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
>>>
The execute command exists for security reasons. We don't want any
methods other than execute exposed to Javascript. I also prefer this
approach because it is less prone to less catastrophic bugs than using
Java reflection. We try and only use reflection when we have to.
On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
wrote:
>
> Hi,
>
> When one is writing a plugin for android ATM the api that you have to
implement has a execute method that has the action as a string:
>
> @Override
> public boolean execute(String action, JSONArray args,
CallbackContext callbackContext) throws JSONException {
>
> if ("beep".equals(action)) {
> this.beep(args.getLong(0));
> callbackContext.success();
> return true;
> }
> return false; // Returning false results in a
> "MethodNotFound"
error.
>
> }
> When you have multiple actions this method gets very long, if you
compare this with iOS here you don’t need a method like this you could
‘just’ implement the method directly:
>
> - (void)beep:(CDVInvokedUrlCommand*)command
> {
> CDVPluginResult* pluginResult =il;
> NSString* myarg =command.arguments objectAtIndex:0];
>
> if (myarg !=il) {
> pluginResult =CDVPluginResult
resultWithStatus:CDVCommandStatus_OK];
>
> } else {
> pluginResult =CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was
null"];
>
> }
> [self.commandDelegate sendPluginResult:pluginResult
callbackId:command.callbackId];
>
> }
> We could do the same thing for android if we use reflection, making the
API more similar and removing all the string test by the user. What do
you
think?
>
> Cheers,
> Erik Jan
>
>
Re: Android Plugin API
Please correct me if I'm wrong but, as I understand it, the
vulnerability stems from injecting a Java object into the WebView which,
in API levels 16 and below, exposed all of the public methods of the
object (small 'o') including the methods inherited from the Object class.
-Terence Bandoian
On 5/28/2014 9:54 AM, Joe Bowser wrote:
In case anyone is curious, here's why we minimize reflection:
https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve wrote:
Another reasonable approach would be to use a Map, but
that can be implemented on top of what is currently exposed. I'm quite wary
of Reflection as well.
On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
The execute command exists for security reasons. We don't want any
methods other than execute exposed to Javascript. I also prefer this
approach because it is less prone to less catastrophic bugs than using
Java reflection. We try and only use reflection when we have to.
On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
wrote:
Hi,
When one is writing a plugin for android ATM the api that you have to
implement has a execute method that has the action as a string:
@Override
public boolean execute(String action, JSONArray args,
CallbackContext callbackContext) throws JSONException {
if ("beep".equals(action)) {
this.beep(args.getLong(0));
callbackContext.success();
return true;
}
return false; // Returning false results in a "MethodNotFound"
error.
}
When you have multiple actions this method gets very long, if you
compare this with iOS here you don’t need a method like this you could
‘just’ implement the method directly:
- (void)beep:(CDVInvokedUrlCommand*)command
{
CDVPluginResult* pluginResult =il;
NSString* myarg =command.arguments objectAtIndex:0];
if (myarg !=il) {
pluginResult =CDVPluginResult
resultWithStatus:CDVCommandStatus_OK];
} else {
pluginResult =CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
}
[self.commandDelegate sendPluginResult:pluginResult
callbackId:command.callbackId];
}
We could do the same thing for android if we use reflection, making the
API more similar and removing all the string test by the user. What do you
think?
Cheers,
Erik Jan
Re: Android Plugin API
Even even nicer might be to use annotations: @ExecCall private void someCall(JSONArray args, CallbackContext callbackContext) On Thu, May 29, 2014 at 4:01 PM, Joe Bowser wrote: > This plugin illustrates why applying this to the general case is a > terrible idea. Here it's catching every type of exception, which is > correct since we don't know whether we're running on a device that > passes CTS. However, for example, if there's a permissions problem, > or a problem with the GPS being busted, we have to rely on a stack > trace in logcat and it won't crash. I personally prefer crashing over > silent failing, since one will get caught in my debugger, and one will > have me sifting through logcat and will go undetected for months. If > you don't know what I'm talking about, go look at Camera or File!!! > > I'm not sure what exceptions Google Maps throws, but debugging a > Google Maps problem would be a nightmare since this will catch all the > exceptions. You can do this at your own peril. > > On Thu, May 29, 2014 at 12:29 PM, Michal Mocny > wrote: > > Erik, > > > > I think it should be possible leave the platform plugin interface as-is, > > and publish a plugin that does nothing but implement > > reflection-based-execute. Then, plugin authors can opt to use that as a > > base class for plugin development. > > > > I hesitate to give opinions on this topic, except to say that its also > > something I've thought would be useful, but not enough of a big deal to > > make a stink over it. > > > > Andrew also points out that there are plugins that have been published > that > > use this approach, i.e.: > > > https://github.com/wf9a5m75/phonegap-googlemaps-plugin/blob/master/src/android/plugin/google/maps/MyPlugin.java#L45-L57 > > > > > > On Thu, May 29, 2014 at 3:01 PM, Joe Bowser wrote: > > > >> On Thu, May 29, 2014 at 11:46 AM, Erik Jan de Wit > >> wrote: > >> > > >> >> What is the benefit of using this logic? I personally don't see any > >> >> benefit, since this makes our code more complex. > >> > > >> > It would benefit the users as they don’t have to implement this > >> boilerplate code of dispatching based on the string. And this logic > will be > >> then on the android side where it’s implemented once instead of each > time > >> one is building a plugin. > >> > >> Except that now every method exposed has to have a particular > >> signature. I personally would rather see us use a Map as what Andrew > >> suggested, and I really don't want this to happen. This is going to > >> cause a lot more problems than it's going to solve since plugins that > >> already throw exceptions may end up having it caught by the try/catch > >> which will surround this whole thing. This is a BIG change in the > >> API, and I don't think your reason justifies breaking every plugin > >> that currently exists. > >> >
Re: Android Plugin API
This plugin illustrates why applying this to the general case is a terrible idea. Here it's catching every type of exception, which is correct since we don't know whether we're running on a device that passes CTS. However, for example, if there's a permissions problem, or a problem with the GPS being busted, we have to rely on a stack trace in logcat and it won't crash. I personally prefer crashing over silent failing, since one will get caught in my debugger, and one will have me sifting through logcat and will go undetected for months. If you don't know what I'm talking about, go look at Camera or File!!! I'm not sure what exceptions Google Maps throws, but debugging a Google Maps problem would be a nightmare since this will catch all the exceptions. You can do this at your own peril. On Thu, May 29, 2014 at 12:29 PM, Michal Mocny wrote: > Erik, > > I think it should be possible leave the platform plugin interface as-is, > and publish a plugin that does nothing but implement > reflection-based-execute. Then, plugin authors can opt to use that as a > base class for plugin development. > > I hesitate to give opinions on this topic, except to say that its also > something I've thought would be useful, but not enough of a big deal to > make a stink over it. > > Andrew also points out that there are plugins that have been published that > use this approach, i.e.: > https://github.com/wf9a5m75/phonegap-googlemaps-plugin/blob/master/src/android/plugin/google/maps/MyPlugin.java#L45-L57 > > > On Thu, May 29, 2014 at 3:01 PM, Joe Bowser wrote: > >> On Thu, May 29, 2014 at 11:46 AM, Erik Jan de Wit >> wrote: >> > >> >> What is the benefit of using this logic? I personally don't see any >> >> benefit, since this makes our code more complex. >> > >> > It would benefit the users as they don’t have to implement this >> boilerplate code of dispatching based on the string. And this logic will be >> then on the android side where it’s implemented once instead of each time >> one is building a plugin. >> >> Except that now every method exposed has to have a particular >> signature. I personally would rather see us use a Map as what Andrew >> suggested, and I really don't want this to happen. This is going to >> cause a lot more problems than it's going to solve since plugins that >> already throw exceptions may end up having it caught by the try/catch >> which will surround this whole thing. This is a BIG change in the >> API, and I don't think your reason justifies breaking every plugin >> that currently exists. >>
Re: Android Plugin API
I am with Joe, this is too big and breaking of a change for a small
semantic win.
Another approach might be to define an execute method in a plugin ( pick
one ) and have it self reflect, and call it's own exposed methods.
@Override
public boolean execute(String action, JSONArray args, CallbackContext
callbackContext) throws JSONException {
// pseudocode ...
if(this[action]) {
return this[action](args, callbackContext);
}
return false; // Returning false results in a "MethodNotFound"
error.
}
Then every callable method would be :
boolean methodName(JSONArray args, CallbackContext callbackContext) throws
JSONException
Potentially this could become the base class implementation of 'execute'
and presumably nothing would break ...
@purplecabbage
risingj.com
On Thu, May 29, 2014 at 12:01 PM, Joe Bowser wrote:
> On Thu, May 29, 2014 at 11:46 AM, Erik Jan de Wit
> wrote:
> >
> >> What is the benefit of using this logic? I personally don't see any
> >> benefit, since this makes our code more complex.
> >
> > It would benefit the users as they don’t have to implement this
> boilerplate code of dispatching based on the string. And this logic will be
> then on the android side where it’s implemented once instead of each time
> one is building a plugin.
>
> Except that now every method exposed has to have a particular
> signature. I personally would rather see us use a Map as what Andrew
> suggested, and I really don't want this to happen. This is going to
> cause a lot more problems than it's going to solve since plugins that
> already throw exceptions may end up having it caught by the try/catch
> which will surround this whole thing. This is a BIG change in the
> API, and I don't think your reason justifies breaking every plugin
> that currently exists.
>
Re: Android Plugin API
Erik, I think it should be possible leave the platform plugin interface as-is, and publish a plugin that does nothing but implement reflection-based-execute. Then, plugin authors can opt to use that as a base class for plugin development. I hesitate to give opinions on this topic, except to say that its also something I've thought would be useful, but not enough of a big deal to make a stink over it. Andrew also points out that there are plugins that have been published that use this approach, i.e.: https://github.com/wf9a5m75/phonegap-googlemaps-plugin/blob/master/src/android/plugin/google/maps/MyPlugin.java#L45-L57 On Thu, May 29, 2014 at 3:01 PM, Joe Bowser wrote: > On Thu, May 29, 2014 at 11:46 AM, Erik Jan de Wit > wrote: > > > >> What is the benefit of using this logic? I personally don't see any > >> benefit, since this makes our code more complex. > > > > It would benefit the users as they don’t have to implement this > boilerplate code of dispatching based on the string. And this logic will be > then on the android side where it’s implemented once instead of each time > one is building a plugin. > > Except that now every method exposed has to have a particular > signature. I personally would rather see us use a Map as what Andrew > suggested, and I really don't want this to happen. This is going to > cause a lot more problems than it's going to solve since plugins that > already throw exceptions may end up having it caught by the try/catch > which will surround this whole thing. This is a BIG change in the > API, and I don't think your reason justifies breaking every plugin > that currently exists. >
Re: Android Plugin API
On Thu, May 29, 2014 at 11:46 AM, Erik Jan de Wit wrote: > >> What is the benefit of using this logic? I personally don't see any >> benefit, since this makes our code more complex. > > It would benefit the users as they don’t have to implement this boilerplate > code of dispatching based on the string. And this logic will be then on the > android side where it’s implemented once instead of each time one is building > a plugin. Except that now every method exposed has to have a particular signature. I personally would rather see us use a Map as what Andrew suggested, and I really don't want this to happen. This is going to cause a lot more problems than it's going to solve since plugins that already throw exceptions may end up having it caught by the try/catch which will surround this whole thing. This is a BIG change in the API, and I don't think your reason justifies breaking every plugin that currently exists.
Re: Android Plugin API
> What is the benefit of using this logic? I personally don't see any > benefit, since this makes our code more complex. It would benefit the users as they don’t have to implement this boilerplate code of dispatching based on the string. And this logic will be then on the android side where it’s implemented once instead of each time one is building a plugin.
Re: Android Plugin API
I could build a little prototype that shows what I’m talking about, how does that sound?
Re: Android Plugin API
On Thu, May 29, 2014 at 1:25 AM, Erik Jan de Wit wrote: > > On 28 May,2014, at 22:07 , Shazron wrote: > >> https://github.com/apache/cordova-ios/blob/50ca482c8e861c1aa480dadba726b1abbacbc0e1/CordovaLib/Classes/CDVCommandQueue.m#L193-L198 > > Right thanks, that is how I expected it to work, so why not use the same > logic in Android as on iOS? In Java one can also find a method based on the > action name and invoke it. I would say it’s less error prone then doing > string comparison yourself. > What is the benefit of using this logic? I personally don't see any benefit, since this makes our code more complex. >> >> >> On Wed, May 28, 2014 at 12:05 PM, Erik Jan de Wit wrote: >> >>> I don't know, it very much could be. It could be that this makes sense >>> in Obj-C but not in Java based on how they handle NoSuchMethod. I'd prefer >>> to not have to rely on an exception being caught, especially since it could suppress other exceptions being thrown that I want to know about. >>> >>> Sending a message (calling a method) in object-c for a method that doesn’t >>> exist will also throw an exception, I haven’t looked at the implementation >>> but I would suspect that there is a test to see if the method (selector) is >>> there. >>> Also, I'm assuming the exception is NoSuchMethod, which isn't a safe assumption given that each device has their own quirks and this isn't guaranteed. >>> >>> One could just lookup if the method exist and not just try to invoke it >>> and wait for the NoSuchMethod. That way one could make the error handling >>> nicer, for example: >>> >>> You have a method called ‘myAction’ but it does not have the proper method >>> signature! Found public void myAction() but should be pubic PluginResult >>> myAction(JSONArray, CallbackContext) >>> >>> >
Re: Android Plugin API
On 28 May,2014, at 22:07 , Shazron wrote: > https://github.com/apache/cordova-ios/blob/50ca482c8e861c1aa480dadba726b1abbacbc0e1/CordovaLib/Classes/CDVCommandQueue.m#L193-L198 Right thanks, that is how I expected it to work, so why not use the same logic in Android as on iOS? In Java one can also find a method based on the action name and invoke it. I would say it’s less error prone then doing string comparison yourself. > > > On Wed, May 28, 2014 at 12:05 PM, Erik Jan de Wit wrote: > >> >>> >>> I don't know, it very much could be. It could be that this makes sense >> in >>> Obj-C but not in Java based on how they handle NoSuchMethod. I'd prefer >> to >>> not have to rely on an exception being caught, especially since it could >>> suppress other exceptions being thrown that I want to know about. >> >> Sending a message (calling a method) in object-c for a method that doesn’t >> exist will also throw an exception, I haven’t looked at the implementation >> but I would suspect that there is a test to see if the method (selector) is >> there. >> >>> >>> Also, I'm assuming the exception is NoSuchMethod, which isn't a safe >>> assumption given that each device has their own quirks and this isn't >>> guaranteed. >> >> One could just lookup if the method exist and not just try to invoke it >> and wait for the NoSuchMethod. That way one could make the error handling >> nicer, for example: >> >> You have a method called ‘myAction’ but it does not have the proper method >> signature! Found public void myAction() but should be pubic PluginResult >> myAction(JSONArray, CallbackContext) >> >>
Re: Android Plugin API
https://github.com/apache/cordova-ios/blob/50ca482c8e861c1aa480dadba726b1abbacbc0e1/CordovaLib/Classes/CDVCommandQueue.m#L193-L198 On Wed, May 28, 2014 at 12:05 PM, Erik Jan de Wit wrote: > > > > > I don't know, it very much could be. It could be that this makes sense > in > > Obj-C but not in Java based on how they handle NoSuchMethod. I'd prefer > to > > not have to rely on an exception being caught, especially since it could > > suppress other exceptions being thrown that I want to know about. > > Sending a message (calling a method) in object-c for a method that doesn’t > exist will also throw an exception, I haven’t looked at the implementation > but I would suspect that there is a test to see if the method (selector) is > there. > > > > > Also, I'm assuming the exception is NoSuchMethod, which isn't a safe > > assumption given that each device has their own quirks and this isn't > > guaranteed. > > One could just lookup if the method exist and not just try to invoke it > and wait for the NoSuchMethod. That way one could make the error handling > nicer, for example: > > You have a method called ‘myAction’ but it does not have the proper method > signature! Found public void myAction() but should be pubic PluginResult > myAction(JSONArray, CallbackContext) > >
Re: Android Plugin API
iOS has [object respondsToSelector:@selector(selector)] to check if selector exists or not. On 5/28/14 12:05 PM, "Erik Jan de Wit" wrote: > >> >> I don't know, it very much could be. It could be that this makes sense >>in >> Obj-C but not in Java based on how they handle NoSuchMethod. I'd >>prefer to >> not have to rely on an exception being caught, especially since it could >> suppress other exceptions being thrown that I want to know about. > >Sending a message (calling a method) in object-c for a method that >doesn¹t exist will also throw an exception, I haven¹t looked at the >implementation but I would suspect that there is a test to see if the >method (selector) is there. > >> >> Also, I'm assuming the exception is NoSuchMethod, which isn't a safe >> assumption given that each device has their own quirks and this isn't >> guaranteed. > >One could just lookup if the method exist and not just try to invoke it >and wait for the NoSuchMethod. That way one could make the error handling >nicer, for example: > >You have a method called ŒmyAction¹ but it does not have the proper >method signature! Found public void myAction() but should be pubic >PluginResult myAction(JSONArray, CallbackContext) >
Re: Android Plugin API
> > I don't know, it very much could be. It could be that this makes sense in > Obj-C but not in Java based on how they handle NoSuchMethod. I'd prefer to > not have to rely on an exception being caught, especially since it could > suppress other exceptions being thrown that I want to know about. Sending a message (calling a method) in object-c for a method that doesn’t exist will also throw an exception, I haven’t looked at the implementation but I would suspect that there is a test to see if the method (selector) is there. > > Also, I'm assuming the exception is NoSuchMethod, which isn't a safe > assumption given that each device has their own quirks and this isn't > guaranteed. One could just lookup if the method exist and not just try to invoke it and wait for the NoSuchMethod. That way one could make the error handling nicer, for example: You have a method called ‘myAction’ but it does not have the proper method signature! Found public void myAction() but should be pubic PluginResult myAction(JSONArray, CallbackContext)
Re: Android Plugin API
On May 28, 2014 11:21 AM, "Erik Jan de Wit" wrote: > > > On 28 May,2014, at 19:06 , Joe Bowser wrote: > > > We don't want this pattern for Android because it is also more bug prone. > > Doesn’t the same hold true for iOS? > I don't know, it very much could be. It could be that this makes sense in Obj-C but not in Java based on how they handle NoSuchMethod. I'd prefer to not have to rely on an exception being caught, especially since it could suppress other exceptions being thrown that I want to know about. Also, I'm assuming the exception is NoSuchMethod, which isn't a safe assumption given that each device has their own quirks and this isn't guaranteed. > > > > On May 28, 2014 8:28 AM, "Erik Jan de Wit" wrote: > >> > >> So this security issue is only a problem if you are able to inject some > > arbitrary js code. If your app ships with it’s own html and js this is very > > hard to do. > > > > No, it's not. Any trusted input could have the potential to inject JS. > > We're not even touching on the third-party ad networks code, frameworks or > > other code that developers add on a regular basis. > > Still in the example android permits any method to be executed (getClass) there could be checks. For instance only public methods that have a JSONArray and a CallbackContext as parameters and have the name of the action. That way you can’t inject any arbitrary code. If a user implements the wrong method the error logging can be in a way that one can easily correct the issue, because of these checks. >
Re: Android Plugin API
On 28 May,2014, at 19:06 , Joe Bowser wrote: > We don't want this pattern for Android because it is also more bug prone. Doesn’t the same hold true for iOS? > > On May 28, 2014 8:28 AM, "Erik Jan de Wit" wrote: >> >> So this security issue is only a problem if you are able to inject some > arbitrary js code. If your app ships with it’s own html and js this is very > hard to do. > > No, it's not. Any trusted input could have the potential to inject JS. > We're not even touching on the third-party ad networks code, frameworks or > other code that developers add on a regular basis. Still in the example android permits any method to be executed (getClass) there could be checks. For instance only public methods that have a JSONArray and a CallbackContext as parameters and have the name of the action. That way you can’t inject any arbitrary code. If a user implements the wrong method the error logging can be in a way that one can easily correct the issue, because of these checks.
Re: Android Plugin API
We don't want this pattern for Android because it is also more bug prone. On May 28, 2014 8:28 AM, "Erik Jan de Wit" wrote: > > So this security issue is only a problem if you are able to inject some arbitrary js code. If your app ships with it’s own html and js this is very hard to do. No, it's not. Any trusted input could have the potential to inject JS. We're not even touching on the third-party ad networks code, frameworks or other code that developers add on a regular basis.
Re: Android Plugin API
So this security issue is only a problem if you are able to inject some
arbitrary js code. If your app ships with it’s own html and js this is very
hard to do. If the only reason for this is security then why not have the same
approach on iOS here methods are invoked dynamicly.
On 28 May,2014, at 16:54 , Joe Bowser wrote:
> In case anyone is curious, here's why we minimize reflection:
>
> https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
>
> On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve wrote:
>> Another reasonable approach would be to use a Map, but
>> that can be implemented on top of what is currently exposed. I'm quite wary
>> of Reflection as well.
>>
>>
>> On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
>>
>>> The execute command exists for security reasons. We don't want any
>>> methods other than execute exposed to Javascript. I also prefer this
>>> approach because it is less prone to less catastrophic bugs than using
>>> Java reflection. We try and only use reflection when we have to.
>>>
>>> On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
>>> wrote:
Hi,
When one is writing a plugin for android ATM the api that you have to
>>> implement has a execute method that has the action as a string:
@Override
public boolean execute(String action, JSONArray args,
>>> CallbackContext callbackContext) throws JSONException {
if ("beep".equals(action)) {
this.beep(args.getLong(0));
callbackContext.success();
return true;
}
return false; // Returning false results in a "MethodNotFound"
>>> error.
}
When you have multiple actions this method gets very long, if you
>>> compare this with iOS here you don’t need a method like this you could
>>> ‘just’ implement the method directly:
- (void)beep:(CDVInvokedUrlCommand*)command
{
CDVPluginResult* pluginResult = nil;
NSString* myarg = [command.arguments objectAtIndex:0];
if (myarg != nil) {
pluginResult = [CDVPluginResult
>>> resultWithStatus:CDVCommandStatus_OK];
} else {
pluginResult = [CDVPluginResult
>>> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
}
[self.commandDelegate sendPluginResult:pluginResult
>>> callbackId:command.callbackId];
}
We could do the same thing for android if we use reflection, making the
>>> API more similar and removing all the string test by the user. What do you
>>> think?
Cheers,
Erik Jan
>>>
Re: Android Plugin API
In case anyone is curious, here's why we minimize reflection:
https://labs.mwrinfosecurity.com/blog/2013/09/24/webview-addjavascriptinterface-remote-code-execution/
On Wed, May 28, 2014 at 7:33 AM, Andrew Grieve wrote:
> Another reasonable approach would be to use a Map, but
> that can be implemented on top of what is currently exposed. I'm quite wary
> of Reflection as well.
>
>
> On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
>
>> The execute command exists for security reasons. We don't want any
>> methods other than execute exposed to Javascript. I also prefer this
>> approach because it is less prone to less catastrophic bugs than using
>> Java reflection. We try and only use reflection when we have to.
>>
>> On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
>> wrote:
>> > Hi,
>> >
>> > When one is writing a plugin for android ATM the api that you have to
>> implement has a execute method that has the action as a string:
>> > @Override
>> > public boolean execute(String action, JSONArray args,
>> CallbackContext callbackContext) throws JSONException {
>> > if ("beep".equals(action)) {
>> > this.beep(args.getLong(0));
>> > callbackContext.success();
>> > return true;
>> > }
>> > return false; // Returning false results in a "MethodNotFound"
>> error.
>> > }
>> > When you have multiple actions this method gets very long, if you
>> compare this with iOS here you don’t need a method like this you could
>> ‘just’ implement the method directly:
>> > - (void)beep:(CDVInvokedUrlCommand*)command
>> > {
>> > CDVPluginResult* pluginResult = nil;
>> > NSString* myarg = [command.arguments objectAtIndex:0];
>> >
>> > if (myarg != nil) {
>> > pluginResult = [CDVPluginResult
>> resultWithStatus:CDVCommandStatus_OK];
>> > } else {
>> > pluginResult = [CDVPluginResult
>> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
>> > }
>> > [self.commandDelegate sendPluginResult:pluginResult
>> callbackId:command.callbackId];
>> > }
>> > We could do the same thing for android if we use reflection, making the
>> API more similar and removing all the string test by the user. What do you
>> think?
>> >
>> > Cheers,
>> > Erik Jan
>>
Re: Android Plugin API
Another reasonable approach would be to use a Map, but
that can be implemented on top of what is currently exposed. I'm quite wary
of Reflection as well.
On Wed, May 28, 2014 at 10:06 AM, Joe Bowser wrote:
> The execute command exists for security reasons. We don't want any
> methods other than execute exposed to Javascript. I also prefer this
> approach because it is less prone to less catastrophic bugs than using
> Java reflection. We try and only use reflection when we have to.
>
> On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit
> wrote:
> > Hi,
> >
> > When one is writing a plugin for android ATM the api that you have to
> implement has a execute method that has the action as a string:
> > @Override
> > public boolean execute(String action, JSONArray args,
> CallbackContext callbackContext) throws JSONException {
> > if ("beep".equals(action)) {
> > this.beep(args.getLong(0));
> > callbackContext.success();
> > return true;
> > }
> > return false; // Returning false results in a "MethodNotFound"
> error.
> > }
> > When you have multiple actions this method gets very long, if you
> compare this with iOS here you don’t need a method like this you could
> ‘just’ implement the method directly:
> > - (void)beep:(CDVInvokedUrlCommand*)command
> > {
> > CDVPluginResult* pluginResult = nil;
> > NSString* myarg = [command.arguments objectAtIndex:0];
> >
> > if (myarg != nil) {
> > pluginResult = [CDVPluginResult
> resultWithStatus:CDVCommandStatus_OK];
> > } else {
> > pluginResult = [CDVPluginResult
> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
> > }
> > [self.commandDelegate sendPluginResult:pluginResult
> callbackId:command.callbackId];
> > }
> > We could do the same thing for android if we use reflection, making the
> API more similar and removing all the string test by the user. What do you
> think?
> >
> > Cheers,
> > Erik Jan
>
Re: Android Plugin API
The execute command exists for security reasons. We don't want any
methods other than execute exposed to Javascript. I also prefer this
approach because it is less prone to less catastrophic bugs than using
Java reflection. We try and only use reflection when we have to.
On Wed, May 28, 2014 at 5:50 AM, Erik Jan de Wit wrote:
> Hi,
>
> When one is writing a plugin for android ATM the api that you have to
> implement has a execute method that has the action as a string:
> @Override
> public boolean execute(String action, JSONArray args, CallbackContext
> callbackContext) throws JSONException {
> if ("beep".equals(action)) {
> this.beep(args.getLong(0));
> callbackContext.success();
> return true;
> }
> return false; // Returning false results in a "MethodNotFound" error.
> }
> When you have multiple actions this method gets very long, if you compare
> this with iOS here you don’t need a method like this you could ‘just’
> implement the method directly:
> - (void)beep:(CDVInvokedUrlCommand*)command
> {
> CDVPluginResult* pluginResult = nil;
> NSString* myarg = [command.arguments objectAtIndex:0];
>
> if (myarg != nil) {
> pluginResult = [CDVPluginResult
> resultWithStatus:CDVCommandStatus_OK];
> } else {
> pluginResult = [CDVPluginResult
> resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
> }
> [self.commandDelegate sendPluginResult:pluginResult
> callbackId:command.callbackId];
> }
> We could do the same thing for android if we use reflection, making the API
> more similar and removing all the string test by the user. What do you think?
>
> Cheers,
> Erik Jan
Android Plugin API
Hi,
When one is writing a plugin for android ATM the api that you have to implement
has a execute method that has the action as a string:
@Override
public boolean execute(String action, JSONArray args, CallbackContext
callbackContext) throws JSONException {
if ("beep".equals(action)) {
this.beep(args.getLong(0));
callbackContext.success();
return true;
}
return false; // Returning false results in a "MethodNotFound" error.
}
When you have multiple actions this method gets very long, if you compare this
with iOS here you don’t need a method like this you could ‘just’ implement the
method directly:
- (void)beep:(CDVInvokedUrlCommand*)command
{
CDVPluginResult* pluginResult = nil;
NSString* myarg = [command.arguments objectAtIndex:0];
if (myarg != nil) {
pluginResult = [CDVPluginResult
resultWithStatus:CDVCommandStatus_OK];
} else {
pluginResult = [CDVPluginResult
resultWithStatus:CDVCommandStatus_ERROR messageAsString:@"Arg was null"];
}
[self.commandDelegate sendPluginResult:pluginResult
callbackId:command.callbackId];
}
We could do the same thing for android if we use reflection, making the API
more similar and removing all the string test by the user. What do you think?
Cheers,
Erik Jan
