forrestguice / SuntimesWidget

Android app (and widget collection) that displays sunlight and moonlight times.
GNU General Public License v3.0
348 stars 61 forks source link

[Bug] Potential NullPointerException in AlarmSettings #633

Closed LightningRS closed 2 years ago

LightningRS commented 2 years ago

Description: We inspected the latest release version of Suntimes (0.14.8 from F-droid) using an Android application testing tool under development, and found a potential NullPointerException bug.

In the public function com.forrestguice.suntimeswidget.alarmclock.AlarmSettings.setDefaultRingtone(), line 303 and line 309 in AlarmSettings.java, an Android API RingtoneManager.getActualDefaultRingtoneUri() has been called, and the returned Uri object is stored in the variable uri.

    uri = RingtoneManager.getActualDefaultRingtoneUri(context, RingtoneManager.TYPE_ALARM);
    ...
    uri = RingtoneManager.getActualDefaultRingtoneUri(context, RingtoneManager.TYPE_NOTIFICATION);

However, according to Android SDK 25, this API call may return null.

    public static Uri getActualDefaultRingtoneUri(Context context, int type) {
        String setting = getSettingForType(type);
        if (setting == null) return null;  // might return null
        final String uriString = Settings.System.getStringForUser(context.getContentResolver(),
                setting, context.getUserId());
        return uriString != null ? Uri.parse(uriString) : null;  // might return null
    }

At line 316, uri.toString() method is called without checking whether uri is null, which cause NullPointerException.

        prefs.putString(key_uri, uri.toString()); // Potential NPE

Suggested solution:

We suggest changing the code on line 316 to the following form to avoid the problem.

        prefs.putString(key_uri, uri != null ? uri.toString() : null); // Avoid NPE

Stacktrace:

E AndroidRuntime: java.lang.RuntimeException: An error occurred while executing doInBackground()
E AndroidRuntime:   at android.os.AsyncTask$3.done(AsyncTask.java:325)
E AndroidRuntime:   at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:354)
E AndroidRuntime:   at java.util.concurrent.FutureTask.setException(FutureTask.java:223)
E AndroidRuntime:   at java.util.concurrent.FutureTask.run(FutureTask.java:242)
E AndroidRuntime:   at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:243)
E AndroidRuntime:   at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
E AndroidRuntime:   at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
E AndroidRuntime:   at java.lang.Thread.run(Thread.java:761)
E AndroidRuntime: Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.String android.net.Uri.toString()' on a null object reference
E AndroidRuntime:   at com.forrestguice.suntimeswidget.alarmclock.AlarmSettings.setDefaultRingtone(AlarmSettings.java:316)
E AndroidRuntime:   at com.forrestguice.suntimeswidget.alarmclock.AlarmSettings$CacheDefaultRingtoneTask.doInBackground(AlarmSettings.java:345)
E AndroidRuntime:   at com.forrestguice.suntimeswidget.alarmclock.AlarmSettings$CacheDefaultRingtoneTask.doInBackground(AlarmSettings.java:332)
E AndroidRuntime:   at android.os.AsyncTask$2.call(AsyncTask.java:305)
E AndroidRuntime:   at java.util.concurrent.FutureTask.run(FutureTask.java:237)

Version Info:

forrestguice commented 2 years ago

Thanks for bringing this to my attention. I've applied the nullcheck recommended in #634 - it should be fixed as part of v0.14.9.