service timeouts + no signals

Author: medilies

src/Dompurify/DompurifyService.php

@@ -58,14 +58,27 @@ public function start(): static
             $this->config->port,
         ]);
 
+        $this->serviceProcess->setIdleTimeout($this->config->startupTimeout / 1000);
+
         $this->serviceProcess->start();
 
+        // ? rm check
+        if (! $this->serviceProcess->isRunning()) {
+            throw new ProcessFailedException($this->serviceProcess);
+        }
+
         $this->serviceProcess->waitUntil(function (string $type, string $buffer) {
-            // ? timeout
-            // ! ensure service always returns output
-            return strlen($buffer) > 5;
+            if ($type === Process::ERR) {
+                throw new ProcessFailedException($this->serviceProcess);
+            }
+
+            // Must output when service is listening
+            return strlen($buffer) > 4;
         });
 
+        $this->serviceProcess->setIdleTimeout(null);
+
+        // ? rm check
         if (! $this->serviceProcess->isRunning()) {
             throw new ProcessFailedException($this->serviceProcess);
         }
@@ -75,8 +88,23 @@ public function start(): static
 
     public function stop(): static
     {
-        if ($this->isRunning()) {
-            $this->serviceProcess->stop();
+        $this->serviceProcess->stop();
+
+        if (is_null($this->config->shutdownTimeout)) {
+            return $this;
+        }
+
+        $waitUntilMicroSec = \hrtime(true) + $this->config->shutdownTimeout * 1000;
+
+        $sleep = 10_000_000; // 10us
+
+        // TODO: test
+        while ($this->isRunning()) {
+            if (\hrtime(true) + $sleep >= $waitUntilMicroSec) {
+                throw new XsslessException('Shutdown timed out');
+            }
+
+            \usleep($sleep);
         }
 
         return $this;
@@ -96,53 +124,4 @@ public function getIncrementalErrorOutput(): string
     {
         return $this->serviceProcess->getIncrementalErrorOutput();
     }
-
-    public function throwIfFailedOnTerm(): void
-    {
-        if ($this->serviceProcess->isRunning()) {
-            // ? stop it
-            throw new XsslessException('The service is still running');
-        }
-
-        // TODO: fix windows check
-        // https://github.com/medilies/xssless/actions/runs/10288495452/job/28474063301#step:7:26
-        if ($this->isSigTerm() || $this->isWindows()) {
-            return;
-        }
-
-        throw new ProcessFailedException($this->serviceProcess);
-    }
-
-    // ? interface
-    public function waitForTermination(int $timeout): void
-    {
-        $elapsed = 0;
-        $sleepInterval = 100; // Sleep for 100 milliseconds
-
-        while ($this->serviceProcess->isRunning() && $elapsed < $timeout) {
-            usleep($sleepInterval * 1000);
-            $elapsed += $sleepInterval;
-        }
-
-        if ($this->serviceProcess->isRunning()) {
-            throw new XsslessException('Process did not terminate within the given timeout');
-        }
-    }
-
-    // ========================================================================
-
-    private function isWindows(): bool
-    {
-        return strtoupper(substr(PHP_OS, 0, 3)) === 'WIN';
-    }
-
-    private function isSigTerm(): bool
-    {
-        return $this->serviceProcess->getTermSignal() === 15;
-    }
-
-    // private function isSigHup(): bool
-    // {
-    //     return $this->serviceProcess->getTermSignal() === 1;
-    // }
 }

src/Dompurify/DompurifyServiceConfig.php

@@ -14,6 +14,8 @@ public function __construct(
         public string $host = '127.0.0.1',
         public int $port = 63000,
         public ?string $binary = null,
+        public int $startupTimeout = 6000, // ms
+        public ?int $shutdownTimeout = null // ms
     ) {
         $this->class = DompurifyService::class;
     }

src/ServiceInterface.php

@@ -17,6 +17,4 @@ public function isRunning(): bool;
     public function getIncrementalOutput(): string;
 
     public function getIncrementalErrorOutput(): string;
-
-    public function throwIfFailedOnTerm(): void;
 }

src/laravel/Commands/StartCommand.php

@@ -24,6 +24,7 @@ public function handle(): void
             exit;
         };
 
+        // ? Is this necessary
         pcntl_signal(SIGTERM, $terminate);
         pcntl_signal(SIGINT, $terminate);
 
@@ -43,7 +44,5 @@ public function handle(): void
             // Sleep for a short period to avoid busy-waiting
             usleep(100_000);
         }
