more config and coverage

medilies · Aug 10, 2024 · 1a29bcc · 1a29bcc
1 parent d95a89b
commit 1a29bcc
Show file tree

Hide file tree

Showing 10 changed files with 154 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -21,6 +21,8 @@ The default driver aligns with [OWASP](https://cheatsheetseries.owasp.org/cheats
 
 - PHP >= 8.2
 - ext-json
+- Node >= 18
+- NPM
 
 ## Installation
 
@@ -56,14 +58,17 @@ return [
 
     'drivers' => [
         'dompurify-cli' => new DompurifyCliConfig(
-            env('NODE_PATH', 'node'),
-            env('NPM_PATH', 'npm'),
+            node: env('NODE_PATH'),
+            npm: env('NPM_PATH'),
+            binary: null,
+            tempFolder: null,
         ),
         'dompurify-service' => new DompurifyServiceConfig(
-            env('NODE_PATH', 'node'),
-            env('NPM_PATH', 'npm'),
-            '127.0.0.1',
-            63000,
+            node: env('NODE_PATH'),
+            npm: env('NPM_PATH'),
+            host: '127.0.0.1',
+            port: 63000,
+            binary: null,
         ),
     ],
 ];

diff --git a/src/Dompurify/DompurifyCli.php b/src/Dompurify/DompurifyCli.php
@@ -38,6 +38,10 @@ public function exec(string $html): string
         $output = $process->getOutput();
         $cleanHtmlPath = trim($output);
 
+        if (! file_exists($cleanHtmlPath)) {
+            throw new XsslessException("Could not locate the file '{$cleanHtmlPath}'");
+        }
+
         $clean = file_get_contents($cleanHtmlPath);
 
         if ($clean === false) {
@@ -53,8 +57,7 @@ public function exec(string $html): string
 
     private function binPath(): string
     {
-        // TODO: allow config to override
-        $binPath = __DIR__.DIRECTORY_SEPARATOR.'cli.js';
+        $binPath = $this->config->binary ?? __DIR__.DIRECTORY_SEPARATOR.'cli.js';
 
         $binAbsPath = realpath($binPath);
 
@@ -69,6 +72,7 @@ private function saveHtml(string $value): string
     {
         $dir = $this->tempDir();
 
+        // ? use tempnam
         $fileName = mt_rand().'-'.str_replace([' ', '.'], '', microtime()).'.xss';
 
         $path = $dir.DIRECTORY_SEPARATOR.$fileName;
@@ -82,16 +86,20 @@ private function saveHtml(string $value): string
 
     private function tempDir(): string
     {
-        // TODO: take path from config
-        $tempDir = rtrim(sys_get_temp_dir(), DIRECTORY_SEPARATOR);
-        $dir = $tempDir.DIRECTORY_SEPARATOR.'xssless';
+        if (is_null($this->config->tempFolder)) {
+            $dir = sys_get_temp_dir().DIRECTORY_SEPARATOR.'xssless';
 
-        if (! file_exists($dir)) {
-            if (mkdir($dir, 0777, true) === false) {
+            if (! file_exists($dir) && mkdir($dir, 0777, true) === false) {
                 throw new XsslessException("Could not create temporary directory '{$dir}'");
             }
+
+            return $dir;
+        }
+
+        if (! file_exists($this->config->tempFolder)) {
+            throw new XsslessException("Could not locate temporary directory '{$this->config->tempFolder}'");
         }
 
-        return $dir;
+        return $this->config->tempFolder;
     }
 }
diff --git a/src/Dompurify/DompurifyCliConfig.php b/src/Dompurify/DompurifyCliConfig.php
@@ -9,8 +9,10 @@ class DompurifyCliConfig implements ConfigInterface
     private readonly string $class;
 
     public function __construct(
-        public string $node,
-        public string $npm,
+        public string $node = 'node',
+        public string $npm = 'npm',
+        public ?string $binary = null,
+        public ?string $tempFolder = null,
     ) {
         $this->class = DompurifyCli::class;
     }

diff --git a/src/Dompurify/DompurifyService.php b/src/Dompurify/DompurifyService.php
@@ -53,7 +53,7 @@ public function start(): static
     {
         $this->serviceProcess = new Process([
             $this->config->node,
-            __DIR__.DIRECTORY_SEPARATOR.'http.js',
+            $this->config->binary ?? __DIR__.DIRECTORY_SEPARATOR.'http.js',
             $this->config->host,
             $this->config->port,
         ]);
@@ -145,4 +145,6 @@ private function isSigTerm(): bool
     // {
     //     return $this->serviceProcess->getTermSignal() === 1;
     // }
+
+    // 92..97, 104, 113..136
 }
diff --git a/src/Dompurify/DompurifyServiceConfig.php b/src/Dompurify/DompurifyServiceConfig.php
@@ -9,10 +9,11 @@ class DompurifyServiceConfig implements ConfigInterface
     public readonly string $class;
 
     public function __construct(
-        public string $node,
-        public string $npm,
-        public string $host,
-        public int $port,
+        public string $node = 'node',
+        public string $npm = 'npm',
+        public string $host = '127.0.0.1',
+        public int $port = 6300,
+        public ?string $binary = null,
     ) {
         $this->class = DompurifyService::class;
     }

diff --git a/src/laravel/config/xssless.php b/src/laravel/config/xssless.php
@@ -8,14 +8,17 @@
 
     'drivers' => [
         'dompurify-cli' => new DompurifyCliConfig(
-            env('NODE_PATH', 'node'), // @phpstan-ignore argument.type
-            env('NPM_PATH', 'npm'), // @phpstan-ignore argument.type
+            node: env('NODE_PATH'), // @phpstan-ignore argument.type
+            npm: env('NPM_PATH'), // @phpstan-ignore argument.type
+            binary: null,
+            tempFolder: null,
         ),
         'dompurify-service' => new DompurifyServiceConfig(
-            env('NODE_PATH', 'node'), // @phpstan-ignore argument.type
-            env('NPM_PATH', 'npm'), // @phpstan-ignore argument.type
-            '127.0.0.1',
-            63000,
+            node: env('NODE_PATH'), // @phpstan-ignore argument.type
+            npm: env('NPM_PATH'), // @phpstan-ignore argument.type
+            host: '127.0.0.1',
+            port: 63000,
+            binary: null,
         ),
     ],
 ];
diff --git a/tests/Dompurify/DompurifyCliTest.php b/tests/Dompurify/DompurifyCliTest.php
@@ -3,10 +3,20 @@
 use Medilies\Xssless\Dompurify\DompurifyCli;
 use Medilies\Xssless\Dompurify\DompurifyCliConfig;
 use Medilies\Xssless\Xssless;
+use Medilies\Xssless\XsslessException;
 use Symfony\Component\Process\Exception\ProcessFailedException;
 
-test('setup()', function () {
+it('throws on bad node path', function () {
     $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
+        'nodeZz',
+        'npm',
+    ));
+
+    expect(fn () => $cleaner->exec('foo'))->toThrow(ProcessFailedException::class);
+});
+
+test('setup()', function () {
+    $cleaner = (new Xssless)->using(new DompurifyCliConfig(
         'node',
         'npm',
     ));
@@ -27,20 +37,42 @@
 
 test('clean()', function () {
     $cleaner = (new Xssless)->using(new DompurifyCliConfig(
-        'node',
-        'npm',
+        node: 'node',
+        npm: 'npm',
+        tempFolder: __DIR__,
     ));
 
     $clean = $cleaner->clean('<IMG """><SCRIPT>alert("XSS")</SCRIPT>">');
 
     expect($clean)->toBe('<img>"&gt;');
 })->depends('setup()');
 
-it('throws on bad node path', function () {
+it('throws when cannot read cleaned file', function () {
     $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
-        'nodeZz',
-        'npm',
+        node: 'node',
+        npm: 'npm',
+        binary: __DIR__.'/js-mocks/cli-returns-bad-path.js',
     ));
 
-    expect(fn () => $cleaner->exec('foo'))->toThrow(ProcessFailedException::class);
-});
+    expect(fn () => $cleaner->exec('foo'))->toThrow(XsslessException::class);
+})->depends('setup()');
+
+it('throws when cannot find binary file', function () {
+    $cleaner = (new DompurifyCli)->configure(new DompurifyCliConfig(
+        node: 'node',
+        npm: 'npm',
+        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(
+        node: 'node',
+        npm: 'npm',
+        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
@@ -46,9 +46,7 @@
 
     $cleaner = (new Xssless)->using($config);
 
-    $service = (new DompurifyService)->configure($config);
-
-    $service->start();
+    $service = $cleaner->start();
 
     $dirty = '<IMG """><SCRIPT>alert("XSS")</SCRIPT>">';
 

diff --git a/tests/Dompurify/js-mocks/cli-returns-bad-path.js b/tests/Dompurify/js-mocks/cli-returns-bad-path.js
@@ -0,0 +1,4 @@
+const htmlFile = process.argv[2];
+
+console.log(htmlFile + ".clean13465789");
+process.exit(0);
diff --git a/tests/XsslessTest.php b/tests/XsslessTest.php
@@ -0,0 +1,61 @@
+<?php
+
+use Medilies\Xssless\CliInterface;
+use Medilies\Xssless\ConfigInterface;
+use Medilies\Xssless\Dompurify\DompurifyCliConfig;
+use Medilies\Xssless\Xssless;
+use Medilies\Xssless\XsslessException;
+
+it('throws when makeCleaner() with no config', function () {
+    $cleaner = new Xssless;
+
+    $cleaner->using(new class implements ConfigInterface
+    {
+        public function getClass(): string
+        {
+            return Xssless::class;
+        }
+    });
+
+    expect(fn () => $cleaner->clean('foo'))->toThrow(XsslessException::class);
+});
+
+it('throws when makeCleaner() with no interface', function () {
+    $cleaner = new Xssless;
+
+    expect(fn () => $cleaner->clean('foo'))->toThrow(XsslessException::class);
+});
+
+it('throws when start() with CliInterface', function () {
+    $cleaner = new Xssless;
+    $cleaner->using(new DompurifyCliConfig);
+
+    expect(fn () => $cleaner->start())->toThrow(XsslessException::class);
+});
+
+it('throws when setup() without HasSetupInterface', function () {
+    $cleaner = new Xssless;
+
+    $cleaner->using(new class implements ConfigInterface
+    {
+        public function getClass(): string
+        {
+            return NoSetupDriver::class;
+        }
+    });
+
+    expect(fn () => $cleaner->setup())->toThrow(XsslessException::class);
+});
+
+class NoSetupDriver implements CliInterface
+{
+    public function configure(ConfigInterface $config): static
+    {
+        return $this;
+    }
+
+    public function exec(string $html): string
+    {
+        return '';
+    }
+}