Merge pull request #157 from calvinalkan/dev

fix(http-routing-bundle): enforce request data is not slashed

composer-require-checker.json

@@ -14,6 +14,8 @@
     "wp_mail",
     "add_action",
     "add_filter",
+    "did_action",
+    "doing_action",
     "apply_filters",
     "apply_filters_ref_array",
     "do_action",

src/Snicco/Bundle/http-routing/src/HttpKernelRunner.php

@@ -4,6 +4,7 @@
 
 namespace Snicco\Bundle\HttpRouting;
 
+use LogicException;
 use Nyholm\Psr7Server\ServerRequestCreator;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Psr\Http\Message\ServerRequestInterface;
@@ -16,7 +17,10 @@
 use Snicco\Component\StrArr\Str;
 
 use function add_action;
+use function did_action;
+use function doing_action;
 use function ltrim;
+use function sprintf;
 
 use const PHP_INT_MIN;
 
@@ -70,6 +74,8 @@ public function __construct(
      */
     public function listen(bool $is_admin, string $frontend_hook = 'wp_loaded', string $api_hook = 'init'): void
     {
+        $this->assertNoMagicQuotesAdded();
+
         $psr_request = $this->request_creator->fromGlobals();
 
         if ($this->isApiRequest($psr_request)) {
@@ -94,6 +100,8 @@ public function listen(bool $is_admin, string $frontend_hook = 'wp_loaded', stri
      */
     public function run(): void
     {
+        $this->assertNoMagicQuotesAdded();
+
         $psr_request = $this->request_creator->fromGlobals();
 
         $type = $this->isApiRequest($psr_request)
@@ -188,4 +196,26 @@ private function isApiRequest(ServerRequestInterface $request): bool
     {
         return $this->api_prefix && Str::startsWith($request->getUri()->getPath(), $this->api_prefix);
     }
+
+    /**
+     * We must create the PSR request before WordPress calls {@see
+     * wp_magic_quotes} and nukes the real server request data.
+     *
+     * {@see wp_magic_quotes} is called in wp-settings.php just after the plugins_loaded hook.
+     *
+     * @see https://core.trac.wordpress.org/ticket/18322
+     */
+    private function assertNoMagicQuotesAdded(): void
+    {
+        if (! did_action('plugins_loaded') || doing_action('plugins_loaded')) {
+            return;
+        }
+
+        throw new LogicException(
+            sprintf(
+                "You must call HttpKernelRunner::listen/run before the plugins_loaded hook. Otherwise WordPress will have nuked the SAPI request variables.\nSee: %s",
+                'https://core.trac.wordpress.org/ticket/18322'
+            )
+        );
+    }
 }

src/Snicco/Bundle/http-routing/tests/wordpress/HttpKernelRunnerTest.php

@@ -5,6 +5,7 @@
 namespace Snicco\Bundle\HttpRouting\Tests\wordpress;
 
 use Codeception\TestCase\WPTestCase;
+use LogicException;
 use ReflectionProperty;
 use Snicco\Bridge\Pimple\PimpleContainerAdapter;
 use Snicco\Bundle\HttpRouting\Event\HandledRequest;
@@ -23,6 +24,8 @@
 use Snicco\Component\Kernel\Kernel;
 use Snicco\Component\Kernel\ValueObject\Environment;
 
+use function add_action;
+use function did_action;
 use function dirname;
 use function do_action;
 use function remove_all_filters;
@@ -62,6 +65,13 @@ protected function setUp(): void
             ->make(HttpKernelRunner::class);
         remove_all_filters('admin_init');
         remove_all_filters('all_admin_notices');
+        if (did_action('plugins_loaded')) {
+            /**
+             * @psalm-suppress MixedArrayAccess
+             * @psalm-suppress InvalidArrayOffset
+             */
+            unset($GLOBALS['wp_actions']['plugins_loaded']);
+        }
     }
 
     protected function tearDown(): void
@@ -111,7 +121,7 @@ public function test_frontend_requests(): void
         $dispatcher->assertDispatched(
             'test_emitter',
             fn (Response $response): bool => RoutingBundleTestController::class === (string) $response->getBody()
-                && ! $response instanceof DelegatedResponse
+                                            && ! $response instanceof DelegatedResponse
         );
         $dispatcher->assertDispatched(TerminatedResponse::class);
     }
@@ -525,7 +535,7 @@ public function test_run_sends_a_frontend_response_immediately(): void
         $dispatcher->assertDispatched(
             'test_emitter',
             fn (Response $response): bool => RoutingBundleTestController::class === (string) $response->getBody()
-                && ! $response instanceof DelegatedResponse
+                                            && ! $response instanceof DelegatedResponse
         );
         $dispatcher->assertDispatched(TerminatedResponse::class);
     }
@@ -584,6 +594,47 @@ public function test_laminas_is_used_in_non_testing_env(): void
         $this->assertInstanceOf(LaminasEmitterStack::class, $emitter);
     }
 
+    /**
+     * @test
+     */
+    public function that_an_exception_is_thrown_if_http_kernel_runner_listens_after_plugins_loaded(): void
+    {
+        $this->expectException(LogicException::class);
+
+        do_action('plugins_loaded');
+
+        $this->http_dispatcher->listen(false);
+    }
+
+    /**
+     * @test
+     */
+    public function that_an_exception_is_thrown_if_http_kernel_runner_runs_after_plugins_loaded(): void
+    {
+        $this->expectException(LogicException::class);
+
+        do_action('plugins_loaded');
+
+        $this->http_dispatcher->run();
+    }
+
+    /**
+     * @test
+     */
+    public function than_no_exception_is_thrown_if_http_kernel_runner_is_used_within_plugins_loaded(): void
+    {
+        $called = false;
+
+        add_action('plugins_loaded', function () use (&$called) {
+            $this->http_dispatcher->listen(false);
+            $called = true;
+        });
+
+        do_action('plugins_loaded');
+
+        $this->assertTrue($called);
+    }
+
     protected function fixturesDir(): string
     {
         return dirname(__DIR__) . '/fixtures';