Skip to content

Commit 8846b40

Browse files
committed
#369 Fix BC break introduced in 3.5
The BC break was caused by the changes to `ContentSecurityPolicyListener::getReport` and `ContentSecurityPolicyListener::getEnforcement`. In 3.5, these methods return a new DirectiveSet instead of the same instance as in previous versions. Code that used these methods to modify the DirectiveSets no longer saw these modifications reflected in the CSP header. This is fixed by making `ConfigurationDirectiveSetBuilder` (the default builder) always return the same DirectiveSet by building it once rather than on each call.
1 parent 0b81201 commit 8846b40

File tree

3 files changed

+56
-18
lines changed

3 files changed

+56
-18
lines changed

phpstan-baseline.neon

+16
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,19 @@ parameters:
7171
"""
7272
count: 1
7373
path: tests/Listener/XssProtectionListenerTest.php
74+
75+
-
76+
message: """
77+
#^Call to deprecated method getEnforcement\\(\\) of class Nelmio\\\\SecurityBundle\\\\EventListener\\\\ContentSecurityPolicyListener\\:
78+
Use `nelmio_security\\.directive_set_builder\\.enforce` instead\\.$#
79+
"""
80+
count: 1
81+
path: tests/Listener/ContentSecurityPolicyListenerTest.php
82+
83+
-
84+
message: """
85+
#^Call to deprecated method getReport\\(\\) of class Nelmio\\\\SecurityBundle\\\\EventListener\\\\ContentSecurityPolicyListener\\:
86+
Use `nelmio_security\\.directive_set_builder\\.report` instead\\.$#
87+
"""
88+
count: 1
89+
path: tests/Listener/ContentSecurityPolicyListenerTest.php

src/ContentSecurityPolicy/ConfigurationDirectiveSetBuilder.php

+3-18
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,7 @@
1515

1616
class ConfigurationDirectiveSetBuilder implements DirectiveSetBuilderInterface
1717
{
18-
private PolicyManager $policyManager;
19-
20-
/**
21-
* @phpstan-var array{
22-
* enforce?: array<string, mixed>,
23-
* report?: array<string, mixed>,
24-
* } $config
25-
*/
26-
private array $config;
27-
28-
/**
29-
* @phpstan-var 'enforce'|'report' $kind
30-
*/
31-
private string $kind;
18+
private DirectiveSet $directiveSet;
3219

3320
/**
3421
* @phpstan-param array{
@@ -39,14 +26,12 @@ class ConfigurationDirectiveSetBuilder implements DirectiveSetBuilderInterface
3926
*/
4027
public function __construct(PolicyManager $policyManager, array $config, string $kind)
4128
{
42-
$this->policyManager = $policyManager;
43-
$this->config = $config;
44-
$this->kind = $kind;
29+
$this->directiveSet = DirectiveSet::fromConfig($policyManager, $config, $kind);
4530
}
4631

4732
public function buildDirectiveSet(): DirectiveSet
4833
{
49-
return DirectiveSet::fromConfig($this->policyManager, $this->config, $this->kind);
34+
return $this->directiveSet;
5035
}
5136

5237
/**

tests/Listener/ContentSecurityPolicyListenerTest.php

+37
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
namespace Nelmio\SecurityBundle\Tests\Listener;
1515

16+
use Nelmio\SecurityBundle\ContentSecurityPolicy\ConfigurationDirectiveSetBuilder;
1617
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSet;
1718
use Nelmio\SecurityBundle\ContentSecurityPolicy\DirectiveSetBuilderInterface;
1819
use Nelmio\SecurityBundle\ContentSecurityPolicy\NonceGeneratorInterface;
@@ -481,6 +482,42 @@ public function testLegacyConstructorCreatesDirectiveSetBuilder(): void
481482
$this->assertSame('script-src https://enforce.deprecation-test.example.com', $response->headers->get('Content-Security-Policy'));
482483
}
483484

485+
public function testChangesThroughGetReportAreReflectedInTheHeader(): void
486+
{
487+
$listener = new ContentSecurityPolicyListener(
488+
new ConfigurationDirectiveSetBuilder(new PolicyManager(), [], 'report'),
489+
new ConfigurationDirectiveSetBuilder(new PolicyManager(), [], 'enforce'),
490+
$this->nonceGenerator,
491+
$this->shaComputer
492+
);
493+
494+
$report = $listener->getReport();
495+
$report->setDirective('script-src', 'https://example.test');
496+
497+
$response = $this->callListener($listener, '/', true);
498+
499+
$header = $response->headers->get('Content-Security-Policy-Report-Only');
500+
$this->assertSame('script-src https://example.test', $header);
501+
}
502+
503+
public function testChangesThroughGetEnforcementAreReflectedInTheHeader(): void
504+
{
505+
$listener = new ContentSecurityPolicyListener(
506+
new ConfigurationDirectiveSetBuilder(new PolicyManager(), [], 'report'),
507+
new ConfigurationDirectiveSetBuilder(new PolicyManager(), [], 'enforce'),
508+
$this->nonceGenerator,
509+
$this->shaComputer
510+
);
511+
512+
$enforce = $listener->getEnforcement();
513+
$enforce->setDirective('script-src', 'https://example.test');
514+
515+
$response = $this->callListener($listener, '/', true);
516+
517+
$header = $response->headers->get('Content-Security-Policy');
518+
$this->assertSame('script-src https://example.test', $header);
519+
}
520+
484521
/**
485522
* @param array<string, string|true> $directives
486523
* @param list<string> $contentTypes

0 commit comments

Comments
 (0)