General cleanup

jdesrosiers · Sep 12, 2015 · 0695405 · 0695405
1 parent c529a98
commit 0695405
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 14 deletions.
diff --git 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
@@ -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
@@ -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,15 +298,15 @@ 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"));
         $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());
-        $this->assertEquals("GET,POST", $response->headers->get("Access-Control-Allow-Methods"));
     }
 
     public function testAllowCredentialsAndExposeCredentials()