-
-        $service->throwIfFailedOnTerm();
     }
 }

tests/Dompurify/DompurifyCliTest.php

@@ -14,6 +14,22 @@
     expect(fn () => $cleaner->exec('foo'))->toThrow(ProcessFailedException::class);
 });
 
+it('throws when cannot find binary file', function () {
+    $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
+        binary: __DIR__.'/js-mocks/x.js',
+    ));
+
+    expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
+});
+
+it('throws when cannot locate temp folder', function () {
+    $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
+        tempFolder: __DIR__.'/x',
+    ));
+
+    expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
+});
+
 test('setup()', function () {
     $cleaner = (new Xssless)->using(new DompurifyCliConfig);
 
@@ -44,20 +60,4 @@
     ));
 
     expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
-})->depends('setup()');
-
-it('throws when cannot find binary file', function () {
-    $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
-        binary: __DIR__.'/js-mocks/x.js',
-    ));
-
-    expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
-})->depends('setup()');
-
-it('throws when cannot locate temp folder', function () {
-    $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
-        tempFolder: __DIR__.'/x',
-    ));
-
-    expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
-})->depends('setup()');
+});

tests/Dompurify/DompurifyServiceTest.php

@@ -5,6 +5,44 @@
 use Medilies\Xssless\Dompurify\DompurifyServiceConfig;
 use Medilies\Xssless\Xssless;
 use Symfony\Component\Process\Exception\ProcessFailedException;
+use Symfony\Component\Process\Exception\ProcessTimedOutException;
+
+it('throws on bad node path', function () {
+    $service = (new DompurifyService)->configure(new DompurifyServiceConfig(
+        node: 'nodeZz',
+    ));
+
+    expect(fn () => $service->start())->toThrow(ProcessFailedException::class);
+
+    // expect($service->serviceProcess->getExitCode())->toBe(127);
+});
+
+it('throws when cannot find binary file', function () {
+    $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig(
+        binary: __DIR__.'/js-mocks/x.js',
+    ));
+
+    expect(fn () => $cleaner->start('foo'))->toThrow(ProcessFailedException::class);
+});
+
+it('throws on start() timeout', function () {
+    $service = (new DompurifyService)->configure(new DompurifyServiceConfig(
+        binary: __DIR__.'/js-mocks/service-start-timeout.js',
+        startupTimeout: 50,
+    ));
+
+    expect(fn () => $service->start())->toThrow(ProcessTimedOutException::class);
+});
+
+it('throws on bad host', function () {
+    $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig(
+        host: 'a.b.c.example.com',
+    ));
+
+    $dirty = '<IMG """><SCRIPT>alert("XSS")</SCRIPT>">';
+
+    expect(fn () => $cleaner->send($dirty))->toThrow(ConnectException::class);
+});
 
 test('setup()', function () {
     $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig);
@@ -21,7 +59,7 @@
 
     $clean = $cleaner->send($dirty);
 
-    $cleaner->stop()->throwIfFailedOnTerm();
+    $cleaner->stop();
 
     expect($clean)->toBe(str_repeat('*/', 34 * 1000).'<img>"&gt;');
 })->depends('setup()');
@@ -39,29 +77,8 @@
 
     $clean = $cleaner->clean($dirty);
 
-    $service->stop()->throwIfFailedOnTerm();
+    $service->stop();
 
     expect($clean)->toBe('<img>"&gt;');
+    // TODO: expect exit with 143
 })->depends('setup()');
-
-it('throws on bad host', function () {
-    $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig(
-        host: 'a.b.c.example.com',
-    ));
-
-    $dirty = '<IMG """><SCRIPT>alert("XSS")</SCRIPT>">';
-
-    expect(fn () => $cleaner->send($dirty))->toThrow(ConnectException::class);
-});
-
-it('throws on bad node path', function () {
-    $service = (new DompurifyService)->configure(new DompurifyServiceConfig(
-        node: 'nodeZz',
-    ));
-
-    expect(fn () => $service->start())->toThrow(ProcessFailedException::class);
-
-    // $service->waitForTermination(2000);
-    // expect($service->serviceProcess->getExitCode())->toBe(127);
-    // TODO: fix https://github.com/medilies/xssless/actions/runs/10284119857/job/28459470742#step:7:28
-});

tests/Dompurify/js-mocks/service-start-timeout.js

@@ -0,0 +1,4 @@
+const start = Date.now();
+while (Date.now() - start < 500) {
+
+}