From aa8d27f6792576d091350acac678ac74296cf45a Mon Sep 17 00:00:00 2001 From: schroda <50052685+schroda@users.noreply.github.com> Date: Wed, 26 Nov 2025 03:56:57 +0100 Subject: [PATCH] Fix/backup restore with invalid settings (#1793) * Fix typo in setting validation error message * Convert value to internal type before validating it * Only update setting in case value is valid * Ignore settings validation errors on backup restore * Remove potential not privacy safe value from logs --- .../SettingsBackupSettingsHandlerGenerator.kt | 4 ++-- .../server/settings/SettingDelegate.kt | 6 +++--- .../server/settings/SettingsUpdater.kt | 13 ++++++++++++- .../server/settings/SettingsValidator.kt | 18 +++++++++++++----- 4 files changed, 30 insertions(+), 11 deletions(-) diff --git a/server/server-config-generate/src/main/kotlin/suwayomi/tachidesk/server/settings/generation/SettingsBackupSettingsHandlerGenerator.kt b/server/server-config-generate/src/main/kotlin/suwayomi/tachidesk/server/settings/generation/SettingsBackupSettingsHandlerGenerator.kt index 2f66d542..ab942acf 100644 --- a/server/server-config-generate/src/main/kotlin/suwayomi/tachidesk/server/settings/generation/SettingsBackupSettingsHandlerGenerator.kt +++ b/server/server-config-generate/src/main/kotlin/suwayomi/tachidesk/server/settings/generation/SettingsBackupSettingsHandlerGenerator.kt @@ -35,7 +35,7 @@ object SettingsBackupSettingsHandlerGenerator { appendLine( KotlinFileGeneratorHelper.createImports( listOf( - "suwayomi.tachidesk.graphql.mutations.SettingsMutation", + "suwayomi.tachidesk.server.settings.SettingsUpdater", "suwayomi.tachidesk.manga.impl.backup.BackupFlags", "suwayomi.tachidesk.manga.impl.backup.proto.models.BackupServerSettings", "suwayomi.tachidesk.server.serverConfig", @@ -77,7 +77,7 @@ object SettingsBackupSettingsHandlerGenerator { appendLine("fun restore(backupServerSettings: BackupServerSettings?) {".addIndentation(indentation)) appendLine("if (backupServerSettings == null) { return }".addIndentation(contentIndentation)) appendLine() - appendLine("SettingsMutation().updateSettings(".addIndentation(contentIndentation)) + appendLine("SettingsUpdater.updateAll(".addIndentation(contentIndentation)) appendLine("backupServerSettings.copy(".addIndentation(indentation * 3)) val deprecatedSettings = settings.filter { it.typeInfo.restoreLegacy != null } diff --git a/server/server-config/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingDelegate.kt b/server/server-config/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingDelegate.kt index b8f26963..98fc9f2b 100644 --- a/server/server-config/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingDelegate.kt +++ b/server/server-config/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingDelegate.kt @@ -109,7 +109,7 @@ open class SettingDelegate( val error = validate(initialValue) if (error != null) { KotlinLogging.logger { }.warn { - "Invalid config value ($initialValue) for $moduleName.$propertyName: $error. Using default value: $defaultValue" + "Invalid config value for $moduleName.$propertyName: $error. Using default value: $defaultValue" } stateFlow.value = toValidValue?.let { it(initialValue) } ?: defaultValue @@ -406,8 +406,8 @@ class DisableableDoubleSetting( validator = { value -> when { value == 0.0 -> null - min != null && value < min -> "Value must 0.0 or be at least $min" - max != null && value > max -> "Value must 0.0 or not exceed $max" + min != null && value < min -> "Value must be 0.0 or be at least $min" + max != null && value > max -> "Value must be 0.0 or not exceed $max" else -> null } }, diff --git a/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsUpdater.kt b/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsUpdater.kt index 2146596e..bd2046bd 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsUpdater.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsUpdater.kt @@ -9,6 +9,8 @@ import kotlin.reflect.KProperty1 import kotlin.reflect.full.memberProperties object SettingsUpdater { + private val logger = KotlinLogging.logger { } + private fun updateSetting( name: String, value: Any, @@ -30,12 +32,21 @@ object SettingsUpdater { ?.convertToInternalType ?.invoke(value) ?: value + val validationError = SettingsValidator.validate(name, maybeConvertedValue) + val isValid = validationError == null + + if (!isValid) { + logger.warn { "Invalid value for setting $name: $validationError. Ignoring update." } + + return + } + // Normal update - MigratedConfigValue handles deprecated mappings automatically @Suppress("UNCHECKED_CAST") (stateFlow as MutableStateFlow).value = maybeConvertedValue } } catch (e: Exception) { - KotlinLogging.logger { }.error(e) { "Failed to update setting $name due to" } + logger.error(e) { "Failed to update setting $name due to" } } } diff --git a/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsValidator.kt b/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsValidator.kt index 34d09ed4..0c0443ea 100644 --- a/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsValidator.kt +++ b/server/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsValidator.kt @@ -3,24 +3,32 @@ package suwayomi.tachidesk.server.settings import suwayomi.tachidesk.graphql.types.Settings object SettingsValidator { - private fun validateSingle( + fun validate( name: String, value: Any?, ): String? { val metadata = SettingsRegistry.get(name) ?: return null - return metadata.validator?.invoke(value) + + val maybeConvertedValue = + if (value != null) { + metadata.typeInfo.convertToInternalType?.invoke(value) ?: value + } else { + value + } + + return metadata.validator?.invoke(maybeConvertedValue) } - private fun validateAll( + fun validate( values: Map, ignoreNull: Boolean?, ): List = values .filterValues { value -> ignoreNull == false || value != null } - .mapNotNull { (name, value) -> validateSingle(name, value)?.let { error -> "$name: $error" } } + .mapNotNull { (name, value) -> validate(name, value)?.let { error -> "$name: $error" } } fun validate( settings: Settings, ignoreNull: Boolean = false, - ): List = validateAll(settings.asMap(), ignoreNull) + ): List = validate(settings.asMap(), ignoreNull) }