Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
75 changes: 72 additions & 3 deletions modules/updater/updater.lua
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ 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 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 +140,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 +206,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 +241,69 @@ function Updater.error(message)
Updater.abort()
end
end

-- IMPORTANTE: A Funcao changeUrl foi criada pelo Claude Opus 4.5
-- Nao foi testada pois nao utilizo client mobile, por favor verificar seu funcionamento e corrigir possiveis erros.
-- Esta funcao era chamada no updater.otui porem nao havia implementacao em Lua, por conta disso solicitei ao Claude que a criasse.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
function Updater.changeUrl()
if not updaterWindow then return end

-- Pause the update process
removeEvent(scheduledEvent)
HTTP.cancel(httpOperationId)

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
Services.updater = newUrl
dialog:destroy()
-- Restart updater with new URL
if updaterWindow then
updaterWindow:destroy()
updaterWindow = nil
end
Updater.check()
end
end

local cancelButton = g_ui.createWidget('Button', buttonBox)
cancelButton:setText(tr('Cancel'))
cancelButton:setWidth(80)
cancelButton.onClick = function()
dialog:destroy()
-- Resume update process
Updater.check()
end
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

dialog:show()
dialog:focus()
dialog:raise()
textEdit:focus()
end
71 changes: 50 additions & 21 deletions tools/api/updater.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ function sendError($error) {
}

$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;
Expand All @@ -41,39 +41,68 @@ function sendError($error) {
}
if(!$cache) { // update cache
$dir = realpath($files_dir);
$rii = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($dir));
$cache = array();
foreach ($rii as $file) {
if (!$dir || !is_dir($dir)) {
sendError("Server configuration error: files directory not found");
}

// 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) {
$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 === true || $checksum != "") {
$parsed_checksum = ltrim($checksum, '0');
if ($parsed_checksum === '') {
$parsed_checksum = '0';
if ($checksum !== false && $checksum !== "") {
$parsed_checksum = ltrim($checksum, '0');
if ($parsed_checksum === '') {
$parsed_checksum = '0';
}
$cache[$path] = $parsed_checksum;
}
$cache[$path] = $parsed_checksum;
}
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);
}
Comment on lines +49 to 98

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)) {
$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));
header("Content-Type: application/json");
header("Content-Length: " . strlen($body));
header("X-Content-Type-Options: nosniff");
header("Cache-Control: no-store");
echo($body);

?>