glpi-project · cedric-anne · Jun 2, 2025 · May 28, 2025
diff --git a/.phpstan-baseline.php b/.phpstan-baseline.php
@@ -4579,12 +4579,6 @@
 	'count' => 3,
 	'path' => __DIR__ . '/src/Glpi/Controller/DropdownFormController.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Function ini_set is unsafe to use\\. It can return FALSE instead of throwing an exception\\. Please add \'use function Safe\\\\ini_set;\' at the beginning of the file to use the variant provided by the \'thecodingmachine/safe\' library\\.$#',
-	'identifier' => 'theCodingMachineSafe.function',
-	'count' => 2,
-	'path' => __DIR__ . '/src/Glpi/Controller/InstallController.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Function unlink is unsafe to use\\. It can return FALSE instead of throwing an exception\\. Please add \'use function Safe\\\\unlink;\' at the beginning of the file to use the variant provided by the \'thecodingmachine/safe\' library\\.$#',
 	'identifier' => 'theCodingMachineSafe.function',
@@ -5995,18 +5989,6 @@
 	'count' => 3,
 	'path' => __DIR__ . '/src/Glpi/Progress/AbstractProgressIndicator.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Function ini_set is unsafe to use\\. It can return FALSE instead of throwing an exception\\. Please add \'use function Safe\\\\ini_set;\' at the beginning of the file to use the variant provided by the \'thecodingmachine/safe\' library\\.$#',
-	'identifier' => 'theCodingMachineSafe.function',
-	'count' => 1,
-	'path' => __DIR__ . '/src/Glpi/Progress/ProgressStorage.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Function session_write_close is unsafe to use\\. It can return FALSE instead of throwing an exception\\. Please add \'use function Safe\\\\session_write_close;\' at the beginning of the file to use the variant provided by the \'thecodingmachine/safe\' library\\.$#',
-	'identifier' => 'theCodingMachineSafe.function',
-	'count' => 1,
-	'path' => __DIR__ . '/src/Glpi/Progress/ProgressStorage.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Call to function is_array\\(\\) with array will always evaluate to true\\.$#',
 	'identifier' => 'function.alreadyNarrowedType',

diff --git a/install/install.php b/install/install.php
@@ -345,15 +345,12 @@ public function __construct($dbh)
         echo '</div>';
         echo '</div>';
 
