Skip to content

fix: client updater #1632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LaercioLeal wants to merge 7 commits into
base: main
Choose a base branch
from
Open

fix: client updater #1632

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
47b759a
Fix: client updater
LaercioLeal Feb 4, 2026
07b9f44
fix: potential issues
LaercioLeal Feb 4, 2026
63390e7
Fix: HTTP cancellation into the OK and Restart button handlers
LaercioLeal Feb 4, 2026
e17abfb
Fix: Restart button won’t actually restart while updaterWindow exists
LaercioLeal Feb 4, 2026
dd08712
Fix: Adding changeUrl guard flag for mobile testing
LaercioLeal Feb 4, 2026
4a35f31
Merge branch 'main' into fix-autoupdater
majestyotbr Jul 31, 2026
799a015
feat(updater): restrict local file scan to server roots and optimize …
LaercioLeal Aug 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 105 additions & 3 deletions modules/updater/updater.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ local function updateFiles(data, keepCurrentFiles)

-- update binary
local binary = nil
if type(data.binary) == "table" and data.binary.file:len() > 1 then
if type(data.binary) == "table"
and type(data.binary.file) == "string" and data.binary.file:len() > 1
and type(data.binary.checksum) == "string" and data.binary.checksum:len() > 0 then
local selfChecksum = g_resources.selfChecksum()
if selfChecksum:len() > 0 and selfChecksum ~= data.binary.checksum then
binary = data.binary.file
Expand Down Expand Up @@ -140,7 +142,9 @@ local function updateFiles(data, keepCurrentFiles)
updaterWindow.downloadProgress:setPercent(0)
updaterWindow.downloadProgress:show()
updaterWindow.downloadStatus:show()
updaterWindow.changeUrlButton:hide()
if updaterWindow.changeUrlButton then
updaterWindow.changeUrlButton:hide()
end

downloadFiles(data["url"], toUpdate, 1, 0, function()
updaterWindow.status:setText(tr("Updating client (may take few seconds)"))
Expand Down Expand Up @@ -204,12 +208,13 @@ function Updater.check(args)
updaterWindow:raise()

local updateData = nil
local allowCustomServers = ALLOW_CUSTOM_SERVERS or false
local function progressUpdater(value)
removeEvent(scheduledEvent)
if value == 100 then
return Updater.error(tr("Timeout"))
end
if updateData and (value > 60 or (not g_platform.isMobile() or not ALLOW_CUSTOM_SERVERS or not loadModulesFunc)) then -- gives 3s to set custom updater for mobile version
if updateData and (value > 60 or (not g_platform.isMobile() or not allowCustomServers or not loadModulesFunction)) then -- gives 3s to set custom updater for mobile version
return updateFiles(updateData)
end
scheduledEvent = scheduleEvent(function() progressUpdater(value + 1) end, 100)
Expand Down Expand Up @@ -238,3 +243,100 @@ function Updater.error(message)
Updater.abort()
end
end

-- TODO: [MOBILE-TEST] Mobile testing required for changeUrl functionality
-- Feature flag to enable/disable changeUrl (set to false until mobile testing is complete)
Updater.enableChangeUrl = false

-- Test Checklist for changeUrl on mobile:
-- [ ] 1. Verify dialog opens correctly on mobile screen sizes
-- [ ] 2. Test TextEdit input works with mobile keyboard
-- [ ] 3. Confirm OK button saves URL with trailing slash and restarts updater
-- [ ] 4. Confirm Restart button restarts updater without changing URL
-- [ ] 5. Verify dialog closes properly when buttons are clicked
-- [ ] 6. Test with valid and invalid URLs (less than 5 characters)
-- [ ] 7. Verify update process cancellation works correctly

function Updater.changeUrl()
if not updaterWindow then
return
end

-- Guard: only allow if feature flag is enabled
if not Updater.enableChangeUrl then
g_logger.warning("Updater.changeUrl is disabled. Set Updater.enableChangeUrl = true to enable.")
return
end

local dialog = g_ui.createWidget('MainWindow', rootWidget)
dialog:setId('changeUrlDialog')
dialog:setText(tr('Change Updater URL'))
dialog:setSize({
width = 400,
height = 120
})

local layout = g_ui.createWidget('VerticalBox', dialog)
layout:setId('layout')
layout:addAnchor(AnchorTop, 'parent', AnchorTop)
layout:addAnchor(AnchorLeft, 'parent', AnchorLeft)
layout:addAnchor(AnchorRight, 'parent', AnchorRight)
layout:setMarginTop(30)
layout:setMarginLeft(10)
layout:setMarginRight(10)

local textEdit = g_ui.createWidget('TextEdit', layout)
textEdit:setId('urlInput')
textEdit:setText(Services.updater or '')
textEdit:setHeight(20)

local buttonBox = g_ui.createWidget('HorizontalBox', layout)
buttonBox:setMarginTop(10)
buttonBox:setHeight(25)

local okButton = g_ui.createWidget('Button', buttonBox)
okButton:setText(tr('OK'))
okButton:setWidth(80)
okButton:setMarginRight(5)
okButton.onClick = function()
local newUrl = textEdit:getText()
if newUrl and newUrl:len() > 4 then
-- Cancel current update process before restarting with new URL
removeEvent(scheduledEvent)
HTTP.cancel(httpOperationId)
-- Normalize URL to ensure trailing slash
if not newUrl:match("/$") then
newUrl = newUrl .. "/"
end
Services.updater = newUrl
dialog:destroy()
-- Restart updater with new URL
if updaterWindow then
updaterWindow:destroy()
updaterWindow = nil
end
Updater.check()
end
end

local restartButton = g_ui.createWidget('Button', buttonBox)
restartButton:setText(tr('Restart'))
restartButton:setWidth(80)
restartButton.onClick = function()
-- Cancel current update process before restarting
removeEvent(scheduledEvent)
HTTP.cancel(httpOperationId)
dialog:destroy()
if updaterWindow then
updaterWindow:destroy()
updaterWindow = nil
end
-- Restart update process from beginning
Updater.check()
end

dialog:show()
dialog:focus()
dialog:raise()
textEdit:focus()
end
2 changes: 1 addition & 1 deletion modules/updater/updater.otui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ StaticMainWindow
margin-top: 5
margin-bottom: 10
!text: tr('Change updater URL')
@onClick: Updater.changeUrl()
@onClick: if Updater.enableChangeUrl then Updater.changeUrl() end
$!mobile:
visible: false

Expand Down
111 changes: 75 additions & 36 deletions tools/api/updater.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,63 +17,102 @@
);
// CONFIG END

