Merge pull request #1695 from brefphp/support-invalid-cookies

Fix "internal error" on invalid cookies
brefphp · Nov 21, 2023 · 8d73b63 · 8d73b63
2 parents 7545dbb + 2cb61e8
commit 8d73b63
Show file tree

Hide file tree

Showing 8 changed files with 158 additions and 8 deletions.
diff --git a/src/Event/Http/HttpRequestEvent.php b/src/Event/Http/HttpRequestEvent.php
@@ -190,7 +190,11 @@ public function getCookies(): array
 
         $cookies = [];
         foreach ($cookieParts as $cookiePart) {
-            [$cookieName, $cookieValue] = explode('=', $cookiePart, 2);
+            $explode = explode('=', $cookiePart, 2);
+            if (count($explode) !== 2) {
+                continue;
+            }
+            [$cookieName, $cookieValue] = $explode;
             $cookies[$cookieName] = urldecode($cookieValue);
         }
         return $cookies;

diff --git a/tests/Event/Http/CommonHttpTest.php b/tests/Event/Http/CommonHttpTest.php
@@ -416,6 +416,17 @@ public function test request with cookies(int $version)
         ]);
     }
 
+    /**
+     * @dataProvider provide API Gateway versions
+     */
+    public function test request with invalid cookies(int $version)
+    {
+        $this->fromFixture(__DIR__ . "/Fixture/ag-v$version-cookies-invalid.json");
+
+        // See https://stackoverflow.com/a/61695783/245552
+        $this->assertCookies([], 'foo');
+    }
+
     /**
      * @dataProvider provide API Gateway versions
      */
@@ -497,7 +508,7 @@ abstract protected function assertBody(string $expected): void;
 
     abstract protected function assertContentType(?string $expected): void;
 
-    abstract protected function assertCookies(array $expected): void;
+    abstract protected function assertCookies(array $expected, string |null $expectedHeader = null): void;
 
     abstract protected function assertHeaders(array $expected): void;
 

diff --git a/tests/Event/Http/Fixture/ag-v1-cookies-invalid.json b/tests/Event/Http/Fixture/ag-v1-cookies-invalid.json
@@ -0,0 +1,53 @@
+{
+    "version": "1.0",
+    "resource": "/path",
+    "path": "/path",
+    "httpMethod": "GET",
+    "headers": {
+        "Accept": "*/*",
+        "Accept-Encoding": "gzip, deflate",
+        "Cache-Control": "no-cache",
+        "Host": "example.org",
+        "User-Agent": "PostmanRuntime/7.20.1",
+        "X-Amzn-Trace-Id": "Root=1-ffffffff-ffffffffffffffffffffffff",
+        "X-Forwarded-For": "1.1.1.1",
+        "X-Forwarded-Port": "443",
+        "X-Forwarded-Proto": "https",
+        "Cookie": "foo"
+    },
+    "queryStringParameters": null,
+    "pathParameters": null,
+    "stageVariables": null,
+    "requestContext": {
+        "resourceId": "xxxxxx",
+        "resourcePath": "/path",
+        "httpMethod": "PUT",
+        "extendedRequestId": "XXXXXX-xxxxxxxx=",
+        "requestTime": "24/Nov/2019:18:55:08 +0000",
+        "path": "/path",
+        "accountId": "123400000000",
+        "protocol": "HTTP/1.1",
+        "stage": "dev",
+        "domainPrefix": "dev",
+        "requestTimeEpoch": 1574621708700,
+        "requestId": "ffffffff-ffff-4fff-ffff-ffffffffffff",
+        "identity": {
+            "cognitoIdentityPoolId": null,
+            "accountId": null,
+            "cognitoIdentityId": null,
+            "caller": null,
+            "sourceIp": "1.1.1.1",
+            "principalOrgId": null,
+            "accessKey": null,
+            "cognitoAuthenticationType": null,
+            "cognitoAuthenticationProvider": null,
+            "userArn": null,
+            "userAgent": "PostmanRuntime/7.20.1",
+            "user": null
+        },
+        "domainName": "example.org",
+        "apiId": "xxxxxxxxxx"
+    },
+    "body": "",
+    "isBase64Encoded": false
+}
diff --git a/tests/Event/Http/Fixture/ag-v2-cookies-invalid.json b/tests/Event/Http/Fixture/ag-v2-cookies-invalid.json
@@ -0,0 +1,41 @@
+{
+    "version": "2.0",
+    "routeKey": "ANY /path",
+    "rawPath": "/path",
+    "rawQueryString": "",
+    "cookies": ["foo"],
+    "headers": {
+        "Accept": "*/*",
+        "Accept-Encoding": "gzip, deflate",
+        "Cache-Control": "no-cache",
+        "Host": "example.org",
+        "User-Agent": "PostmanRuntime/7.20.1",
+        "X-Amzn-Trace-Id": "Root=1-ffffffff-ffffffffffffffffffffffff",
+        "X-Forwarded-For": "1.1.1.1",
+        "X-Forwarded-Port": "443",
+        "X-Forwarded-Proto": "https"
+    },
+    "queryStringParameters": null,
+    "stageVariables": null,
+    "requestContext": {
+        "accountId": "123400000000",
+        "apiId": "xxxxxxxxxx",
+        "domainName": "example.org",
+        "domainPrefix": "0000000000",
+        "http": {
+            "method": "GET",
+            "path": "/path",
+            "protocol": "HTTP/1.1",
+            "sourceIp": "1.1.1.1",
+            "userAgent": "PostmanRuntime/7.20.1"
+        },
+        "requestId": "JTHoQgr2oAMEPMg=",
+        "routeId": "47matwk",
+        "routeKey": "ANY /path",
+        "stage": "$default",
+        "time": "24/Nov/2019:18:55:08 +0000",
+        "timeEpoch": 1574621708700
+    },
+    "body": "",
+    "isBase64Encoded": false
+}
diff --git a/tests/Event/Http/HttpRequestEventTest.php b/tests/Event/Http/HttpRequestEventTest.php
@@ -27,15 +27,18 @@ protected function assertContentType(?string $expected): void
         }
     }
 
