fix(reader): sanitize KOReader device download paths

Use KOReader's destination filesystem rules for each server-provided path segment so Kindle downloads reject invalid names without changing Linux-compatible names. Preserve bulk collision handling after sanitization.

Fixes #1203

koreader-plugin/bookorbit.koplugin/bookorbit_catalog_download.lua

@@ -28,12 +28,16 @@ local safeFilenameBase = CatalogUtil.safeFilenameBase
 
 local CatalogDownload = {}
 
-local function sanitizeDevicePath(device_path)
+local function sanitizeDevicePath(device_path, download_dir)
     local normalized = tostring(device_path or ""):gsub("\\", "/"):gsub("^/+", "")
     local segments = {}
     for segment in normalized:gmatch("[^/]+") do
         if segment == ".." then return nil end
-        if segment ~= "." then table.insert(segments, segment) end
+        if segment ~= "." then
+            local safe_segment = util.getSafeFilename(segment, download_dir)
+            if not safe_segment or safe_segment == "" or safe_segment == "." or safe_segment == ".." then return nil end
+            table.insert(segments, safe_segment)
+        end
     end
     if #segments == 0 then return nil end
     return table.concat(segments, "/")
@@ -44,7 +48,7 @@ local function joinDownloadPath(download_dir, relative_path)
 end
 
 local function resolveRelativeDownloadPath(download_dir, filename, filetype, device_path, filename_override)
-    local relative = sanitizeDevicePath(device_path)
+    local relative = sanitizeDevicePath(device_path, download_dir)
     if relative and not filename_override then return relative end
 
     local parent = relative and relative:match("^(.*)/[^/]+$")

koreader-plugin/bookorbit.koplugin/main.lua

@@ -48,7 +48,7 @@ local BookOrbitProgressSync = require("bookorbit_progress_sync")
 local BookOrbitSweep = require("bookorbit_sweep")
 local BookOrbitUpdater = require("bookorbit_updater")
 
-local PLUGIN_VERSION = "1.5.1"
+local PLUGIN_VERSION = "1.5.2"
 
 local SYNC_STRATEGY = {
     PROMPT = 1,

koreader-plugin/spec/bookorbit_catalog_bulk_download_test.lua

@@ -92,7 +92,7 @@ package.loaded["util"] = {
         table.insert(made_paths, path)
     end,
     getSafeFilename = function(filename)
-        return filename
+        return filename:gsub("[\\/:*?\"<>|]", "_")
     end,
     removeFile = function(path)
         files[path] = nil
@@ -188,9 +188,14 @@ G_reader_settings = {
 assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "Series/Book.epub"), "/downloads/Series/Book.epub", "valid device path")
 assertEqual(made_paths[1], "/downloads/Series", "valid device parent created")
 assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "./Series//Book.epub"), "/downloads/Series/Book.epub", "dot segments normalized")
+assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "Standalone/Book: One.epub"), "/downloads/Standalone/Book_ One.epub", "device filename is filesystem safe")
+assertEqual(made_paths[3], "/downloads/Standalone", "sanitized device filename keeps its parent")
+assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "Series: One/Book? One.epub"), "/downloads/Series_ One/Book_ One.epub", "every device path segment is filesystem safe")
+assertEqual(made_paths[4], "/downloads/Series_ One", "sanitized device parent is created")
+made_paths = {}
 assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "../outside.epub"), "/downloads/fallback.epub", "parent traversal falls back")
 assertEqual(Catalog:getLocalDownloadPath("fallback", "epub", "Series\\..\\..\\outside.epub"), "/downloads/fallback.epub", "mixed separator traversal falls back")
