From 397f93b079fb118c3f17650057f5ad13484b60d3 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 1 Dec 2021 10:28:35 +0100 Subject: [PATCH] Prevent exception from being serialized in ErrorInfo The wrong @Decorator was put in the wrong place to mark the throwable fieldd as transient, now this is fixed and the exception is not serialized. So if a non-serializable throwable is passed, that's not an issue, since it's not going to be serialized. The need for EnsureExceptionSerializable is also gone. --- .../newpipe/error/AcraReportSender.java | 3 +- .../error/EnsureExceptionSerializable.java | 103 ------------------ .../org/schabi/newpipe/error/ErrorInfo.kt | 24 ++-- .../org/schabi/newpipe/error/ErrorUtil.kt | 4 - .../SubscriptionsImportFragment.java | 3 +- .../playererror/PlayerErrorHandler.java | 3 +- .../giga/ui/adapter/MissionAdapter.java | 2 +- 7 files changed, 19 insertions(+), 123 deletions(-) delete mode 100644 app/src/main/java/org/schabi/newpipe/error/EnsureExceptionSerializable.java diff --git a/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java b/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java index bf9030509..4d9966364 100644 --- a/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java +++ b/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java @@ -38,7 +38,6 @@ public class AcraReportSender implements ReportSender { UserAction.UI_ERROR, ErrorInfo.SERVICE_NONE, "ACRA report", - R.string.app_ui_crash, - null)); + R.string.app_ui_crash)); } } diff --git a/app/src/main/java/org/schabi/newpipe/error/EnsureExceptionSerializable.java b/app/src/main/java/org/schabi/newpipe/error/EnsureExceptionSerializable.java deleted file mode 100644 index db94de5e5..000000000 --- a/app/src/main/java/org/schabi/newpipe/error/EnsureExceptionSerializable.java +++ /dev/null @@ -1,103 +0,0 @@ -package org.schabi.newpipe.error; - -import android.util.Log; - -import androidx.annotation.NonNull; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.ObjectOutputStream; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -/** - * Ensures that a Exception is serializable. - * This is - */ -public final class EnsureExceptionSerializable { - private static final String TAG = "EnsureExSerializable"; - - private EnsureExceptionSerializable() { - // No instance - } - - /** - * Ensures that an exception is serializable. - *
- * If that is not the case a {@link WorkaroundNotSerializableException} is created. - * - * @param exception - * @return if an exception is not serializable a new {@link WorkaroundNotSerializableException} - * otherwise the exception from the parameter - */ - public static Exception ensureSerializable(@NonNull final Exception exception) { - return checkIfSerializable(exception) - ? exception - : WorkaroundNotSerializableException.create(exception); - } - - public static boolean checkIfSerializable(@NonNull final Exception exception) { - try { - // Check by creating a new ObjectOutputStream which does the serialization - try (ByteArrayOutputStream bos = new ByteArrayOutputStream(); - ObjectOutputStream oos = new ObjectOutputStream(bos) - ) { - oos.writeObject(exception); - oos.flush(); - - bos.toByteArray(); - } - - return true; - } catch (final IOException ex) { - Log.d(TAG, "Exception is not serializable", ex); - return false; - } - } - - public static class WorkaroundNotSerializableException extends Exception { - protected WorkaroundNotSerializableException( - final Throwable notSerializableException, - final Throwable cause) { - super(notSerializableException.toString(), cause); - setStackTrace(notSerializableException.getStackTrace()); - } - - protected WorkaroundNotSerializableException(final Throwable notSerializableException) { - super(notSerializableException.toString()); - setStackTrace(notSerializableException.getStackTrace()); - } - - public static WorkaroundNotSerializableException create( - @NonNull final Exception notSerializableException - ) { - // Build a list of the exception + all causes - final List throwableList = new ArrayList<>(); - - int pos = 0; - Throwable throwableToProcess = notSerializableException; - - while (throwableToProcess != null) { - throwableList.add(throwableToProcess); - - pos++; - throwableToProcess = throwableToProcess.getCause(); - } - - // Reverse list so that it starts with the last one - Collections.reverse(throwableList); - - // Build exception stack - WorkaroundNotSerializableException cause = null; - for (final Throwable t : throwableList) { - cause = cause == null - ? new WorkaroundNotSerializableException(t) - : new WorkaroundNotSerializableException(t, cause); - } - - return cause; - } - - } -} diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt index 6581b5752..ba1204e12 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt @@ -2,6 +2,7 @@ package org.schabi.newpipe.error import android.os.Parcelable import androidx.annotation.StringRes +import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize import org.schabi.newpipe.R import org.schabi.newpipe.extractor.Info @@ -21,11 +22,14 @@ class ErrorInfo( val userAction: UserAction, val serviceName: String, val request: String, - val messageStringId: Int, - @Transient // no need to store throwable, all data for report is in other variables - var throwable: Throwable? = null + val messageStringId: Int ) : Parcelable { + // no need to store throwable, all data for report is in other variables + // also, the throwable might not be serializable, see TeamNewPipe/NewPipe#7302 + @IgnoredOnParcel + var throwable: Throwable? = null + private constructor( throwable: Throwable, userAction: UserAction, @@ -36,9 +40,10 @@ class ErrorInfo( userAction, serviceName, request, - getMessageStringId(throwable, userAction), - throwable - ) + getMessageStringId(throwable, userAction) + ) { + this.throwable = throwable + } private constructor( throwable: List, @@ -50,9 +55,10 @@ class ErrorInfo( userAction, serviceName, request, - getMessageStringId(throwable.firstOrNull(), userAction), - throwable.firstOrNull() - ) + getMessageStringId(throwable.firstOrNull(), userAction) + ) { + this.throwable = throwable.firstOrNull() + } // constructors with single throwable constructor(throwable: Throwable, userAction: UserAction, request: String) : diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorUtil.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorUtil.kt index e62aad1df..5fb8bff92 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorUtil.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorUtil.kt @@ -20,10 +20,6 @@ class ErrorUtil { /** * Reports a new error by starting a new activity. - *

- * Ensure that the data within errorInfo is serializable otherwise - * an exception will be thrown!

- * [EnsureExceptionSerializable] might help. * * @param context * @param errorInfo diff --git a/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionsImportFragment.java b/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionsImportFragment.java index 8164c656d..8dbd7b2c5 100644 --- a/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionsImportFragment.java +++ b/app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionsImportFragment.java @@ -90,8 +90,7 @@ public class SubscriptionsImportFragment extends BaseFragment { new ErrorInfo(new String[]{}, UserAction.SUBSCRIPTION_IMPORT_EXPORT, NewPipe.getNameOfService(currentServiceId), "Service does not support importing subscriptions", - R.string.general_error, - null)); + R.string.general_error)); activity.finish(); } } diff --git a/app/src/main/java/org/schabi/newpipe/player/playererror/PlayerErrorHandler.java b/app/src/main/java/org/schabi/newpipe/player/playererror/PlayerErrorHandler.java index d77073e00..ca9b4e7df 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playererror/PlayerErrorHandler.java +++ b/app/src/main/java/org/schabi/newpipe/player/playererror/PlayerErrorHandler.java @@ -12,7 +12,6 @@ import androidx.preference.PreferenceManager; import com.google.android.exoplayer2.ExoPlaybackException; import org.schabi.newpipe.R; -import org.schabi.newpipe.error.EnsureExceptionSerializable; import org.schabi.newpipe.error.ErrorInfo; import org.schabi.newpipe.error.ErrorUtil; import org.schabi.newpipe.error.UserAction; @@ -70,7 +69,7 @@ public class PlayerErrorHandler { ErrorUtil.createNotification( context, new ErrorInfo( - EnsureExceptionSerializable.ensureSerializable(exception), + exception, UserAction.PLAY_STREAM, "Player error[type=" + exception.type + "] occurred while playing: " + info.getUrl(), diff --git a/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java b/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java index 569c50001..39bdefbe0 100644 --- a/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java +++ b/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java @@ -583,7 +583,7 @@ public class MissionAdapter extends Adapter implements Handler.Callb ErrorUtil.createNotification(mContext, new ErrorInfo(ErrorInfo.Companion.throwableToStringList(mission.errObject), action, - service, request.toString(), reason, null)); + service, request.toString(), reason)); } public void clearFinishedDownloads(boolean delete) {