-        echo \sprintf(
-            <<<HTML
-                <script defer type="module">
-                    import { init_database } from '/js/modules/GlpiInstall.js';
-                    init_database("%s");
-                </script>
-HTML,
-            \Glpi\Controller\InstallController::PROGRESS_KEY_INIT_DATABASE,
-        );
+        echo <<<HTML
+            <script defer type="module">
+                import { init_database } from '/js/modules/GlpiInstall.js';
+                init_database();
+            </script>
+        HTML;
     } else { // can't create config_db file
         echo "<p>" . __s('Impossible to write the database setup file') . "</p>";
         $prev_form($host, $user, $password);

diff --git a/install/update.php b/install/update.php
@@ -184,15 +184,12 @@ function showSecurityKeyCheckForm()
             echo '</div>';
             echo '</div>';
 
-            echo \sprintf(
-                <<<HTML
-                    <script defer type="module">
-                        import { update_database } from '/js/modules/GlpiInstall.js';
-                        update_database("%s");
-                    </script>
-HTML,
-                \Glpi\Controller\InstallController::PROGRESS_KEY_UPDATE_DATABASE,
-            );
+            echo <<<HTML
+                <script defer type="module">
+                    import { update_database } from '/js/modules/GlpiInstall.js';
+                    update_database();
+                </script>
+            HTML;
         }
     } else {
         echo "<h3>";

diff --git a/js/modules/GlpiInstall.js b/js/modules/GlpiInstall.js
@@ -34,7 +34,7 @@
 
 import { ProgressIndicator } from '/js/modules/ProgressIndicator.js';
 
-export function init_database(progress_key)
+export function init_database()
 {
     const messages_container = document.getElementById('glpi_install_messages_container');
     const success_container = document.getElementById('glpi_install_success');
@@ -54,7 +54,6 @@ export function init_database(progress_key)
     );
 
     const progress_indicator = new ProgressIndicator({
-        key: progress_key,
         container: messages_container,
         request: request,
         success_callback: () => {
@@ -70,7 +69,7 @@ export function init_database(progress_key)
     progress_indicator.start();
 }
 
-export async function update_database(progress_key)
+export async function update_database()
 {
     const messages_container = document.getElementById('glpi_update_messages_container');
     const success_container = document.getElementById('glpi_update_success');
@@ -89,7 +88,6 @@ export async function update_database(progress_key)
     );
 
     const progress_indicator = new ProgressIndicator({
-        key: progress_key,
         container: messages_container,
         request: request,
         success_callback: () => {

diff --git a/js/modules/ProgressIndicator.js b/js/modules/ProgressIndicator.js
@@ -84,30 +84,24 @@ export class ProgressIndicator
 
     /**
      * @param parameters
-     * @param {string} parameters.key Mandatory. The progress indicator unique key.
      * @param {HTMLElement} parameters.container Mandatory. The HTML container that will contain the progress indicator.
      * @param {Request} parameters.request Mandatory. The request corresponding to the operation to execute.
      * @param {Function} parameters.success_callback The function that will be called if the operation succeed.
      * @param {Function} parameters.error_callback The function that will be called if the operation fails.
      */
     constructor({
-        key,
         container,
         request,
         success_callback = () => {},
         error_callback = () => {},
     }) {
-        if (!key) {
-            throw new Error('`key` key is mandatory.');
-        }
         if (!(container instanceof HTMLElement)) {
             throw new Error(`\`container\` must be an \`HTMLElement\`, "${container?.constructor?.name || typeof container}" found.`);
         }
         if (!(request instanceof Request)) {
             throw new Error(`\`request\` must be a \`Request\`, "${request?.constructor?.name || typeof request}" found.`);
         }
 
-        this.#key = key;
         this.#container = container;
         this.#request = request;
         this.#success_callback = success_callback;
@@ -135,21 +129,23 @@ export class ProgressIndicator
         `;
         this.#container.appendChild(self_dom_el);
 
+        const response = await fetch(this.#request);
+
+        // Read the first chunk only (the one that contains the progress indicator key).
+        // Reading the full response will make the fetch API to wait for the initial request to be fully processed,
+        // because even if the browser consider the reponse as fully received, the TCP connection is still alive
+        // until the end of the operation, and the fetch API waits for the TCP connection to be closed
+        // to consider the response as fully received.
+        const encoded_key = (await response.body.getReader().read()).value;
+
+        this.#key = (new TextDecoder()).decode(encoded_key);
+
         setTimeout(
             () => {
                 this.#check_progress();
             },
             this.#refresh_timeout
         );
-
-        try {
-            await fetch(this.#request);
-        } catch {
-            // DB installation is really long and can result in a `Proxy timeout` error.
-            // It does not mean that the process is killed, it just mean that the proxy did not wait for the response
-            // and send an error to the client.
-            // Here we catch any error to make it silent, but we will handle it with the progress indicator error_callback.
-        }
     }
 
     /**
@@ -186,13 +182,13 @@ export class ProgressIndicator
      */
     async #check_progress() {
         try {
-            const res = await fetch(`${CFG_GLPI.root_doc}/progress/check/${this.#key}`);
+            const response = await fetch(`${CFG_GLPI.root_doc}/progress/check/${this.#key}`);
 
-            if (res.status >= 400) {
-                throw new Error(`Error response from server with code "${res.status.toString()}".`);
+            if (response.status >= 400) {
+                throw new Error(`Error response from server with code "${response.status.toString()}".`);
             }
 
-            const json =  await res.json();
+            const json = await response.json();
 
             this.#update_progress_bar(json['current_step'], json['max_steps'], json['progress_bar_message']);
 

diff --git a/phpunit/functional/Glpi/Progress/ProgressStorageTest.php b/phpunit/functional/Glpi/Progress/ProgressStorageTest.php
@@ -0,0 +1,195 @@
+<?php
+
+/**
+ * ---------------------------------------------------------------------
+ *
+ * GLPI - Gestionnaire Libre de Parc Informatique
+ *
+ * http://glpi-project.org
+ *
+ * @copyright 2015-2025 Teclib' and contributors.
+ * @licence   https://www.gnu.org/licenses/gpl-3.0.html
+ *
+ * ---------------------------------------------------------------------
+ *
+ * LICENSE
+ *
+ * This file is part of GLPI.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ * ---------------------------------------------------------------------
+ */
+
+namespace tests\units\Glpi\Log;
+
+use Glpi\Progress\ProgressStorage;
+use Glpi\Progress\StoredProgressIndicator;
+use GLPITestCase;
+use org\bovigo\vfs\vfsStream;
+
+class ProgressStorageTest extends GLPITestCase
+{
+    public function testSpawnProgressIndicator(): void
+    {
+        // Arrange
+        vfsStream::setup(
+            'glpi',
+            null,
+            [
+                'files' => [
+                    '_tmp' => [
+                    ],
+                ],
+            ]
+        );
+        $storage = new ProgressStorage(vfsStream::url('glpi/files/_tmp'));
+
+        // Act
+        $progress_indicator = $storage->spawnProgressIndicator();
+
+        // Assert
+        $this->assertInstanceOf(StoredProgressIndicator::class, $progress_indicator);
+        $expected_file_path = vfsStream::url('glpi/files/_tmp/' . $progress_indicator->getStorageKey() . '.progress');
+        $this->assertFileExists($expected_file_path);
+
+        $stored_content = \file_get_contents($expected_file_path);
+        $this->assertEquals(\serialize($progress_indicator), $stored_content);
+    }
+
+    public function testSaveAndGetProgressIndicator(): void
+    {
+        // Arrange
+        $storage_key = \session_id() . 'abcdef0123456789';
+
+        vfsStream::setup(
+            'glpi',
+            null,
+            [
+                'files' => [
+                    '_tmp' => [
+                    ],
+                ],
+            ]
+        );
+        $storage = new ProgressStorage(vfsStream::url('glpi/files/_tmp'));
+        $progress_indicator = new StoredProgressIndicator($storage, $storage_key);
+
+        // Act
+        $storage->save($progress_indicator);
+        $fetched_progress_indicator = $storage->getProgressIndicator($storage_key);
+
+        // Assert
+        $this->assertEquals($progress_indicator, $fetched_progress_indicator);
+    }
+
+    public function testGetProgressIndicatorThatDoesNotExists(): void
+    {
+        // Arrange
+        $storage_key = \session_id() . 'abcdef0123456789';
+
+        vfsStream::setup(
+            'glpi',
+            null,
+            [
+                'files' => [
+                    '_tmp' => [
+                    ],
+                ],
+            ]
+        );
+        $storage = new ProgressStorage(vfsStream::url('glpi/files/_tmp'));
+
+        // Act
+        $fetched_progress_indicator = $storage->getProgressIndicator($storage_key);
+
+        // Assert
+        $this->assertNull($fetched_progress_indicator);
+    }
+
+    public function testGetProgressIndicatorFromAnotherSession(): void
+    {
+        // Arrange
+        $another_sess_id = '55066aedc96e8e49533b45362d124840';
+        \assert($another_sess_id !== \session_id());
+
+        $storage_key_suffix = 'abcdef0123456789';
+
+        $structure = vfsStream::setup(
+            'glpi',
+            null,
+            [
+                'files' => [
+                    '_tmp' => [
+                    ],
+                ],
+            ]
+        );
+        $storage = new ProgressStorage(vfsStream::url('glpi/files/_tmp'));
+
+        foreach ([$another_sess_id, \session_id()] as $prefix) {
+            $progress_indicator = new StoredProgressIndicator($storage, $prefix . $storage_key_suffix);
+
+            vfsStream::newFile($prefix . $storage_key_suffix . '.progress')
+                ->at($structure->getChild('files/_tmp'))
+                ->setContent(\serialize($progress_indicator));
+        }
+
+        // Act
+        $another_session_progress_indicator = $storage->getProgressIndicator($another_sess_id . $storage_key_suffix);
+        $current_session_progress_indicator = $storage->getProgressIndicator(\session_id() . $storage_key_suffix);
+
+        // Assert
+        $this->assertNull($another_session_progress_indicator, 'Progress indicator from another session should not be readable.');
+        $this->assertInstanceOf(StoredProgressIndicator::class, $current_session_progress_indicator);
+    }
+
+    public function testSaveProgressIndicatorFromAnotherSession(): void
+    {
+        // Arrange
+        $another_sess_id = '55066aedc96e8e49533b45362d124840';
+        \assert($another_sess_id !== \session_id());
+
+        $storage_key = $another_sess_id . 'abcdef0123456789';
+
+        vfsStream::setup(
+            'glpi',
+            null,
+            [
+                'files' => [
+                    '_tmp' => [
+                    ],
+                ],
+            ]
+        );
+        $storage = new ProgressStorage(vfsStream::url('glpi/files/_tmp'));
+        $progress_indicator = new StoredProgressIndicator($storage, $storage_key);
+
+        // Act
+        $exception = null;
+        try {
+            $storage->save($progress_indicator);
+        } catch (\Throwable $e) {
+            $exception = $e;
+        }
+        $fetched_progress_indicator = $storage->getProgressIndicator($storage_key);
+
+        // Assert
+        $this->assertInstanceOf(\Throwable::class, $exception);
+        $this->assertNull($fetched_progress_indicator, 'Progress indicator from another session should not be readable.');
+
+        $expected_file_path = vfsStream::url('glpi/files/_tmp/' . $progress_indicator->getStorageKey() . '.progress');
+        $this->assertFileDoesNotExist($expected_file_path);
+    }
+}
diff --git a/phpunit/functional/Glpi/Progress/StoredProgressIndicatorTest.php b/phpunit/functional/Glpi/Progress/StoredProgressIndicatorTest.php
@@ -48,7 +48,6 @@ public function testConstructor(): void
         $storage_key = $this->getUniqueString();
 
         $storage = $this->createMock(ProgressStorage::class);
-        $storage->expects($this->once())->method('save'); // Instance constructor must trigger its own saving into the storage
 
         $date = new DateTimeImmutable();