-assertEqual(#made_paths, 2, "traversal paths do not create directories")
+assertEqual(#made_paths, 0, "traversal paths do not create directories")
 
 Catalog.settings = {}
 Catalog:initBulkDownloadState()
@@ -608,12 +613,12 @@ local collision_ctx = {
     pending_matches = {},
     completed = {},
 }
-local function collisionBook(id, title)
+local function collisionBook(id, title, device_path)
     return {
         id = id,
         title = title,
         formats = { "epub" },
-        files = { { id = 100 + id, format = "epub", devicePath = "Shared/Book.epub" } },
+        files = { { id = 100 + id, format = "epub", devicePath = device_path } },
     }
 end
 Catalog.bulkShowStatus = function() end
@@ -621,21 +626,21 @@ Catalog.bulkDownloadFile = function(_, _, _, _, local_path)
     table.insert(transfers, local_path)
     return true
 end
-Catalog:bulkProcessBook(collision_ctx, collisionBook(1, "First"))
+Catalog:bulkProcessBook(collision_ctx, collisionBook(1, "First", "Shared/Book: One.epub"))
 collision_ctx.index = 2
-Catalog:bulkProcessBook(collision_ctx, collisionBook(2, "Second"))
+Catalog:bulkProcessBook(collision_ctx, collisionBook(2, "Second", "Shared/Book? One.epub"))
 collision_ctx.index = 3
-Catalog:bulkProcessBook(collision_ctx, collisionBook(3, "Third"))
+Catalog:bulkProcessBook(collision_ctx, collisionBook(3, "Third", "Shared/Book* One.epub"))
 
-assertEqual(transfers[1], "/downloads/Shared/Book.epub", "first collision path remains unchanged")
-assertEqual(transfers[2], "/downloads/Shared/Book [2].epub", "second collision path gains book identity")
-assertEqual(transfers[3], "/downloads/Shared/Book [3].epub", "third collision path gains book identity")
+assertEqual(transfers[1], "/downloads/Shared/Book_ One.epub", "first sanitized collision path remains unchanged")
+assertEqual(transfers[2], "/downloads/Shared/Book_ One [2].epub", "second sanitized collision path gains book identity")
+assertEqual(transfers[3], "/downloads/Shared/Book_ One [3].epub", "third sanitized collision path gains book identity")
 assertEqual(collision_ctx.counts.downloaded, 3, "all colliding books download")
 assertEqual(collision_ctx.counts.path_conflicts, 2, "collision count recorded")
 assertEqual(collision_ctx.path_conflicts[1].conflicting_book_id, 1, "collision owner recorded")
-assertEqual(collision_ctx.path_conflicts[1].resolved_path, "/downloads/Shared/Book [2].epub", "resolved collision path recorded")
-assertEqual(collision_ctx.destination_paths["/downloads/shared/book.epub"].book_id, 1, "original path ownership retained")
-assertEqual(collision_ctx.completed["2"], "/downloads/Shared/Book [2].epub", "checkpoint records the renamed destination")
+assertEqual(collision_ctx.path_conflicts[1].resolved_path, "/downloads/Shared/Book_ One [2].epub", "resolved collision path recorded")
+assertEqual(collision_ctx.destination_paths["/downloads/shared/book_ one.epub"].book_id, 1, "original path ownership retained")
+assertEqual(collision_ctx.completed["2"], "/downloads/Shared/Book_ One [2].epub", "checkpoint records the renamed destination")
 
 Catalog.bulkReleaseStandby = function() end
 Catalog:bulkFinish(collision_ctx)

koreader-plugin/spec/bookorbit_catalog_download_test.lua

@@ -66,8 +66,11 @@ package.loaded["util"] = {
     makePath = function(path)
         table.insert(made_paths, path)
     end,
-    getSafeFilename = function(filename)
-        return filename:gsub("[\\/:*?\"<>|]", "_")
+    getSafeFilename = function(filename, path)
+        if path and path:match("^/mnt/") then
+            return filename:gsub("[\\/:*?\"<>|]", "_")
+        end
+        return filename
     end,
     trim = function(value)
         return tostring(value or ""):match("^%s*(.-)%s*$")
@@ -131,6 +134,27 @@ local resolved = Catalog:getLocalDownloadPath(detail.fallbackFilename, file.form
 assertEqual(resolved, "/mnt/onboard/library/Series/Southern Reach/1.00 - Annihilation.epub", "configured destination")
 assertEqual(made_paths[1], "/mnt/onboard/library/Series/Southern Reach", "configured parent created")
 
+made_paths = {}
+resolved = Catalog:getLocalDownloadPath(
+    "Bret Easton Ellis - The Shards: A Novel",
+    "epub",
+    "Standalone/Bret Easton Ellis - The Shards: A Novel.epub"
+)
+assertEqual(resolved, "/mnt/onboard/library/Standalone/Bret Easton Ellis - The Shards_ A Novel.epub", "device filename is filesystem safe")
+assertEqual(made_paths[1], "/mnt/onboard/library/Standalone", "sanitized filename keeps configured parent")
+
+made_paths = {}
+resolved = Catalog:getLocalDownloadPath("Fallback", "epub", "Series: One/Book? One.epub")
+assertEqual(resolved, "/mnt/onboard/library/Series_ One/Book_ One.epub", "every device path segment is filesystem safe")
+assertEqual(made_paths[1], "/mnt/onboard/library/Series_ One", "sanitized device parent is created")
+
+download_dir = "/library"
+made_paths = {}
+resolved = Catalog:getLocalDownloadPath("Fallback", "epub", "Standalone/Book: One.epub")
+assertEqual(resolved, "/library/Standalone/Book: One.epub", "permissive filesystem keeps supported characters")
+assertEqual(made_paths[1], "/library/Standalone", "permissive filesystem keeps configured parent")
+download_dir = "/mnt/onboard/library"
+
 made_paths = {}
 resolved = Catalog:getLocalDownloadPath("Preferred/Name", "EPUB", file.devicePath, true)
 assertEqual(resolved, "/mnt/onboard/library/Series/Southern Reach/Preferred_Name.epub", "filename override preserves configured folder")