From 9401559409ae7470736de66809e68749bccedaed Mon Sep 17 00:00:00 2001 From: medilies Date: Mon, 12 Aug 2024 11:51:47 +0100 Subject: [PATCH] service timeouts + no signals + no exit codes --- src/Dompurify/DompurifyService.php | 71 ++++--------------- src/Dompurify/DompurifyServiceConfig.php | 1 + src/ServiceInterface.php | 2 - src/laravel/Commands/StartCommand.php | 3 +- tests/Dompurify/DompurifyCliTest.php | 34 ++++----- tests/Dompurify/DompurifyServiceTest.php | 62 +++++++++------- .../js-mocks/service-start-timeout.js | 4 ++ 7 files changed, 75 insertions(+), 102 deletions(-) create mode 100644 tests/Dompurify/js-mocks/service-start-timeout.js diff --git a/src/Dompurify/DompurifyService.php b/src/Dompurify/DompurifyService.php index caf04e6..67d277a 100644 --- a/src/Dompurify/DompurifyService.php +++ b/src/Dompurify/DompurifyService.php @@ -7,7 +7,6 @@ use Medilies\Xssless\ConfigInterface; use Medilies\Xssless\HasSetupInterface; use Medilies\Xssless\ServiceInterface; -use Medilies\Xssless\XsslessException; use Symfony\Component\Process\Exception\ProcessFailedException; use Symfony\Component\Process\Process; @@ -58,15 +57,24 @@ public function start(): static $this->config->port, ]); + $this->serviceProcess->setIdleTimeout($this->config->startupTimeoutMs / 1000); + $this->serviceProcess->start(); + // ? is there a possibility to miss output before running the waitUntil $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; }); - if (! $this->serviceProcess->isRunning()) { + $this->serviceProcess->setIdleTimeout(null); + + // ? rm check + if (! $this->isRunning()) { throw new ProcessFailedException($this->serviceProcess); } @@ -75,9 +83,7 @@ public function start(): static public function stop(): static { - if ($this->isRunning()) { - $this->serviceProcess->stop(); - } + $this->serviceProcess->stop(); return $this; } @@ -96,53 +102,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; - // } } diff --git a/src/Dompurify/DompurifyServiceConfig.php b/src/Dompurify/DompurifyServiceConfig.php index c953ffa..0cbad17 100644 --- a/src/Dompurify/DompurifyServiceConfig.php +++ b/src/Dompurify/DompurifyServiceConfig.php @@ -14,6 +14,7 @@ public function __construct( public string $host = '127.0.0.1', public int $port = 63000, public ?string $binary = null, + public int $startupTimeoutMs = 6000, ) { $this->class = DompurifyService::class; } diff --git a/src/ServiceInterface.php b/src/ServiceInterface.php index 9bc8e10..fde1f00 100755 --- a/src/ServiceInterface.php +++ b/src/ServiceInterface.php @@ -17,6 +17,4 @@ public function isRunning(): bool; public function getIncrementalOutput(): string; public function getIncrementalErrorOutput(): string; - - public function throwIfFailedOnTerm(): void; } diff --git a/src/laravel/Commands/StartCommand.php b/src/laravel/Commands/StartCommand.php index 8eb88ec..35c1213 100644 --- a/src/laravel/Commands/StartCommand.php +++ b/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(); } } diff --git a/tests/Dompurify/DompurifyCliTest.php b/tests/Dompurify/DompurifyCliTest.php index 84336ff..8c8a19c 100644 --- a/tests/Dompurify/DompurifyCliTest.php +++ b/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()'); +}); diff --git a/tests/Dompurify/DompurifyServiceTest.php b/tests/Dompurify/DompurifyServiceTest.php index 5422165..95747ff 100644 --- a/tests/Dompurify/DompurifyServiceTest.php +++ b/tests/Dompurify/DompurifyServiceTest.php @@ -5,6 +5,42 @@ 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); +}); + +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', + startupTimeoutMs: 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 = '">'; + + expect(fn () => $cleaner->send($dirty))->toThrow(ConnectException::class); +}); test('setup()', function () { $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig); @@ -21,7 +57,7 @@ $clean = $cleaner->send($dirty); - $cleaner->stop()->throwIfFailedOnTerm(); + $cleaner->stop(); expect($clean)->toBe(str_repeat('*/', 34 * 1000).'">'); })->depends('setup()'); @@ -39,29 +75,7 @@ $clean = $cleaner->clean($dirty); - $service->stop()->throwIfFailedOnTerm(); + $service->stop(); expect($clean)->toBe('">'); })->depends('setup()'); - -it('throws on bad host', function () { - $cleaner = (new DompurifyService)->configure(new DompurifyServiceConfig( - host: 'a.b.c.example.com', - )); - - $dirty = '">'; - - 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 -}); diff --git a/tests/Dompurify/js-mocks/service-start-timeout.js b/tests/Dompurify/js-mocks/service-start-timeout.js new file mode 100644 index 0000000..e6d12e6 --- /dev/null +++ b/tests/Dompurify/js-mocks/service-start-timeout.js @@ -0,0 +1,4 @@ +const start = Date.now(); +while (Date.now() - start < 500) { + +}