Commit 189bdcd8 authored by WorldTeacher's avatar WorldTeacher
Browse files

fix(review): prevent multi-arg log data loss in util.redactUrls

parent 9e9d5f23
Loading
Loading
Loading
Loading
+4 −0
Original line number Diff line number Diff line
@@ -33,6 +33,10 @@ jobs:
            msgfmt "$po" -o "$mo"
          done

      - name: Strip comments from Lua sources
        run: |
          python3 scripts/luastripper.py --remove-empty-lines --in-place bookloresync.koplugin

      - name: Create plugin archive
        run: |
          zip -r bookloresync.koplugin.zip bookloresync.koplugin/ -x "*.git*" "*.zip"
+1 −1
Original line number Diff line number Diff line
@@ -15,5 +15,5 @@ return {
    name = "bookloresync",
    fullname = _("Booklore Sync"),
    description = _([[Sync your reading sessions to Booklore server.]]),
    version = "0.0.0-dev+179f0b9",
    version = "0.0.0-dev",
}
+0 −167
Original line number Diff line number Diff line
@@ -2099,173 +2099,6 @@ function APIClient:downloadBook(book_id, save_path, username, password)
    return false, err_msg
end

--[[--
Download a book file with progress reporting and cancellation support.

Uses a custom LuaSocket sink that counts bytes written per chunk and
optionally reports progress and checks for cancellation.

@param book_id   number  Book ID to download
@param save_path string  Destination file path
@param username  string  Booklore username (for Bearer auth)
@param password  string  Booklore password
@param opts      table|nil  Optional table:
  on_progress = function(bytes_written, total_bytes)  — called at most ~1/s
  is_cancelled = function() → bool                     — called per chunk
@return boolean success
@return string|nil error or nil on success
--]]
---APIClient:downloadBookWithProgress.
function APIClient:downloadBookWithProgress(book_id, save_path, username, password, opts)
    opts = opts or {}
    self:logInfo("BookloreSync API: downloadBookWithProgress - id:", book_id, "->", save_path)

    local token_ok, token = self:getOrRefreshBearerToken(username, password)
    if not token_ok then
        self:logErr("BookloreSync API: downloadBookWithProgress - auth failed:", token)
        return false, "Authentication failed"
    end

    local url = self.server_url .. "/api/v1/books/" .. book_id .. "/download"
    local http_client = url:match("^https://") and https or http

    -- Write to a temp file, then rename to avoid partial reads.
    local tmp_path = save_path .. ".tmp"
    local file, open_err = io.open(tmp_path, "wb")
    if not file then
        self:logErr("BookloreSync API: downloadBookWithProgress - cannot open temp file:", open_err)
        return false, "Cannot create file: " .. tostring(open_err)
    end

    local bytes_written = 0
    local on_progress  = opts.on_progress
    local is_cancelled = opts.is_cancelled
    local last_progress_time = 0
    local file_closed = false
    local function close_file()
        if not file_closed then
            file_closed = true
            file:close()
        end
    end

    -- Custom sink: write chunk, track bytes, report progress, check cancel.
    local function progress_sink(chunk, sink_err)
        if chunk == nil then
            close_file()
            return nil
        end
        if chunk == "" then
            return 1
        end
        if is_cancelled and is_cancelled() then
            close_file()
            os.remove(tmp_path)
            return nil, "cancelled"
        end
        local write_ok, write_err = file:write(chunk)
        if not write_ok then
            close_file()
            return nil, write_err
        end
        bytes_written = bytes_written + #chunk
        local now = os.time()
        if on_progress and now ~= last_progress_time then
            last_progress_time = now
            on_progress(bytes_written, nil)
        end
        return 1
    end

    local req_args = {
        url     = url,
        method  = "GET",
        headers = { ["Authorization"] = "Bearer " .. token },
        sink    = progress_sink,
    }
    http_client.TIMEOUT = self.timeout

    local res, code, _ = http_client.request(req_args)

    -- Retry on 401/403
    if type(code) == "number" and (code == 401 or code == 403) then
        self:logWarn("BookloreSync API: downloadBookWithProgress - token rejected, refreshing")
        if self.db then self.db:deleteBearerToken(username) end
        local ref_ok, new_token = self:getOrRefreshBearerToken(username, password, true)
        if ref_ok then
            close_file()
            bytes_written = 0
            file, open_err = io.open(tmp_path, "wb")
            if file then
                file_closed = false
                req_args.headers = { ["Authorization"] = "Bearer " .. new_token }
                -- Recreate sink for the fresh file
                local function retry_sink(chunk, sink_err)
                    if chunk == nil then
                        close_file()
                        return nil
                    end
                    if chunk == "" then
                        return 1
                    end
                    if is_cancelled and is_cancelled() then
                        close_file()
                        os.remove(tmp_path)
                        return nil, "cancelled"
                    end
                    local write_ok2, write_err2 = file:write(chunk)
                    if not write_ok2 then
                        close_file()
                        return nil, write_err2
                    end
                    bytes_written = bytes_written + #chunk
                    local now2 = os.time()
                    if on_progress and now2 ~= last_progress_time then
                        last_progress_time = now2
                        on_progress(bytes_written, nil)
                    end
                    return 1
                end
                req_args.sink = retry_sink
                res, code, _ = http_client.request(req_args)
            end
        end
    end

    close_file()

    -- Check cancellation first
    if is_cancelled and is_cancelled() then
        os.remove(tmp_path)
        return false, "Download cancelled"
    end

    if not code then
        os.remove(tmp_path)
        local err_msg = "Network error: " .. tostring(res)
        self:logErr("BookloreSync API: downloadBookWithProgress failed:", err_msg)
        return false, err_msg
    end

    if type(code) == "number" and code >= 200 and code < 300 then
        -- Final progress report at 100%
        if on_progress and bytes_written > 0 then
            on_progress(bytes_written, bytes_written)
        end
        local rename_ok, rename_err = os.rename(tmp_path, save_path)
        if not rename_ok then
            os.remove(tmp_path)
            return false, "Failed to save file: " .. tostring(rename_err)
        end
        self:logInfo("BookloreSync API: downloadBookWithProgress - success,", bytes_written, "bytes")
        return true, nil
    end

    os.remove(tmp_path)
    local err_msg = "HTTP " .. tostring(code) .. ": download failed"
    self:logErr("BookloreSync API: downloadBookWithProgress failed:", err_msg)
    return false, err_msg
