From 06954050d4a031884c4a68a8c3efb7ec7910ebde Mon Sep 17 00:00:00 2001 From: Jason Desrosiers Date: Sat, 12 Sep 2015 12:28:28 -0700 Subject: [PATCH] General cleanup --- .scrutinizer.yml | 5 ++++ src/Cors.php | 16 ++++++------ src/Test/CorsServiceProviderTest.php | 37 +++++++++++++++++++++++----- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 1833e8e..8353a52 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -1,6 +1,11 @@ filter: excluded_paths: [vendor/*] +checks: + php: + code_rating: true + duplication: true + build: tests: override: diff --git a/src/Cors.php b/src/Cors.php index 82c4fa7..2b4e4c5 100644 --- a/src/Cors.php +++ b/src/Cors.php @@ -29,21 +29,22 @@ private function corsHeaders(Request $request, $allow) } if ($this->isPreflightRequest($request)) { - $allowedMethods = $this->allowedMethods($request, $allow); - if (!in_array($request->headers->get("Access-Control-Request-Method"), $allowedMethods)) { + $allowedMethods = $this->allowedMethods($allow); + $requestMethod = $request->headers->get("Access-Control-Request-Method"); + if (!in_array($requestMethod, preg_split("/\s*,\s*/", $allowedMethods))) { return array(); } // TODO: Allow cors.allowHeaders to be set and use it to validate the request $headers["Access-Control-Allow-Headers"] = $request->headers->get("Access-Control-Request-Headers"); - $headers["Access-Control-Allow-Methods"] = implode(',', $allowedMethods); + $headers["Access-Control-Allow-Methods"] = $allowedMethods; $headers["Access-Control-Max-Age"] = $this->app["cors.maxAge"]; } else { $headers["Access-Control-Expose-Headers"] = $this->app["cors.exposeHeaders"]; } $headers["Access-Control-Allow-Origin"] = $this->allowOrigin($request); - $headers["Access-Control-Allow-Credentials"] = $this->allowCredentials($request); + $headers["Access-Control-Allow-Credentials"] = $this->allowCredentials(); return array_filter($headers); } @@ -58,10 +59,9 @@ private function isPreflightRequest(Request $request) return $request->getMethod() === "OPTIONS" && $request->headers->has("Access-Control-Request-Method"); } - private function allowedMethods(Request $request, $allow) + private function allowedMethods($allow) { - $allowMethods = !is_null($this->app["cors.allowMethods"]) ? $this->app["cors.allowMethods"] : $allow; - return preg_split("/\s*,\s*/", $allowMethods); + return !is_null($this->app["cors.allowMethods"]) ? $this->app["cors.allowMethods"] : $allow; } private function allowOrigin(Request $request) @@ -78,7 +78,7 @@ private function allowOrigin(Request $request) return in_array($origin, preg_split('/\s+/', $this->app["cors.allowOrigin"])) ? $origin : "null"; } - private function allowCredentials(Request $request) + private function allowCredentials() { return $this->app["cors.allowCredentials"] === true ? "true" : null; } diff --git a/src/Test/CorsServiceProviderTest.php b/src/Test/CorsServiceProviderTest.php index db39ccd..e83758f 100644 --- a/src/Test/CorsServiceProviderTest.php +++ b/src/Test/CorsServiceProviderTest.php @@ -6,8 +6,6 @@ use Silex\Application; use Symfony\Component\HttpKernel\Client; -require_once __DIR__ . "/../../vendor/autoload.php"; - class CorsServiceProviderTest extends \PHPUnit_Framework_TestCase { protected $app; @@ -197,6 +195,33 @@ public function testAllowOriginFail() $this->assertEquals("", $response->getContent()); } + public function testDefaultAllowMethodsWithMultipleAllow() + { + $this->app->match("/foo", function () { + return "foo"; + })->method("GET|POST"); + + $headers = array( + "HTTP_ORIGIN" => "www.foo.com", + "HTTP_ACCESS_CONTROL_REQUEST_METHOD" => "GET", + ); + $client = new Client($this->app, $headers); + $client->request("OPTIONS", "/foo"); + + $response = $client->getResponse(); + + $this->assertEquals("204", $response->getStatusCode()); + $this->assertEquals("GET,POST", $response->headers->get("Allow")); + $this->assertEquals("GET,POST", $response->headers->get("Access-Control-Allow-Methods")); + $this->assertEquals("www.foo.com", $response->headers->get("Access-Control-Allow-Origin")); + $this->assertFalse($response->headers->has("Access-Control-Allow-Headers")); + $this->assertEquals("15", $response->headers->get("Access-Control-Max-Age")); + $this->assertFalse($response->headers->has("Access-Control-Allow-Credentials")); + $this->assertFalse($response->headers->has("Access-Control-Expose-Headers")); + $this->assertFalse($response->headers->has("Content-Type")); + $this->assertEquals("", $response->getContent()); + } + public function testAllowMethods() { $this->app["cors.allowMethods"] = "GET"; @@ -255,13 +280,13 @@ public function testAllowMethodsFail() $this->assertEquals("", $response->getContent()); } - public function testAllowMultipleAccessControlAllowMethods() + public function testAllowMultipleAllowMethods() { $this->app["cors.allowMethods"] = "GET,POST"; $this->app->match("/foo", function () { return "foo"; - })->method("GET|POST"); + })->method("GET|POST|DELETE"); $headers = array( "HTTP_ORIGIN" => "www.foo.com", @@ -273,7 +298,8 @@ public function testAllowMultipleAccessControlAllowMethods() $response = $client->getResponse(); $this->assertEquals("204", $response->getStatusCode()); - $this->assertEquals("GET,POST", $response->headers->get("Allow")); + $this->assertEquals("GET,POST,DELETE", $response->headers->get("Allow")); + $this->assertEquals("GET,POST", $response->headers->get("Access-Control-Allow-Methods")); $this->assertEquals("www.foo.com", $response->headers->get("Access-Control-Allow-Origin")); $this->assertFalse($response->headers->has("Access-Control-Allow-Headers")); $this->assertEquals("15", $response->headers->get("Access-Control-Max-Age")); @@ -281,7 +307,6 @@ public function testAllowMultipleAccessControlAllowMethods() $this->assertFalse($response->headers->has("Access-Control-Expose-Headers")); $this->assertFalse($response->headers->has("Content-Type")); $this->assertEquals("", $response->getContent()); - $this->assertEquals("GET,POST", $response->headers->get("Access-Control-Allow-Methods")); } public function testAllowCredentialsAndExposeCredentials()