Fix nil deploymentkey crash #109
Conversation
…yment key to updateSuccess so we can report and run without a config.xml deployment key specified
| @@ -58,10 +58,14 @@ - (void)handleUnconfirmedInstall:(BOOL)navigate { | |||
| } | |||
|
|
|||
| - (void)updateSuccess:(CDVInvokedUrlCommand *)command { | |||
| NSString* deploymentKey = [command argumentAtIndex:0 withDefault:nil andClass:[NSString class]]; | |||
| if ([deploymentKey length] == 0) { | |||
There was a problem hiding this comment.
Yes. Calling length on nil will return 0.
…aking. Add in built js file
| // no-op becuase we don't care if we didn't get a deployment key from script; we will just try to use the one from preferences | ||
| } | ||
|
|
||
| if (deploymentKey.equals(null) || deploymentKey.equals("") || deploymentKey.equals("null") || deploymentKey.equals("undefined")) { |
There was a problem hiding this comment.
deploymentKey.equals(null) will never be true. If deploymentKey == null, deploymentKey.equals(null) will throw a NullPointerException because you are invoking the equals method on a null object ref. You really want to check for deploymentKey == null.
There was a problem hiding this comment.
Right. Forogt about that. Fixed.
|
imo I think this problem is a little more complex than this solution. basically, we now have 3 separate deployment keys right now, the default key is null. when we don't have a key in config.xml and no key was passed into checkForUpdate, then we crash. I agree with @dlebu 's assessment that we can be smarter in our native code to prevent the default key from being null. I like adding the deployment key as an argument to notifyApplicationReady, but I think that if the deployment key is unspecified, it should use the key used in the last call to checkForUpdate (when you call notifyApplicationReady you're probably trying to validate the last update u checked for). If not, it should use the key in the config.xml. |
| } catch (PackageManager.NameNotFoundException e) { | ||
| // Should not happen unless the appVersion is not specified, in which case we can't report anything anyway. | ||
| e.printStackTrace(); | ||
| } |
There was a problem hiding this comment.
With this code removed, wouldn't that mean we wont be able to report binary updates?
|
Pulling this PR in favor of minimum change required to unblock. |
Fixes a crash that was caused by attempting to report status when the deployment key wasn't specified in config.xml.
Addresses #108
@lostintangent @geof90