Fix/invalid server settings gql mutation request (#1092)

* Validate setting values on mutation * Handle invalid negative setting values * Ensure at least one source is downloading at all times * Prevent possible IllegalArgumentException The "serverConfig.maxSourcesInParallel" value could have changed after the if-condition
Suwayomi · Nov 14, 2024 · aa1e985 · aa1e985
1 parent fa4607e
commit aa1e985
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 19 deletions.
diff --git a/AndroidCompat/Config/src/main/java/xyz/nulldev/ts/config/Logging.kt b/AndroidCompat/Config/src/main/java/xyz/nulldev/ts/config/Logging.kt
@@ -60,7 +60,7 @@ private fun createRollingFileAppender(
             context = logContext
             setParent(appender)
             fileNamePattern = "$logDirPath/${logFilename}_%d{yyyy-MM-dd}_%i.log.gz"
-            maxHistory = maxFiles
+            maxHistory = maxFiles.coerceAtLeast(0)
             setMaxFileSize(fileSizeValueOfOrDefault(maxFileSize, "10mb"))
             setTotalSizeCap(fileSizeValueOfOrDefault(maxTotalSize, "100mb"))
             start()

diff --git a/server/src/main/kotlin/suwayomi/tachidesk/graphql/mutations/SettingsMutation.kt b/server/src/main/kotlin/suwayomi/tachidesk/graphql/mutations/SettingsMutation.kt
@@ -4,29 +4,47 @@ import kotlinx.coroutines.flow.MutableStateFlow
 import suwayomi.tachidesk.graphql.types.PartialSettingsType
 import suwayomi.tachidesk.graphql.types.Settings
 import suwayomi.tachidesk.graphql.types.SettingsType
+import suwayomi.tachidesk.manga.impl.extension.ExtensionsList.repoMatchRegex
 import suwayomi.tachidesk.server.SERVER_CONFIG_MODULE_NAME
 import suwayomi.tachidesk.server.ServerConfig
 import suwayomi.tachidesk.server.serverConfig
 import xyz.nulldev.ts.config.GlobalConfigManager
+import java.io.File
 
-private fun validateString(
-    value: String?,
-    pattern: Regex,
-    name: String,
+private fun validateValue(
+    exception: Exception,
+    validate: () -> Boolean,
 ) {
-    validateString(value, pattern, Exception("Invalid format for \"$name\" [$value]"))
+    if (!validate()) {
+        throw exception
+    }
 }
 
-private fun validateString(
-    value: String?,
-    pattern: Regex,
+private fun <T> validateValue(
+    value: T?,
     exception: Exception,
+    validate: (value: T) -> Boolean,
 ) {
-    if (value != null && !value.matches(pattern)) {
-        throw exception
+    if (value != null) {
+        validateValue(exception) { validate(value) }
     }
 }
 
+private fun <T> validateValue(
+    value: T?,
+    name: String,
+    validate: (value: T) -> Boolean,
+) {
+    validateValue(value, Exception("Invalid value for \"$name\" [$value]"), validate)
+}
+
+private fun validateFilePath(
+    value: String?,
+    name: String,
+) {
+    validateValue(value, name) { File(it).exists() }
+}
+
 class SettingsMutation {
     data class SetSettingsInput(
         val clientMutationId: String? = null,
@@ -39,10 +57,43 @@ class SettingsMutation {
     )
 
     private fun validateSettings(settings: Settings) {
+        validateValue(settings.ip, "ip") { it.matches("^((25[0-5]|(2[0-4]|1\\d|[1-9]|)\\d)\\.?\\b){4}$".toRegex()) }
+
+        // proxy
+        validateValue(settings.socksProxyVersion, "socksProxyVersion") { it == 4 || it == 5 }
+
+        // webUI
+        validateFilePath(settings.electronPath, "electronPath")
+        validateValue(settings.webUIUpdateCheckInterval, "webUIUpdateCheckInterval") { it == 0.0 || it in 1.0..23.0 }
+
+        // downloader
+        validateFilePath(settings.downloadsPath, "downloadsPath")
+        validateValue(settings.autoDownloadNewChaptersLimit, "autoDownloadNewChaptersLimit") { it >= 0 }
+
+        // extensions
+        validateValue(settings.extensionRepos, "extensionRepos") { it.all { repoUrl -> repoUrl.matches(repoMatchRegex) } }
+
+        // requests
+        validateValue(settings.maxSourcesInParallel, "maxSourcesInParallel") { it in 1..20 }
+
+        // updater
+        validateValue(settings.globalUpdateInterval, "globalUpdateInterval") { it == 0.0 || it >= 6 }
+
         // misc
-        val logbackSizePattern = "^[0-9]+(|kb|KB|mb|MB|gb|GB)\$".toRegex()
-        validateString(settings.maxLogFileSize, logbackSizePattern, "maxLogFileSize")
-        validateString(settings.maxLogFolderSize, logbackSizePattern, "maxLogFolderSize")
+        validateValue(settings.maxLogFiles, "maxLogFiles") { it >= 0 }
+
+        val logbackSizePattern = "^[0-9]+(|kb|KB|mb|MB|gb|GB)$".toRegex()
+        validateValue(settings.maxLogFileSize, "maxLogFolderSize") { it.matches(logbackSizePattern) }
+        validateValue(settings.maxLogFolderSize, "maxLogFolderSize") { it.matches(logbackSizePattern) }
+
+        // backup
+        validateFilePath(settings.backupPath, "backupPath")
+        validateValue(settings.backupTime, "backupTime") { it.matches("^([0-1]?[0-9]|2[0-3]):[0-5][0-9]$".toRegex()) }
+        validateValue(settings.backupInterval, "backupInterval") { it == 0 || it >= 1 }
+        validateValue(settings.backupTTL, "backupTTL") { it == 0 || it >= 1 }
+
+        // local source
+        validateFilePath(settings.localSourcePath, "localSourcePath")
     }
 
     private fun <SettingType : Any> updateSetting(

diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/Chapter.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/Chapter.kt
@@ -400,7 +400,7 @@ object Chapter {
             if (serverConfig.autoDownloadNewChaptersLimit.value == 0) {
                 chaptersToConsiderForDownloadLimit.size
             } else {
-                serverConfig.autoDownloadNewChaptersLimit.value.coerceAtMost(chaptersToConsiderForDownloadLimit.size)
+                serverConfig.autoDownloadNewChaptersLimit.value.coerceIn(0, chaptersToConsiderForDownloadLimit.size)
             }
         val limitedChaptersToDownload = chaptersToConsiderForDownloadLimit.subList(0, latestChapterToDownloadIndex)
         val limitedChaptersToDownloadWithDuplicates =

diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/DownloadManager.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/download/DownloadManager.kt
@@ -241,14 +241,14 @@ object DownloadManager {
                         "Failed: ${downloadQueue.size - availableDownloads.size}"
                 }
 
-                if (runningDownloaders.size < serverConfig.maxSourcesInParallel.value) {
+                if (runningDownloaders.size < serverConfig.maxSourcesInParallel.value.coerceAtLeast(1)) {
                     availableDownloads
                         .asSequence()
                         .map { it.manga.sourceId }
                         .distinct()
                         .minus(
                             runningDownloaders.map { it.sourceId }.toSet(),
-                        ).take(serverConfig.maxSourcesInParallel.value - runningDownloaders.size)
+                        ).take((serverConfig.maxSourcesInParallel.value - runningDownloaders.size).coerceAtLeast(0))
                         .map { getDownloader(it) }
                         .forEach {
                             it.start()

diff --git a/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/extension/ExtensionsList.kt b/server/src/main/kotlin/suwayomi/tachidesk/manga/impl/extension/ExtensionsList.kt
@@ -224,7 +224,7 @@ object ExtensionsList {
             this
         }
 
-    private val repoMatchRegex =
+    val repoMatchRegex =
         (
             "https:\\/\\/(?>www\\.|raw\\.)?(github|githubusercontent)\\.com" +
                 "\\/([^\\/]+)\\/([^\\/]+)(?>(?>\\/tree|\\/blob)?\\/([^\\/\\n]*))?(?>\\/([^\\/\\n]*\\.json)?)?"

diff --git a/server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt b/server/src/main/kotlin/suwayomi/tachidesk/server/util/WebInterfaceManager.kt
@@ -228,7 +228,7 @@ object WebInterfaceManager {
     private fun getServedWebUIFlavor(): WebUIFlavor =
         WebUIFlavor.from(preferences.getString(SERVED_WEBUI_FLAVOR_KEY, WebUIFlavor.default.uiName)!!)
 
-    private fun isAutoUpdateEnabled(): Boolean = serverConfig.webUIUpdateCheckInterval.value.toInt() != 0
+    private fun isAutoUpdateEnabled(): Boolean = serverConfig.webUIUpdateCheckInterval.value.toInt() > 0
 
     @OptIn(DelicateCoroutinesApi::class)
     private fun scheduleWebUIUpdateCheck() {