-    protected function assertCookies(array $expected): void
+    protected function assertCookies(array $expected, string |null $expectedHeader = null): void
     {
         $this->assertEquals($expected, $this->event->getCookies());
 
         // Also check that the cookies are available in the HTTP headers (they should be)
-        $expectedHeader = array_map(function (string $value, string $key): string {
-            return $key . '=' . urlencode($value);
-        }, $expected, array_keys($expected));
-        $this->assertEquals(implode('; ', $expectedHeader), $this->event->getHeaders()['cookie'][0] ?? '');
+        if ($expectedHeader === null) {
+            $expectedHeader = array_map(function (string $value, string $key): string {
+                return $key . '=' . urlencode($value);
+            }, $expected, array_keys($expected));
+            $expectedHeader = implode('; ', $expectedHeader);
+        }
+        $this->assertEquals($expectedHeader, $this->event->getHeaders()['cookie'][0] ?? '');
     }
 
     protected function assertHeaders(array $expected): void

diff --git a/tests/Event/Http/Psr7BridgeTest.php b/tests/Event/Http/Psr7BridgeTest.php
@@ -48,7 +48,7 @@ protected function assertContentType(?string $expected): void
         $this->assertEquals($expected, $this->request->getHeaderLine('Content-Type'));
     }
 
-    protected function assertCookies(array $expected): void
+    protected function assertCookies(array $expected, string |null $expectedHeader = null): void
     {
         $this->assertEquals($expected, $this->request->getCookieParams());
     }

diff --git a/tests/FpmRuntime/FpmHandlerTest.php b/tests/FpmRuntime/FpmHandlerTest.php
@@ -777,6 +777,42 @@ public function test request with cookies(int $version)
         ]);
     }
 
+    /**
+     * @dataProvider provide API Gateway versions
+     */
+    public function test request with invalid cookies(int $version)
+    {
+        $event = [
+            'version' => '1.0',
+            'httpMethod' => 'GET',
+            'headers' => [
+                'Cookie' => 'foo',
+            ],
+        ];
+        $this->assertGlobalVariables($event, [
+            '$_GET' => [],
+            '$_POST' => [],
+            '$_FILES' => [],
+            '$_COOKIE' => [
+                'foo' => '',
+            ],
+            '$_REQUEST' => [],
+            '$_SERVER' => [
+                'REQUEST_URI' => '/',
+                'PHP_SELF' => '/',
+                'PATH_INFO' => '/',
+                'REQUEST_METHOD' => 'GET',
+                'QUERY_STRING' => '',
+                'HTTP_COOKIE' => 'foo',
+                'CONTENT_LENGTH' => '0',
+                'CONTENT_TYPE' => 'application/x-www-form-urlencoded',
+                'LAMBDA_INVOCATION_CONTEXT' => json_encode($this->fakeContext),
+                'LAMBDA_REQUEST_CONTEXT' => '[]',
+            ],
+            'HTTP_RAW_BODY' => '',
+        ]);
+    }
+
     /**
      * @dataProvider provide API Gateway versions
      */

diff --git a/tests/HttpRequestProxyTest.php b/tests/HttpRequestProxyTest.php
@@ -47,6 +47,8 @@ public function test POST request with multipart file uploads(int $version
 
     public function test request with cookies(int $version);
 
+    public function test request with invalid cookies(int $version);
+
     public function test POST request with base64 encoded body(int $version);
 
     public function test PUT request(int $version);