Skip to content

Commit 947d2aa

Browse files
Proposal: HTTP Response - Fix CSP non existing object when CSP is disabled
If CSP is disabled property $CSP in HTTP/Response is not initialized. If we try to access the CSP methods on the request object anywhere in code with CSP disabled it will crash the framework with "Call to a member function …. on null " In order to avoid this CSP object can be initiated regardless of CSP config. I’m aware that this is not the most efficient way to bypass the issue but some mechanism for disabling CSP should exist without having to do modifications everywhere in code. Maybe better idea will be to create mock class to be loaded instead which will respond with catchall magic methods like __call __set __get ….. But I don’t know if it is worth doing it as it will require adding additional class in framework. Ref #2456 Added unit test testCSPDisabled()
1 parent fedae3c commit 947d2aa

2 files changed

Lines changed: 18 additions & 8 deletions

File tree

system/HTTP/Response.php

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,10 @@ public function __construct($config)
239239
// Also ensures that a Cache-control header exists.
240240
$this->noCache();
241241

242-
// Are we enforcing a Content Security Policy?
243-
if ($config->CSPEnabled === true)
244-
{
245-
$this->CSP = new ContentSecurityPolicy(new \Config\ContentSecurityPolicy());
246-
$this->CSPEnabled = true;
247-
}
242+
// We need CSP object even if not enabled to avoid calls to non existing methods
243+
$this->CSP = new ContentSecurityPolicy(new \Config\ContentSecurityPolicy());
248244

245+
$this->CSPEnabled = $config->CSPEnabled;
249246
$this->cookiePrefix = $config->cookiePrefix;
250247
$this->cookieDomain = $config->cookieDomain;
251248
$this->cookiePath = $config->cookiePath;

tests/system/HTTP/ContentSecurityPolicyTest.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ class ContentSecurityPolicyTest extends \CIUnitTestCase
1313
{
1414

1515
// Having this method as setUp() doesn't work - can't find Config\App !?
16-
protected function prepare()
16+
protected function prepare(bool $CSPEnabled = true)
1717
{
1818
$config = new App();
19-
$config->CSPEnabled = true;
19+
$config->CSPEnabled = $CSPEnabled;
2020
$this->response = new Response($config);
2121
$this->response->pretend(false);
2222
$this->csp = $this->response->CSP;
@@ -490,4 +490,17 @@ public function testHeaderIgnoreCase()
490490
$this->assertContains("base-uri 'self';", $result);
491491
}
492492

493+
/**
494+
* @runInSeparateProcess
495+
* @preserveGlobalState disabled
496+
*/
497+
public function testCSPDisabled()
498+
{
499+
$this->prepare(false);
500+
$result = $this->work();
501+
$this->response->CSP->addStyleSrc('https://example.com');
502+
503+
$this->assertHeaderNotEmitted('content-security-policy', true);
504+
}
505+
493506
}

0 commit comments

Comments
 (0)