end

--[[--
Get the file size of a book by making an HTTP HEAD request.
+4 −3
Original line number Diff line number Diff line
@@ -48,9 +48,8 @@ function M.new(deps)
        return util.generateFilename(book, use_original)
    end

    -- Hash a file the same way BookloreSync:calculateBookHash does.
    -- Filename generation — same logic as module.generateFilename but inline
    -- to avoid self-dependency (used inside the download loop).
    -- generateFilename delegates to util.generateFilename (shared logic).
    -- File hashing uses util.calculateBookHash inside the download loop.
    function module.syncFromBookloreShelf(self, silent, on_complete)
    if silent == nil then silent = (self.ui and self.ui.document and true or false) end

@@ -136,6 +135,8 @@ function M.new(deps)
            local filled = math.floor(pct / 100 * bar_w)
            if filled > bar_w then filled = bar_w end
            lines[#lines + 1] = ""
            -- Block characters: █ (U+2588) = filled, ░ (U+2591) = empty.
            -- Rendered as UTF-8 byte sequences for KOReader's e-ink font stack.
            local bar = string.rep("\226\150\136", filled) .. string.rep("\226\150\145", bar_w - filled)
            lines[#lines + 1] = bar
            lines[#lines + 1] = ""
+19 −27
Original line number Diff line number Diff line
@@ -2229,11 +2229,11 @@ function BookloreSync:getEntryChangedAt(entry)
    if not entry then
        return nil
    end
    local updated_at = self:parseKoreaderDateTime(entry.datetime_updated)
    local updated_at = self:_parseISO8601(entry.datetime_updated)
    if updated_at then
        return updated_at
    end
    return self:parseKoreaderDateTime(entry.datetime)
    return self:_parseISO8601(entry.datetime)
end

function BookloreSync:filterEntriesBySessionStart(entries, session_start_time)
@@ -6215,10 +6215,23 @@ function BookloreSync:normalizeSessionLocationWithFallback(location, koreader_pa
end

--[[--
Resolve server pagecount for a pending session.

@param session table
@return number|nil
Resolve the server page count for a pending session.

Resolution order (first non-nil wins):

1. **Session field** — checks `session.server_pagecount` / `session.serverPagecount`
   directly. Returns immediately without DB or API calls.
2. **In-memory cache** — checks the per-session LRU cache
   (`self._server_pagecount_session_cache`) keyed by hash or book ID.
3. **Database** — looks up the book in `book_cache` by hash then by book_id,
   returning the stored `server_pagecount`.
4. **API refresh** — calls `self:getBookIdByHash()` which fetches fresh metadata
   from the server. Only attempted when step 3 returns nothing useful.
5. **Negative cache** — stores `false` in the session cache so repeated calls
   for the same book skip steps 2-4 on subsequent lookups.

@param session table  Pending-session row with bookHash / bookId fields
@return number|nil    Server page count, or nil if unresolvable
--]]
---BookloreSync:getServerPagecountForSession.
function BookloreSync:getServerPagecountForSession(session)
@@ -9840,27 +9853,6 @@ function BookloreSync:_unixToISO8601(timestamp)
    return require("booklore_util").unixToISO8601(timestamp)
end

---BookloreSync:_parseISO8601.
function BookloreSync:_parseISO8601(iso_string)
    if not iso_string then return nil end
    
    local year, month, day, hour, min, sec = iso_string:match(
        "(%d+)-(%d+)-(%d+)%a(%d+):(%d+):(%d+)"
    )
    
    if not year then return nil end
    
    return os.time({
        year = tonumber(year),
        month = tonumber(month),
        day = tonumber(day),
        hour = tonumber(hour),
        min = tonumber(min),
        sec = tonumber(sec),
        isdst = false,
    })
end

---BookloreSync:_detectBookType.
function BookloreSync:_detectBookType(book)
    local title = book.title or ""