fix(http): prevent mapping request data to reserved properties on request objects (#1374)

Author: brendt

composer.json

@@ -45,10 +45,10 @@
         "carthage-software/mago": "0.26.1",
         "guzzlehttp/psr7": "^2.6.1",
         "league/flysystem-aws-s3-v3": "^3.0",
-        "league/flysystem-local": "^3.25.1",
         "league/flysystem-azure-blob-storage": "^3.0",
         "league/flysystem-ftp": "^3.0",
         "league/flysystem-google-cloud-storage": "^3.0",
+        "league/flysystem-local": "^3.25.1",
         "league/flysystem-memory": "^3.0",
         "league/flysystem-read-only": "^3.0",
         "league/flysystem-sftp-v3": "^3.0",

packages/http/src/Mappers/RequestToObjectMapper.php

@@ -4,13 +4,18 @@
 
 namespace Tempest\Http\Mappers;
 
+use ReflectionClass;
 use Tempest\Http\Request;
+use Tempest\Http\RequestParametersIncludedReservedNames;
 use Tempest\Mapper\Mapper;
 use Tempest\Mapper\Mappers\ArrayToObjectMapper;
+use Tempest\Reflection\ClassReflector;
+use Tempest\Reflection\PropertyReflector;
 use Tempest\Validation\Exceptions\ValidationFailed;
 use Tempest\Validation\Validator;
 
 use function Tempest\map;
+use function Tempest\Support\arr;
 
 final readonly class RequestToObjectMapper implements Mapper
 {
@@ -22,9 +27,17 @@ public function canMap(mixed $from, mixed $to): bool
     public function map(mixed $from, mixed $to): array|object
     {
         /** @var Request $from */
-        $data = $from->body;
+        $data = [...$from->files, ...$from->body, ...$from->query];
 
         if (is_a($to, Request::class, true)) {
+            $invalidReservedProperties = arr(new ClassReflector(Request::class)->getProperties())
+                ->map(fn (PropertyReflector $property) => $property->getName())
+                ->filter(fn (string $property) => array_key_exists($property, $data));
+
+            if ($invalidReservedProperties->isNotEmpty()) {
+                throw new RequestParametersIncludedReservedNames($to, $invalidReservedProperties);
+            }
+
             $data = [
                 ...[
                     'method' => $from->method,
@@ -36,8 +49,6 @@ public function map(mixed $from, mixed $to): array|object
                     'files' => $from->files,
                     'cookies' => $from->cookies,
                 ],
-                ...$from->files,
-                ...$from->query,
                 ...$data,
             ];
         }

packages/http/src/RequestParametersIncludedReservedNames.php

@@ -0,0 +1,20 @@
+<?php
+
+namespace Tempest\Http;
+
+use Exception;
+use Tempest\Support\Arr\ImmutableArray;
+
+final class RequestParametersIncludedReservedNames extends Exception
+{
+    public function __construct(string $requestClass, ImmutableArray $reservedProperties)
+    {
+        $message = sprintf(
+            'The request payload included data with reserved property names: %s. It could not be mapped to `%s`',
+            $reservedProperties->join('`, `')->wrap('`'),
+            $requestClass,
+        );
+
+        parent::__construct($message);
+    }
+}

tests/Integration/Database/Builder/InsertQueryBuilderTest.php

@@ -162,7 +162,7 @@ public function test_insert_on_model_table_with_existing_relation(): void
 
     public function test_inserting_has_many_via_parent_model_throws_exception(): void
     {
-        $this->assertException(HasManyRelationCouldNotBeInsterted::class, function () {
+        $this->assertException(HasManyRelationCouldNotBeInsterted::class, function (): void {
             query(Book::class)
                 ->insert(
                     title: 'Timeline Taxi',
@@ -174,7 +174,7 @@ public function test_inserting_has_many_via_parent_model_throws_exception(): voi
 
     public function test_inserting_has_one_via_parent_model_throws_exception(): void
     {
-        $this->assertException(HasOneRelationCouldNotBeInserted::class, function () {
+        $this->assertException(HasOneRelationCouldNotBeInserted::class, function (): void {
             query(Book::class)
                 ->insert(
                     title: 'Timeline Taxi',

tests/Integration/Route/RequestForInvalidMap.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace Tests\Tempest\Integration\Route;
+
+use Tempest\Http\IsRequest;
+use Tempest\Http\Request;
+
+final class RequestForInvalidMap implements Request
+{
+    use IsRequest;
+}

tests/Integration/Route/RequestToObjectMapperTest.php

@@ -7,6 +7,7 @@
 use Tempest\Http\Mappers\PsrRequestToGenericRequestMapper;
 use Tempest\Http\Mappers\RequestToObjectMapper;
 use Tempest\Http\Method;
+use Tempest\Http\RequestParametersIncludedReservedNames;
 use Tempest\Http\Upload;
 use Tempest\Validation\Exceptions\ValidationFailed;
 use Tempest\Validation\Rules\NotNull;
@@ -109,4 +110,26 @@ public function test_validation_fails_for_enum(): void
             $this->assertArrayHasKey('enumParam', $validationFailed->failingRules);
         }
     }
+
+    public function test_reserved_properties_cannot_be_mapped(): void
+    {
+        $this->assertException(
+            expectedExceptionClass: RequestParametersIncludedReservedNames::class,
+            handler: function (): void {
+                map(new GenericRequest(
+                    method: Method::GET,
+                    uri: '/books?uri=invalid',
+                    body: ['query' => 'invalid'],
+                    files: ['body' => 'invalid'],
+                ))->with(
+                    RequestToObjectMapper::class,
+                )->to(RequestForInvalidMap::class);
+            },
+            assertException: function (RequestParametersIncludedReservedNames $exception): void {
+                $this->assertStringContainsString('uri', $exception->getMessage());
+                $this->assertStringContainsString('query', $exception->getMessage());
+                $this->assertStringContainsString('body', $exception->getMessage());
+            },
+        );
+    }
 }