From 2c3f1017ee5a58a07fc534d31c5b812989886d07 Mon Sep 17 00:00:00 2001 From: LisoUseInAIKyrios <118716522+LisoUseInAIKyrios@users.noreply.github.com> Date: Mon, 2 Sep 2024 19:31:59 -0400 Subject: [PATCH] refactor: Run all checks off main thread in sequence. Fix dialog button null. Do not export environment check setting (setting can still be overwrote by adding to import text). Fix dialog not showing on top of announcements. --- .../integrations/shared/checks/Check.java | 100 +++++------ .../shared/checks/CheckEnvironmentPatch.java | 155 +++++++++--------- .../youtube/settings/Settings.java | 2 +- 3 files changed, 135 insertions(+), 122 deletions(-) diff --git a/app/src/main/java/app/revanced/integrations/shared/checks/Check.java b/app/src/main/java/app/revanced/integrations/shared/checks/Check.java index 93d8293c39..9bec631db5 100644 --- a/app/src/main/java/app/revanced/integrations/shared/checks/Check.java +++ b/app/src/main/java/app/revanced/integrations/shared/checks/Check.java @@ -14,35 +14,25 @@ import app.revanced.integrations.shared.Utils; import app.revanced.integrations.youtube.settings.Settings; -import java.util.function.Function; +import java.util.concurrent.atomic.AtomicReference; import static android.text.Html.FROM_HTML_MODE_COMPACT; import static app.revanced.integrations.shared.StringRef.str; abstract class Check { + private static final String REVANCED_LINKS_HTML_TEXT = + "<>ul>" + + "
  • Website" + + "
  • Discord" + + "
  • Reddit" + + "
  • Twitter" + + "
  • Telegram" + + "
  • YouTube" + + ""; + + private static final int MINIMUM_SECONDS_TO_SHOW_WARNING = 7; + private static final Uri GOOD_SOURCE = Uri.parse("https://revanced.app"); - private static final Function CHECK_FAILED_DIALOG_BUILDER = - (context) -> new AlertDialog.Builder(context) - .setCancelable(false) - .setIcon(android.R.drawable.ic_dialog_alert) - .setTitle(str("revanced_check_environment_failed_title")) - .setPositiveButton( - str("revanced_check_environment_dialog_open_official_source_button"), - (dialog, which) -> { - final var intent = new Intent(Intent.ACTION_VIEW, GOOD_SOURCE); - intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); - - context.startActivity(intent); - } - ).setNegativeButton( - str("revanced_check_environment_dialog_ignore_button"), - (dialog, which) -> { - final int current = Settings.CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT.get(); - Settings.CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT.save(current + 1); - - dialog.dismiss(); - } - ); private final String failedReasonStringKey; @@ -67,55 +57,73 @@ static void disableForever() { @SuppressLint("NewApi") static void issueWarning(Context context, Check... failedChecks) { + Utils.verifyOnMainThread(); + final var reasons = new StringBuilder(); reasons.append(""); - final var finalMessage = Html.fromHtml( - String.format(str("revanced_check_environment_failed_message"), reasons), + var message = Html.fromHtml( + str("revanced_check_environment_failed_message", reasons.toString(), REVANCED_LINKS_HTML_TEXT), FROM_HTML_MODE_COMPACT ); - AlertDialog dialog = CHECK_FAILED_DIALOG_BUILDER.apply(context) - .setMessage(finalMessage).create(); - var dismissButton = dialog.getButton(DialogInterface.BUTTON_NEGATIVE); - // TODO: setEnabled is called on null for some reason. - // dismissButton.setEnabled(false); + AlertDialog dialog = new AlertDialog.Builder(context) + .setCancelable(false) + .setIconAttribute(android.R.attr.alertDialogIcon) + .setTitle(str("revanced_check_environment_failed_title")) + .setMessage(message) + .setPositiveButton( + str("revanced_check_environment_dialog_open_official_source_button"), + (dialog1, which) -> { + final var intent = new Intent(Intent.ACTION_VIEW, GOOD_SOURCE); + intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + + context.startActivity(intent); + } + ).setNegativeButton( + str("revanced_check_environment_dialog_ignore_button"), + (dialog1, which) -> { + final int current = Settings.CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT.get(); + Settings.CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT.save(current + 1); + + dialog1.dismiss(); + } + ).create(); + + dialog.show(); // Must show before getting the dismiss button or setting movement method. - dialog.show(); + var dismissButton = dialog.getButton(DialogInterface.BUTTON_NEGATIVE); + dismissButton.setEnabled(false); ((TextView)dialog.findViewById(android.R.id.message)).setMovementMethod(LinkMovementMethod.getInstance()); - // TODO: This does not work for some reason. - // Use a delay to allow the activity to finish initializing. - // Otherwise, if device is in dark mode the dialog is shown with wrong color scheme. - Utils.runOnMainThreadDelayed(getCountdownRunnable(dismissButton), 100); + // Use a longer delay than any of the other patches that can show a dialog on startup + // (Announcements, Check watch history), but there is still a chance a slow network + // can cause the dialogs to be out of order. + Utils.runOnMainThreadDelayed(getCountdownRunnable(dismissButton), 1000); } private static Runnable getCountdownRunnable(Button dismissButton) { - // Using a reference to an array to be able to modify the value in the Runnable. - final int[] secondsRemaining = {5}; + // Don't need atomic, but do need a mutable reference to modify from inside the runnable. + AtomicReference secondsRemainingRef = new AtomicReference<>(MINIMUM_SECONDS_TO_SHOW_WARNING); return new Runnable() { @Override public void run() { // Reduce the remaining time by 1 second, but only show the countdown when 3 seconds are left // to not draw the user's attention to the dismiss button too early. - if (secondsRemaining[0] > 0) { - if (secondsRemaining[0] <= 3) { - dismissButton.setText(String.format( - str("revanced_check_environment_dialog_ignore_button_countdown"), - secondsRemaining[0]) - ); + final int secondsRemaining = secondsRemainingRef.get(); + if (secondsRemaining > 0) { + if (secondsRemaining <= 3) { + dismissButton.setText(str("revanced_check_environment_dialog_ignore_button_countdown", secondsRemaining)); } - secondsRemaining[0]--; + secondsRemainingRef.set(secondsRemaining - 1); Utils.runOnMainThreadDelayed(this, 1000); } else { diff --git a/app/src/main/java/app/revanced/integrations/shared/checks/CheckEnvironmentPatch.java b/app/src/main/java/app/revanced/integrations/shared/checks/CheckEnvironmentPatch.java index 39eb610fd3..58316b53a8 100644 --- a/app/src/main/java/app/revanced/integrations/shared/checks/CheckEnvironmentPatch.java +++ b/app/src/main/java/app/revanced/integrations/shared/checks/CheckEnvironmentPatch.java @@ -1,12 +1,13 @@ package app.revanced.integrations.shared.checks; +import static app.revanced.integrations.shared.checks.PatchInfo.Build.*; +import static app.revanced.integrations.shared.checks.PatchInfo.*; + import android.annotation.SuppressLint; import android.app.Activity; import android.content.pm.PackageManager; import android.os.Build; import android.util.Base64; -import app.revanced.integrations.shared.Logger; -import app.revanced.integrations.shared.Utils; import java.io.BufferedReader; import java.io.InputStreamReader; @@ -19,8 +20,8 @@ import java.util.Random; import java.util.Set; -import static app.revanced.integrations.shared.checks.PatchInfo.Build.*; -import static app.revanced.integrations.shared.checks.PatchInfo.*; +import app.revanced.integrations.shared.Logger; +import app.revanced.integrations.shared.Utils; /** * This class is used to check if the app was patched by the user @@ -28,7 +29,14 @@ *
    * Various indicators help to detect if the app was patched by the user. */ +@SuppressWarnings("unused") public final class CheckEnvironmentPatch { + /** + * For debugging and development only. + * Forces all checks to be performed, and the check failed dialog to be shown. + */ + private static final boolean DEBUG_ALWAYS_SHOW_CHECK_FAILED_DIALOG = false; + /** * Check if the app is installed by the manager or the app store. *
    @@ -99,8 +107,8 @@ protected boolean run() { *
    * If the build properties are different, the app was likely downloaded pre-patched or patched on another device. */ - private static final Check isPatchTimeBuildCheck = new Check( - "revanced_check_environment_not_patch_time_build" + private static final Check isPatchingDeviceSameCheck = new Check( + "revanced_check_environment_not_same_patching_device" ) { @SuppressLint({"NewApi", "HardwareIds"}) @Override @@ -141,9 +149,9 @@ protected boolean run() { }; /** - * Check if the app was installed within the last 30 minutes after being patched. + * Check if the app was installed within the last 15 minutes after being patched. *
    - * If the app was installed within the last 30 minutes, it is likely, the app was patched by the user. + * If the app was installed within the last 15 minutes, it is likely, the app was patched by the user. *
    * If the app was installed at a later time, it is likely, the app was downloaded pre-patched, the user * waited too long to install the app or the patch time is too long ago. @@ -155,9 +163,10 @@ protected boolean run() { protected boolean run() { final var duration = System.currentTimeMillis() - PATCH_TIME; - Logger.printDebug(() -> "Installed in " + duration / 1000 + " seconds after patch"); + Logger.printDebug(() -> "Installed: " + (duration / 1000) + " seconds after patching"); - return duration < 1000 * 60 * 30; // 1000 ms * 60 s * 30 min; + // Dialog text says 10 minutes, but use 15 in case they're off. + return duration < 15 * 60 * 1000; // 15 minutes. } }; @@ -169,10 +178,11 @@ protected boolean run() { * If the IP address is different, the app was likely downloaded pre-patched or the patch time is too long ago. */ private static final Check hasPatchTimePublicIPCheck = new Check( - "revanced_check_environment_not_patch_time_public_ip" + "revanced_check_environment_not_same_patch_time_network_address" ) { @Override protected boolean run() { + Utils.verifyOffMainThread(); if (!Utils.isNetworkConnected()) return false; // Using multiple services to increase reliability, distribute the load and minimize tracking. @@ -186,38 +196,29 @@ protected boolean run() { } }; - String publicIP = null; - + String publicIP; + HttpURLConnection urlConnection = null; try { var service = getIpServices.get(new Random().nextInt(getIpServices.size() - 1)); - Logger.printDebug(() -> "Using " + service + " to get public IP"); - HttpURLConnection urlConnection = (HttpURLConnection) new URL(service).openConnection(); - urlConnection.setConnectTimeout(1000); - urlConnection.setReadTimeout(1000); - - try { - final var stream = Utils.submitOnBackgroundThread(urlConnection::getInputStream).get(); - BufferedReader in = new BufferedReader(new InputStreamReader(stream)); + urlConnection = (HttpURLConnection) new URL(service).openConnection(); + urlConnection.setFixedLengthStreamingMode(0); + urlConnection.setConnectTimeout(10000); + urlConnection.setReadTimeout(10000); + try (BufferedReader in = new BufferedReader(new InputStreamReader(urlConnection.getInputStream()))) { publicIP = in.readLine(); - - String finalPublicIP = publicIP; - Logger.printDebug(() -> "Got public IP " + finalPublicIP); - - in.close(); - } finally { - urlConnection.disconnect(); } - } catch (Exception e) { + } catch (Exception ex) { // If the app does not have the INTERNET permission or the service is down, // the public IP can not be retrieved. - Logger.printDebug(() -> "Failed to get public IP address", e); - } - - if (publicIP == null) { + Logger.printDebug(() -> "Failed to get public IP address", ex); return false; + } finally { + if (urlConnection != null) { + urlConnection.disconnect(); + } } final var passed = equalsHash( @@ -226,66 +227,70 @@ protected boolean run() { PUBLIC_IP_DURING_PATCH ); - if (passed) { - Logger.printDebug(() -> "Public IP matches patch time public IP"); - } else { - Logger.printDebug(() -> "Public IP does not match patch time public IP"); - } + Logger.printDebug(() -> passed + ? "Public IP matches patch time public IP" + : "Public IP does not match patch time public IP"); return passed; } }; - + /** + * Injection point. + */ public static void check(Activity context) { - Logger.printDebug(() -> "Running environment checks"); - // If the warning was already issued twice, or if the check was successful in the past, // do not run the checks again. - if (!Check.shouldRun()) { + if (!Check.shouldRun() && !DEBUG_ALWAYS_SHOW_CHECK_FAILED_DIALOG) { Logger.printDebug(() -> "Environment checks are disabled"); return; } - // TODO: There should be a better way to run the checks. Running all in parallel is not ideal, - // because you may run checks that could be exempted by others - // such as isPatchTimePublicIPCheck when isNearPatchTimeCheck was already successful. - // Also, a warning should be issued for failed checks that result in the entire environment check failing, - // but currently, the warning is only issued, when all checks fail. - - if (isNearPatchTimeCheck.run() || isPatchTimeBuildCheck.run()) { - Logger.printDebug(() -> "Passed first checks"); - Check.disableForever(); - return; - } + Utils.runOnBackgroundThread(() -> { + Logger.printDebug(() -> "Running environment checks"); - Logger.printDebug(() -> "Failed first checks"); + if (isNearPatchTimeCheck.run() || isPatchingDeviceSameCheck.run()) { + Logger.printDebug(() -> "Passed build time check"); + if (!DEBUG_ALWAYS_SHOW_CHECK_FAILED_DIALOG) { + Check.disableForever(); + return; + } + } - // TODO: One of the checks probably is sufficient. - if (isManagerInstalledCheck.run() || hasExpectedInstallerCheck.run()) { - Logger.printDebug(() -> "Passed second checks"); - Check.disableForever(); - return; - } + Logger.printDebug(() -> "Failed build time check"); - Logger.printDebug(() -> "Failed second checks"); + // TODO: One of the checks probably is sufficient. + if (isManagerInstalledCheck.run() || hasExpectedInstallerCheck.run()) { + Logger.printDebug(() -> "Passed installation source check"); + if (!DEBUG_ALWAYS_SHOW_CHECK_FAILED_DIALOG) { + Check.disableForever(); + return; + } + } - if (hasPatchTimePublicIPCheck.run()) { - Logger.printDebug(() -> "Passed third checks"); - Check.disableForever(); - return; - } + Logger.printDebug(() -> "Failed second checks"); - Logger.printDebug(() -> "Failed all checks"); + if (hasPatchTimePublicIPCheck.run()) { + Logger.printDebug(() -> "Passed network address check"); + if (!DEBUG_ALWAYS_SHOW_CHECK_FAILED_DIALOG) { + Check.disableForever(); + return; + } + } - Check.issueWarning( - context, - isNearPatchTimeCheck, - isPatchTimeBuildCheck, - isManagerInstalledCheck, - hasExpectedInstallerCheck, - hasPatchTimePublicIPCheck - ); + Logger.printDebug(() -> "Failed all checks"); + + Utils.runOnMainThread(() -> { + Check.issueWarning( + context, + isNearPatchTimeCheck, + isPatchingDeviceSameCheck, + isManagerInstalledCheck, + hasExpectedInstallerCheck, + hasPatchTimePublicIPCheck + ); + }); + }); } private static boolean equalsHash(String value, String hash) { diff --git a/app/src/main/java/app/revanced/integrations/youtube/settings/Settings.java b/app/src/main/java/app/revanced/integrations/youtube/settings/Settings.java index a5c4375679..f219bd265d 100644 --- a/app/src/main/java/app/revanced/integrations/youtube/settings/Settings.java +++ b/app/src/main/java/app/revanced/integrations/youtube/settings/Settings.java @@ -264,7 +264,7 @@ public class Settings extends BaseSettings { public static final IntegerSetting ANNOUNCEMENT_LAST_ID = new IntegerSetting("revanced_announcement_last_id", -1); public static final BooleanSetting CHECK_WATCH_HISTORY_DOMAIN_NAME = new BooleanSetting("revanced_check_watch_history_domain_name", TRUE, false, false); public static final BooleanSetting REMOVE_TRACKING_QUERY_PARAMETER = new BooleanSetting("revanced_remove_tracking_query_parameter", TRUE); - public static final IntegerSetting CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT = new IntegerSetting("revanced_check_environment_warning_issued_count", 0); + public static final IntegerSetting CHECK_ENVIRONMENT_WARNING_ISSUED_COUNT = new IntegerSetting("revanced_check_environment_warning_issued_count", 0, false, false); // Debugging /**