btkelly/gandalf

Semantic version numbers can cause app crash

kc8nod opened this issue ยท 20 comments

Setting the value of "optionalVersion" or "minimumVersion" to a value like "1.1.1" can cause the app to crash.

The library was designed to be used with the version code and not the version name. We found that all the possible values a user might set as the version name made it an unreliable source of truth in determining which version was older vs newer. We actually added code which catches the NumberFormatException and generates a new BootstrapException to be clear that is was a caught error and not a library crash. I'm wondering if we should add the ability to set a custom VersionChecker to allow users to change the default behaviour if desired.

Creating separate issues from this

what happened with this issue? Was this fixed?

We are running into this issue with BootstrapException where the minimumVersion is set to a float or double causing the app to crash.

Also how can we handle this error?

In our application code we have the following:

try{ appUpdateRequest(); }catch (BootstrapException be){ be.printStackTrace(); } catch (NumberFormatException Ne){ Ne.printStackTrace(); } catch (Exception e){ e.printStackTrace(); }

However the app will still crash which is unacceptable.

@rankinit couple questions that would help us out!

  1. What version of Gandalf are you using?
  2. What are some examples of version numbers that cause the crashes you are seeing?

Thanks!

Hi thanks for responding so fast!

  1. 1.3.4 = implementation 'com.btkelly:gandalf:1.3.4' - we are still on support library and will migrate soon to androidx
  2. currently we are using 40.12 to invoke the crash when calling the api:

sample testing json:

    "android": {
        "optionalUpdate": {
            "optionalVersion": "1.0.3",
            "message": "Optional test"
        },
        "requiredUpdate": {
            "minimumVersion": "40.12",
            "message": "A new version of this app is available and is required to to continue."
        }
    },
    "ios": {
        "requiredUpdate": {
            "minimumVersion": "1.1.2",
            "message": "A new version of the application is available and is required to continue, please click below to update to the latest version."
        }
    }
}

Cool, thanks for the data. I'll take a peek and see what's up if @btkelly doesn't :)

First option: can you switch to supplying version codes rather than version names? From the README:

The JSON file use the Android versionCode not the versionName for version information.

That's probably the root of this issue; version codes are integers so the . is unexpected.

If that doesn't work for you it looks like there is code in place to allow you to customize that parsing, but let's start with the easiest option first. LMK!

Hi Stuart - we know the version code should be supplied (integers) instead of version name and the library works well when that is the case. However we are worried if the input gets malformed for whatever reason and we didn't catch it the app will crash (this very issue came up in testing when someone did supply the wrong format in our database).

I think it would be ideal being able to handle the exception on our end in the application code or maybe not throwing an runtime exception but log the error in some other fashion.

Ahh, ok, I follow now. Thinking some more...

Is appUpdateRequest your own method? What does it call if so?

Here is a gist: https://gist.github.com/rankinit/68bc73162a26b04cb6a4dcf6529494fa

    private void appUpdateRequest(){
        BootstrapDialogUtil.BootstrapDialogListener bootstrapDialogListener = new BootstrapDialogUtil.BootstrapDialogListener() {
            @Override
            public void continueLoading() {
                initUI();
            }
        };

        GandalfCallback gandalfCallback = new GandalfCallback() {
            @Override
            public void onRequiredUpdate(RequiredUpdate requiredUpdate) {
                BootstrapDialogUtil.showRequiredUpdateDialog(SplashActivity.this, Gandalf.get(), requiredUpdate);
            }

            @Override
            public void onOptionalUpdate(OptionalUpdate optionalUpdate) {
                BootstrapDialogUtil.showOptionalUpdateDialog(SplashActivity.this, Gandalf.get(), optionalUpdate,bootstrapDialogListener);
            }

            @Override
            public void onAlert(Alert alert) {
                BootstrapDialogUtil.showAlertDialog(SplashActivity.this, Gandalf.get(), alert, bootstrapDialogListener);
            }

            @Override
            public void onNoActionRequired() {
                initUI();
            }
        };
        Gandalf.get().shallIPass(gandalfCallback);
    }

@rankinit what is the actual stack trace of the crashes you're seeing? Having trouble figuring out where they are coming from. Thanks :)

Reference for self: v1.3.4 files

stacktrace:

Process: com.projects.my.app, PID: 29043

io.github.btkelly.gandalf.models.BootstrapException: Invalid number format on RequiredUpdate version
        at io.github.btkelly.gandalf.checker.DefaultVersionChecker.showRequiredUpdate(DefaultVersionChecker.java:53)
        at io.github.btkelly.gandalf.checker.GateKeeper.updateIsRequired(GateKeeper.java:61)
        at io.github.btkelly.gandalf.Gandalf$1.onSuccess(Gandalf.java:148)
        at io.github.btkelly.gandalf.network.BootstrapApi$1$1.run(BootstrapApi.java:125)
        at android.os.Handler.handleCallback(Handler.java:790)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:164)
        at android.app.ActivityThread.main(ActivityThread.java:6650)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:547)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:818)
     Caused by: java.lang.NumberFormatException: For input string: "40.12"
        at java.lang.Integer.parseInt(Integer.java:608)
        at java.lang.Integer.valueOf(Integer.java:794)

Interesting, thanks! ๐Ÿ•ต๏ธ

I hear @btkelly is taking a look at this at the moment :)

@stkent thanks for the update!

@rankinit I should have a PR up for @stkent to review tomorrow. As it usually happens this change has turned into a few more updates as it was needed to keep things up to date. I'll release a new version once we have it tested and merged. Also, thanks @stkent for being a much better communicator ๐Ÿ˜‚

@btkelly - thank you so much.. really appreciate how fast and responsive you guys have been!

@rankinit version 1.4.1 has been published with this fix. Let us know if you have any issues with it ๐Ÿ‘