function sendError($error) {
echo(json_encode(array("error" => $error)));
die();
function sendError($error)
{
echo (json_encode(array("error" => $error)));
die();
}

$data = json_decode(file_get_contents("php://input"));
//if(!$data) {
// sendError("Invalid input data");
//}
if (!$data || !is_object($data)) {
sendError("Invalid input data");
}

$version = $data->version ?: 0; // APP_VERSION from init.lua
$build = $data->build ?: ""; // 2.4, 2.4.1, 2.5, etc
$os = $data->os ?: "unknown"; // android, windows, mac, linux, unknown
$platform = $data->platform ?: ""; // WIN32-WGL, X11-GLX, ANDROID-EGL, etc
$args = $data->args; // custom args when calling Updater.check()
$binary = $binaries[$platform] ?: "";
$version = $data->version ?? 0; // APP_VERSION from init.lua
$build = $data->build ?? ""; // 2.4, 2.4.1, 2.5, etc
$os = $data->os ?? "unknown"; // android, windows, mac, linux, unknown
$platform = $data->platform ?? ""; // WIN32-WGL, X11-GLX, ANDROID-EGL, etc
$args = $data->args ?? []; // custom args when calling Updater.check()
$binary = $binaries[$platform] ?? "";

$cache = null;
$cache_file = sys_get_temp_dir() . DIRECTORY_SEPARATOR . $checksum_file;
if (file_exists($cache_file) && (filemtime($cache_file) + $checksum_update_interval > time())) {
$cache = json_decode(file_get_contents($cache_file), true);
}
if(!$cache) { // update cache
if (!$cache) { // update cache
$dir = realpath($files_dir);
$rii = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir));
$cache = array();
foreach ($rii as $file) {
if (!$file->isFile())
continue;
$path = str_replace($dir, '', $file->getPathname());
$path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
$checksum = hash_file("crc32b", $file->getPathname());
if (!$dir || !is_dir($dir)) {
sendError("Server configuration error: files directory not found");
}

if ($checksum === true || $checksum != "") {
$parsed_checksum = ltrim($checksum, '0');
if ($parsed_checksum === '') {
$parsed_checksum = '0';
// File locking to prevent race conditions
$lock_file = $cache_file . ".lock";
$lock = fopen($lock_file, "w");
if (!$lock) {
sendError("Server error: could not create lock file");
}

try {
if (!flock($lock, LOCK_EX | LOCK_NB)) {
// Another process is updating cache, wait and use existing cache
usleep(100000); // 100ms
if (file_exists($cache_file)) {
$cache = json_decode(file_get_contents($cache_file), true);
}
if (!$cache) {
// Wait for the lock if cache still not available
flock($lock, LOCK_EX);
// Re-check cache after acquiring lock
if (file_exists($cache_file)) {
$cache = json_decode(file_get_contents($cache_file), true);
}
}
$cache[$path] = $parsed_checksum;
}

if (!$cache) {
$rii = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir));
$cache = array();
foreach ($rii as $file) {
if (!$file->isFile())
continue;
$path = str_replace($dir, '', $file->getPathname());
$path = str_replace(DIRECTORY_SEPARATOR, '/', $path);
$checksum = hash_file("crc32b", $file->getPathname());

if ($checksum !== false && $checksum !== "") {
$parsed_checksum = ltrim($checksum, '0');
if ($parsed_checksum === '') {
$parsed_checksum = '0';
}
$cache[$path] = $parsed_checksum;
}
}
file_put_contents($cache_file . ".tmp", json_encode($cache));
rename($cache_file . ".tmp", $cache_file);
}
} finally {
// Release lock and close handle - always executes
flock($lock, LOCK_UN);
fclose($lock);
}
Comment on lines +49 to 98

@coderabbitai coderabbitai Bot Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Race condition: cache can be built without lock protection.

When flock() fails to acquire the lock (another process is updating), the code closes $lock on line 53, waits 100ms, then tries to read the cache. If the cache still doesn't exist at line 60, it proceeds to build the cache without holding any lock, defeating the locking mechanism.

Additionally, if this path is taken and cache is built, line 82 calls flock($lock, LOCK_UN) on an already-closed resource.

🐛 Proposed fix to maintain lock throughout cache building 
     // File locking to prevent race conditions
     $lock_file = $cache_file . ".lock";
     $lock = fopen($lock_file, "w");
     if (!flock($lock, LOCK_EX | LOCK_NB)) {
         // Another process is updating cache, wait and use existing cache
-        fclose($lock);
         usleep(100000); // 100ms
         if (file_exists($cache_file)) {
             $cache = json_decode(file_get_contents($cache_file), true);
         }
+        if (!$cache) {
+            // Wait for the lock if cache still not available
+            flock($lock, LOCK_EX);
+            // Re-check cache after acquiring lock
+            if (file_exists($cache_file)) {
+                $cache = json_decode(file_get_contents($cache_file), true);
+            }
+        }
     }
     
     if (!$cache) {
         $rii = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir));
         $cache = array(); 
         foreach ($rii as $file) {
         // ... rest of cache building ...
         }
         file_put_contents($cache_file . ".tmp", json_encode($cache));
         rename($cache_file . ".tmp", $cache_file);
-        
-        // Release lock
-        flock($lock, LOCK_UN);
     }
