fix(reader): allow KOReader downloads from filesystem root

Preserve the filesystem root during destination normalization so valid downloads reach the transfer callback. Report local preflight failures accurately and cover root publishing and traversal rejection.

Closes #1177
This commit is contained in:
neon
2026-08-28 05:00:57 +00:00
parent a51122237d
commit 7eb33761c7
6 changed files with 95 additions and 6 deletions
@@ -832,7 +832,10 @@ function BookOrbitCatalog:showServerError(err)
end end
function BookOrbitCatalog:errorText(err) function BookOrbitCatalog:errorText(err)
if isAuthError(err) then local transfer_error = CatalogUtil.transferErrorText(err)
if transfer_error then
return transfer_error
elseif isAuthError(err) then
return _("BookOrbit login failed. Check your KOReader credentials.") return _("BookOrbit login failed. Check your KOReader credentials.")
elseif type(err) == "number" then elseif type(err) == "number" then
return T(_("BookOrbit server error (%1)."), err) return T(_("BookOrbit server error (%1)."), err)
@@ -386,6 +386,15 @@ function CatalogUtil.safeFilenameBase(detail)
return base return base
end end
function CatalogUtil.transferErrorText(err)
if err == "unsafe_destination" then
return _("The download destination is outside the selected folder.")
elseif err == "temp_dir_failed" then
return _("Could not create a temporary download folder in the selected folder.")
end
return nil
end
function CatalogUtil.coverLabel(book) function CatalogUtil.coverLabel(book)
local lines = {} local lines = {}
local title = book and book.title or _("Untitled") local title = book and book.title or _("Untitled")
@@ -31,11 +31,14 @@ Transfer.TEMP_DIR_NAME = TEMP_DIR_NAME
Transfer.POLL_INTERVAL = POLL_INTERVAL Transfer.POLL_INTERVAL = POLL_INTERVAL
local function normalize(path) local function normalize(path)
return tostring(path or ""):gsub("\\", "/"):gsub("/+", "/"):gsub("/$", "") local normalized = tostring(path or ""):gsub("\\", "/"):gsub("/+", "/")
if normalized == "/" then return normalized end
return normalized:gsub("/$", "")
end end
function Transfer.tempDir(root) function Transfer.tempDir(root)
return normalize(root) .. "/" .. TEMP_DIR_NAME local normalized_root = normalize(root)
return (normalized_root ~= "/" and normalized_root or "") .. "/" .. TEMP_DIR_NAME
end end
function Transfer.ensureTempDir(root) function Transfer.ensureTempDir(root)
@@ -52,7 +55,11 @@ function Transfer.isInsideRoot(root, path)
local normalized_root = normalize(root) local normalized_root = normalize(root)
local normalized_path = normalize(path) local normalized_path = normalize(path)
if normalized_root == "" or normalized_path == "" then return false end if normalized_root == "" or normalized_path == "" then return false end
if normalized_path:sub(1, #normalized_root + 1) ~= normalized_root .. "/" then return false end if normalized_root == "/" then
if normalized_path == "/" or normalized_path:sub(1, 1) ~= "/" then return false end
elseif normalized_path:sub(1, #normalized_root + 1) ~= normalized_root .. "/" then
return false
end
for segment in normalized_path:gmatch("[^/]+") do for segment in normalized_path:gmatch("[^/]+") do
if segment == ".." then return false end if segment == ".." then return false end
end end
+1 -1
View File
@@ -48,7 +48,7 @@ local BookOrbitProgressSync = require("bookorbit_progress_sync")
local BookOrbitSweep = require("bookorbit_sweep") local BookOrbitSweep = require("bookorbit_sweep")
local BookOrbitUpdater = require("bookorbit_updater") local BookOrbitUpdater = require("bookorbit_updater")
local PLUGIN_VERSION = "1.5.0" local PLUGIN_VERSION = "1.5.1"
local SYNC_STRATEGY = { local SYNC_STRATEGY = {
PROMPT = 1, PROMPT = 1,
@@ -0,0 +1,42 @@
local function identity(value)
return value
end
package.loaded["gettext"] = identity
package.loaded["document/documentregistry"] = {
hasProvider = function()
return true
end,
}
package.loaded["util"] = {
fixUtf8 = identity,
}
package.loaded["ffi/util"] = {
template = function(value)
return value
end,
}
package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path
local CatalogUtil = require("bookorbit_catalog_util")
local function assertEqual(actual, expected, label)
if actual ~= expected then
error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual)))
end
end
assertEqual(
CatalogUtil.transferErrorText("unsafe_destination"),
"The download destination is outside the selected folder.",
"unsafe destinations are reported as local path failures"
)
assertEqual(
CatalogUtil.transferErrorText("temp_dir_failed"),
"Could not create a temporary download folder in the selected folder.",
"temporary directory failures are reported as local filesystem failures"
)
assertEqual(CatalogUtil.transferErrorText("network_error"), nil, "network errors keep the existing connection message")
print("bookorbit_catalog_transfer_error_test.lua: ok")
@@ -10,7 +10,10 @@ package.loaded["ui/uimanager"] = {
end, end,
} }
local directories = { ["/downloads/.bookorbit-tmp"] = true } local directories = {
["/.bookorbit-tmp"] = true,
["/downloads/.bookorbit-tmp"] = true,
}
local files = {} local files = {}
local now = 1000 local now = 1000
package.loaded["libs/libkoreader-lfs"] = { package.loaded["libs/libkoreader-lfs"] = {
@@ -66,10 +69,16 @@ end
-- Destination containment is checked before anything is written and again -- Destination containment is checked before anything is written and again
-- before the publishing rename. -- before the publishing rename.
assertEqual(Transfer.isInsideRoot("/downloads", "/downloads/Books/a.epub"), true, "a path under the root is authorized") assertEqual(Transfer.isInsideRoot("/downloads", "/downloads/Books/a.epub"), true, "a path under the root is authorized")
assertEqual(Transfer.isInsideRoot("/downloads", "/downloads/The Shards.epub"), true,
"a file directly under the download root is authorized")
assertEqual(Transfer.isInsideRoot("/downloads", "/elsewhere/a.epub"), false, "a path outside the root is rejected") assertEqual(Transfer.isInsideRoot("/downloads", "/elsewhere/a.epub"), false, "a path outside the root is rejected")
assertEqual(Transfer.isInsideRoot("/downloads", "/downloads/../etc/passwd"), false, "traversal is rejected") assertEqual(Transfer.isInsideRoot("/downloads", "/downloads/../etc/passwd"), false, "traversal is rejected")
assertEqual(Transfer.isInsideRoot("/downloads", "/downloads"), false, "the root itself is not a destination") assertEqual(Transfer.isInsideRoot("/downloads", "/downloads"), false, "the root itself is not a destination")
assertEqual(Transfer.tempDir("/downloads"), "/downloads/.bookorbit-tmp", "temporaries live under the destination root") assertEqual(Transfer.tempDir("/downloads"), "/downloads/.bookorbit-tmp", "temporaries live under the destination root")
assertEqual(Transfer.isInsideRoot("/", "/The Shards.epub"), true, "the filesystem root supports a direct book destination")
assertEqual(Transfer.isInsideRoot("/", "/Standalone/The Shards.epub"), true, "the filesystem root supports nested destinations")
assertEqual(Transfer.isInsideRoot("/", "/../etc/passwd"), false, "the filesystem root still rejects traversal")
assertEqual(Transfer.tempDir("/"), "/.bookorbit-tmp", "the filesystem root gets one temporary directory separator")
-- Leftovers from a killed run are removed, and the sweep stays bounded. -- Leftovers from a killed run are removed, and the sweep stays bounded.
files["/downloads/.bookorbit-tmp/bo_1_1.part"] = { size = 10, modified = 1 } files["/downloads/.bookorbit-tmp/bo_1_1.part"] = { size = 10, modified = 1 }
@@ -141,6 +150,25 @@ runScheduled()
assertEqual(#scheduled, 0, "a poll that fires after completion does not reschedule itself") assertEqual(#scheduled, 0, "a poll that fires after completion does not reschedule itself")
assertEqual(#observed, 1, "no progress is reported once the transfer returned") assertEqual(#observed, 1, "no progress is reported once the transfer returned")
-- A user-selected filesystem root used to normalize to an empty string and
-- reject the destination before perform could start the HTTP request.
local root_request_started = false
local root_completed = Transfer.run{
root = "/",
destination = "/The Shards.epub",
generation = 5,
expected_bytes = 2089926,
is_current = function() return true end,
perform = function(download_opts)
root_request_started = true
files[download_opts.temp_path] = { size = 2089926, modified = now }
return { temp_path = download_opts.temp_path, bytes = 2089926 }
end,
}
assertEqual(root_request_started, true, "the reported book reaches the HTTP transfer callback")
assertEqual(root_completed, true, "the reported book publishes under a filesystem-root download folder")
assertEqual(files["/The Shards.epub"] ~= nil, true, "the reported book is published at the requested destination")
-- A cancelled generation discards a late child result instead of publishing it. -- A cancelled generation discards a late child result instead of publishing it.
renames = {} renames = {}
removed = {} removed = {}