diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 98afb29..f1cd707 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -468,7 +468,7 @@ public function handlePageCache(Request $request) { } $request_policy = \Drupal::service('page_cache_request_policy'); - if ($cache_enabled && $request_policy->check($request) == RequestPolicyInterface::ALLOW) { + if ($cache_enabled && $request_policy->check($request) === RequestPolicyInterface::ALLOW) { // Get the page from the cache. $response = drupal_page_get_cache($request); // If there is a cached page, display it. diff --git a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php index 4d21bc1..95508a1 100644 --- a/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php @@ -105,7 +105,7 @@ public function onRespond(FilterResponseEvent $event) { $response->headers->set($name, $value, FALSE); } - $is_cacheable = ($this->requestPolicy->check($request) == RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) != ResponsePolicyInterface::DENY); + $is_cacheable = ($this->requestPolicy->check($request) === RequestPolicyInterface::ALLOW) && ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY); // Add headers necessary to specify whether the response should be cached by // proxies and/or the browser. diff --git a/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php b/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php index 0b87088..c88e1ea 100644 --- a/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php +++ b/core/lib/Drupal/Core/PageCache/ChainRequestPolicy.php @@ -41,10 +41,10 @@ public function check(Request $request) { foreach ($this->rules as $rule) { $result = $rule->check($request); - if ($result == static::ALLOW) { + if ($result === static::ALLOW) { $allow = $result; } - elseif ($result == static::DENY) { + elseif ($result === static::DENY) { $deny = $result; break; } diff --git a/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php b/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php index 606328b..c9a334e 100644 --- a/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php +++ b/core/lib/Drupal/Core/PageCache/ChainResponsePolicy.php @@ -35,7 +35,7 @@ class ChainResponsePolicy implements ChainResponsePolicyInterface { */ public function check(Response $response, Request $request) { foreach ($this->rules as $rule) { - if ($rule->check($response, $request) == static::DENY) { + if ($rule->check($response, $request) === static::DENY) { return static::DENY; } } diff --git a/core/lib/Drupal/Core/PageCache/RequestPolicy/CommandLineOrUnsafeMethod.php b/core/lib/Drupal/Core/PageCache/RequestPolicy/CommandLineOrUnsafeMethod.php index b2dde0e..66a3bf6 100644 --- a/core/lib/Drupal/Core/PageCache/RequestPolicy/CommandLineOrUnsafeMethod.php +++ b/core/lib/Drupal/Core/PageCache/RequestPolicy/CommandLineOrUnsafeMethod.php @@ -23,7 +23,7 @@ class CommandLineOrUnsafeMethod implements RequestPolicyInterface { * {@inheritdoc} */ public function check(Request $request) { - if ($this->isCliRequest($request) || !$request->isMethodSafe()) { + if ($this->isCli() || !$request->isMethodSafe()) { return static::DENY; } } @@ -31,7 +31,7 @@ public function check(Request $request) { /** * Indicates whether this is a CLI request. */ - protected function isCliRequest(Request $request) { + protected function isCli() { return PHP_SAPI === 'cli'; } diff --git a/core/lib/Drupal/Core/PageCache/RequestPolicy/NoSessionOpen.php b/core/lib/Drupal/Core/PageCache/RequestPolicy/NoSessionOpen.php index b06b4b8..e6fac79 100644 --- a/core/lib/Drupal/Core/PageCache/RequestPolicy/NoSessionOpen.php +++ b/core/lib/Drupal/Core/PageCache/RequestPolicy/NoSessionOpen.php @@ -21,10 +21,24 @@ class NoSessionOpen implements RequestPolicyInterface { /** + * The name of the session cookie. + * + * @var string + */ + protected $sessionCookieName; + + /** + * Constructs a new page cache session policy. + */ + public function __construct($session_cookie_name = NULL) { + $this->sessionCookieName = $session_cookie_name ?: session_name(); + } + + /** * {@inheritdoc} */ public function check(Request $request) { - if (!$request->cookies->has(session_name())) { + if (!$request->cookies->has($this->sessionCookieName)) { return static::ALLOW; } } diff --git a/core/modules/image/src/Tests/PageCache/DenyPrivateImageStyleDownloadTest.php b/core/modules/image/src/Tests/PageCache/DenyPrivateImageStyleDownloadTest.php new file mode 100644 index 0000000..5bb4b83 --- /dev/null +++ b/core/modules/image/src/Tests/PageCache/DenyPrivateImageStyleDownloadTest.php @@ -0,0 +1,90 @@ +routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $this->policy = new DenyPrivateImageStyleDownload($this->routeMatch); + $this->response = new Response(); + $this->request = new Request(); + } + + /** + * Asserts that chaching is denied on the private image style download route. + * + * @dataProvider providerPrivateImageStyleDownloadPolicy + * @covers ::check + */ + public function testPrivateImageStyleDownloadPolicy($expected_result, $route_name) { + $this->routeMatch->expects($this->once()) + ->method('getRouteName') + ->will($this->returnValue($route_name)); + + $actual_result = $this->policy->check($this->response, $this->request); + $this->assertSame($expected_result, $actual_result); + } + + /** + * Provides data and expected results for the test method. + * + * @return array + * Data and expected results. + */ + public function providerPrivateImageStyleDownloadPolicy() { + return [ + [ResponsePolicyInterface::DENY, 'image.style_private'], + [NULL, 'some.other.route'], + [NULL, NULL], + [NULL, FALSE], + [NULL, TRUE], + [NULL, new \StdClass()], + [NULL, [1, 2, 3]], + ]; + } + +} diff --git a/core/modules/node/src/Tests/PageCache/DenyNodePreviewTest.php b/core/modules/node/src/Tests/PageCache/DenyNodePreviewTest.php new file mode 100644 index 0000000..c7aaf01 --- /dev/null +++ b/core/modules/node/src/Tests/PageCache/DenyNodePreviewTest.php @@ -0,0 +1,90 @@ +routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); + $this->policy = new DenyNodePreview($this->routeMatch); + $this->response = new Response(); + $this->request = new Request(); + } + + /** + * Asserts that chaching is denied on the node preview route. + * + * @dataProvider providerPrivateImageStyleDownloadPolicy + * @covers ::check + */ + public function testPrivateImageStyleDownloadPolicy($expected_result, $route_name) { + $this->routeMatch->expects($this->once()) + ->method('getRouteName') + ->will($this->returnValue($route_name)); + + $actual_result = $this->policy->check($this->response, $this->request); + $this->assertSame($expected_result, $actual_result); + } + + /** + * Provides data and expected results for the test method. + * + * @return array + * Data and expected results. + */ + public function providerPrivateImageStyleDownloadPolicy() { + return [ + [ResponsePolicyInterface::DENY, 'entity.node.preview'], + [NULL, 'some.other.route'], + [NULL, NULL], + [NULL, FALSE], + [NULL, TRUE], + [NULL, new \StdClass()], + [NULL, [1, 2, 3]], + ]; + } + +} diff --git a/core/modules/toolbar/src/Tests/PageCache/AllowToolbarPathTest.php b/core/modules/toolbar/src/Tests/PageCache/AllowToolbarPathTest.php new file mode 100644 index 0000000..da2533f --- /dev/null +++ b/core/modules/toolbar/src/Tests/PageCache/AllowToolbarPathTest.php @@ -0,0 +1,60 @@ +policy = new AllowToolbarPath(); + } + + /** + * Asserts that caching is allowed if the request goes to toolbar subtree. + * + * @dataProvider providerTestAllowToolbarPath + * @covers ::check + */ + public function testAllowToolbarPath($expected_result, $path) { + $request = Request::create($path); + $result = $this->policy->check($request); + $this->assertSame($expected_result, $result); + } + + /** + * Provides data and expected results for the test method. + * + * @return array + * Data and expected results. + */ + public function providerTestAllowToolbarPath() { + return [ + [NULL, '/'], + [NULL, '/other-path?q=/toolbar/subtrees/'], + [RequestPolicyInterface::ALLOW, '/de/toolbar/subtrees/abcd'], + [RequestPolicyInterface::ALLOW, '/en-us/toolbar/subtrees/'], + [RequestPolicyInterface::ALLOW, '/toolbar/subtrees/some-hash'], + ]; + } + +} diff --git a/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php b/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php new file mode 100644 index 0000000..fd41185 --- /dev/null +++ b/core/tests/Drupal/Tests/Core/PageCache/ChainRequestPolicyTest.php @@ -0,0 +1,154 @@ +policy = new ChainRequestPolicy(); + $this->request = new Request(); + } + + /** + * Asserts that check() returns NULL if the chain is empty. + * + * @covers ::check + */ + public function testEmptyChain() { + $result = $this->policy->check($this->request); + $this->assertSame(NULL, $result); + } + + /** + * Asserts that check() returns NULL if none of the rules returns an actionable result. + * + * @dataProvider providerChainNoResultOnInvalidReturnValue + * @covers ::check + */ + public function testChainNoResultOnInvalidReturnValue($return_value) { + $rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $rule->expects($this->once()) + ->method('check') + ->with($this->request) + ->will($this->returnValue($return_value)); + + $this->policy->addPolicy($rule); + + $actual_result = $this->policy->check($this->request); + $this->assertSame(NULL, $actual_result); + } + + /** + * Provides test data for testChainNoResultOnInvalidReturnValue. + * + * @return array + * Test input and expected result. + */ + public function providerChainNoResultOnInvalidReturnValue() { + return [ + [NULL], + [FALSE], + [0], + [1], + [TRUE], + [[1, 2, 3]], + [new \stdClass()], + ]; + } + + /** + * Asserts that check() returns ALLOW if any of the rules returns ALLOW. + * + * @dataProvider providerAllowIfAnyRuleReturnedAllow + * @covers ::check + */ + public function testAllowIfAnyRuleReturnedAllow($return_values) { + foreach ($return_values as $return_value) { + $rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $rule->expects($this->once()) + ->method('check') + ->with($this->request) + ->will($this->returnValue($return_value)); + + $this->policy->addPolicy($rule); + } + + $actual_result = $this->policy->check($this->request); + $this->assertSame(RequestPolicyInterface::ALLOW, $actual_result); + } + + /** + * Provides test data for testAllowIfAnyRuleReturnedAllow. + * + * @return array + * Test input and expected result. + */ + public function providerAllowIfAnyRuleReturnedAllow() { + return [ + [[RequestPolicyInterface::ALLOW]], + [[NULL, RequestPolicyInterface::ALLOW]], + [[RequestPolicyInterface::ALLOW, FALSE]], + [[0, RequestPolicyInterface::ALLOW]], + [[RequestPolicyInterface::ALLOW, 1]], + [[TRUE, RequestPolicyInterface::ALLOW]], + [[RequestPolicyInterface::ALLOW, [1, 2, 3]]], + [[new \stdClass(), RequestPolicyInterface::ALLOW]], + ]; + } + + /** + * Asserts that check() returns imediately when a rule returned DENY. + */ + public function testStopChainOnFirstDeny() { + $rule1 = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $rule1->expects($this->once()) + ->method('check') + ->with($this->request) + ->will($this->returnValue(RequestPolicyInterface::ALLOW)); + $this->policy->addPolicy($rule1); + + $deny_rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $deny_rule->expects($this->once()) + ->method('check') + ->with($this->request) + ->will($this->returnValue(RequestPolicyInterface::DENY)); + $this->policy->addPolicy($deny_rule); + + $ignored_rule = $this->getMock('Drupal\Core\PageCache\RequestPolicyInterface'); + $ignored_rule->expects($this->never()) + ->method('check'); + $this->policy->addPolicy($ignored_rule); + + $actual_result = $this->policy->check($this->request); + $this->assertsame(RequestPolicyInterface::DENY, $actual_result); + } + +} diff --git a/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php b/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php new file mode 100644 index 0000000..8195e4c --- /dev/null +++ b/core/tests/Drupal/Tests/Core/PageCache/ChainResponsePolicyTest.php @@ -0,0 +1,122 @@ +policy = new ChainResponsePolicy(); + $this->response = new Response(); + $this->request = new Request(); + } + + /** + * Asserts that check() returns NULL if the chain is empty. + * + * @covers ::check + */ + public function testEmptyChain() { + $result = $this->policy->check($this->response, $this->request); + $this->assertSame(NULL, $result); + } + + /** + * Asserts that check() returns NULL if none of the rules returns an actionable result. + * + * @dataProvider providerChainNoResultOnInvalidReturnValue + * @covers ::check + */ + public function testChainNoResultOnInvalidReturnValue($return_value) { + $rule = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); + $rule->expects($this->once()) + ->method('check') + ->with($this->response, $this->request) + ->will($this->returnValue($return_value)); + + $this->policy->addPolicy($rule); + + $actual_result = $this->policy->check($this->response, $this->request); + $this->assertSame(NULL, $actual_result); + } + + /** + * Provides test data for testChainNoResultOnInvalidReturnValue. + * + * @return array + * Test input and expected result. + */ + public function providerChainNoResultOnInvalidReturnValue() { + return [ + [NULL], + [FALSE], + [0], + [1], + [TRUE], + [[1, 2, 3]], + [new \stdClass()], + ]; + } + + /** + * Asserts that check() returns imediately when a rule returned DENY. + */ + public function testStopChainOnFirstDeny() { + $rule1 = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); + $rule1->expects($this->once()) + ->method('check') + ->with($this->response, $this->request); + $this->policy->addPolicy($rule1); + + $deny_rule = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); + $deny_rule->expects($this->once()) + ->method('check') + ->with($this->response, $this->request) + ->will($this->returnValue(ResponsePolicyInterface::DENY)); + $this->policy->addPolicy($deny_rule); + + $ignored_rule = $this->getMock('Drupal\Core\PageCache\ResponsePolicyInterface'); + $ignored_rule->expects($this->never()) + ->method('check'); + $this->policy->addPolicy($ignored_rule); + + $actual_result = $this->policy->check($this->response, $this->request); + $this->assertsame(ResponsePolicyInterface::DENY, $actual_result); + } + +} diff --git a/core/tests/Drupal/Tests/Core/PageCache/CommandLineOrUnsafeMethodTest.php b/core/tests/Drupal/Tests/Core/PageCache/CommandLineOrUnsafeMethodTest.php new file mode 100644 index 0000000..bb54fbc --- /dev/null +++ b/core/tests/Drupal/Tests/Core/PageCache/CommandLineOrUnsafeMethodTest.php @@ -0,0 +1,83 @@ +policy = $this->getMock('Drupal\Core\PageCache\RequestPolicy\CommandLineOrUnsafeMethod', array('isCli')); + } + + /** + * Asserts that check() returns DENY for unsafe HTTP methods. + * + * @dataProvider providerTestHttpMethod + * @covers ::check + */ + public function testHttpMethod($expected_result, $method) { + $this->policy->expects($this->once()) + ->method('isCli') + ->will($this->returnValue(FALSE)); + + $request = Request::create('/', $method); + $actual_result = $this->policy->check($request); + $this->assertSame($expected_result, $actual_result); + } + + /** + * Provides test data and expected results for the HTTP method test. + * + * @return array + * Test data and expected results. + */ + public function providerTestHttpMethod() { + return [ + [NULL, 'GET'], + [NULL, 'HEAD'], + [RequestPolicyInterface::DENY, 'POST'], + [RequestPolicyInterface::DENY, 'PUT'], + [RequestPolicyInterface::DENY, 'DELETE'], + [RequestPolicyInterface::DENY, 'OPTIONS'], + [RequestPolicyInterface::DENY, 'TRACE'], + [RequestPolicyInterface::DENY, 'CONNECT'], + ]; + } + + /** + * Asserts that check() returns DENY if running from the command line. + * + * @covers ::check + */ + public function testIsCli() { + $this->policy->expects($this->once()) + ->method('isCli') + ->will($this->returnValue(TRUE)); + + $request = Request::create('/', 'GET'); + $actual_result = $this->policy->check($request); + $this->assertSame(RequestPolicyInterface::DENY, $actual_result); + } + +} diff --git a/core/tests/Drupal/Tests/Core/PageCache/NoSessionOpenTest.php b/core/tests/Drupal/Tests/Core/PageCache/NoSessionOpenTest.php new file mode 100644 index 0000000..f0fe4cf --- /dev/null +++ b/core/tests/Drupal/Tests/Core/PageCache/NoSessionOpenTest.php @@ -0,0 +1,54 @@ +sessionCookieName = 'B1ESkdf3V4F8u27myaSAShuuHc'; + $this->policy = new RequestPolicy\NoSessionOpen($this->sessionCookieName); + } + + /** + * Asserts that caching is allowed unless there is a session cookie present. + * + * @covers ::check + */ + public function testNoAllowUnlessSessionCookiePresent() { + $request = new Request(); + $result = $this->policy->check($request); + $this->assertSame(RequestPolicyInterface::ALLOW, $result); + + $request = Request::create('/', 'GET', [], [$this->sessionCookieName => 'some-session-id']); + $result = $this->policy->check($request); + $this->assertSame(NULL, $result); + } +}