-    if (isset($lock) && is_resource($lock)) {
-        fclose($lock);
-    }
+    // Release lock and close handle
+    flock($lock, LOCK_UN);
+    fclose($lock);
 }
🤖 Prompt for AI Agents 
In `@tools/api/updater.php` around lines 48 - 86, The cache-building path
currently proceeds without holding the lock when non-blocking flock() fails;
change the logic so that when acquiring the lock via $lock (created from
$lock_file) fails with LOCK_EX | LOCK_NB you wait/retry and then acquire a
blocking flock() before proceeding to build the cache; ensure $lock remains open
for the entire cache creation/atomic write (file_put_contents(... ".tmp") +
rename(...)) and only call flock($lock, LOCK_UN) and fclose($lock) after the
rename, and guard any flock/ fclose calls with an isset(is_resource($lock))
check to avoid unlocking/closing an already-closed resource.

file_put_contents($cache_file . ".tmp", json_encode($cache));
rename($cache_file . ".tmp", $cache_file);
}
$ret = array("url" => $files_url, "files" => array(), "keepFiles" => false);
foreach($cache as $file => $checksum) {
$base = trim(explode("/", ltrim($file, "/"))[0]);
if(in_array($base, $files_and_dirs)) {
foreach ($cache as $file => $checksum) {
$base = trim(explode("/", ltrim($file, "/"))[0]);
if (in_array($base, $files_and_dirs)) {
$ret["files"][$file] = $checksum;
}
if($base == $binary && !empty($binary)) {
// Use basename to correctly match binary filename regardless of path
$filename = basename($file);
if ($filename == $binary && !empty($binary)) {
$ret["binary"] = array("file" => $file, "checksum" => $checksum);
}
}

$body = json_encode($ret, JSON_PRETTY_PRINT);
header("Content-length: " . strlen($body));
echo($body);

?>
header("Content-Type: application/json");
header("Content-Length: " . strlen($body));
header("X-Content-Type-Options: nosniff");
header("Cache-Control: no-store");
echo ($body);