Skip to content

fix: normalize string values and keys in ContentSecurityPolicy::addOption() - #10636

Open
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/csp-add-option-string-conversion
Open

gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/csp-add-option-string-conversion

Conversation

@gr8man

@gr8man gr8man commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Description
Fixed ContentSecurityPolicy::addOption() and addToHeader() converting string values directly into array keys, which created invalid entries for empty strings, prevented keyword quoting for space-separated lists, and caused a fatal TypeError in PHP 8 strict mode when numeric strings were converted to integer array keys.

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@carson-codeigniter4 carson-codeigniter4 Bot added the bug Verified issues on the current code behavior or pull requests that will fix them label Oct 13, 2026
@michalsn
michalsn requested a balanced review from Copilot October 18, 2026 09:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Empty-value handling and mixed reporting modes can silently remove enforced security restrictions.

4 open issues
What changed in this PR

Normalizes Content Security Policy values to address keyword quoting, empty entries, and numeric-key type errors.

Changes:

  • Splits configured strings and normalizes header keys.
  • Adds regression tests for normalization and sandbox flags.
  • Documents the fix in the changelog.

Base branch is unspecified; develop compatibility rules were assumed. Validation used targeted code inspection; automated tests remain for CI.

File Description
user_guide_src/​source/​changelogs/​v4.7.6.rst Documents the normalization fix.
tests/​system/​HTTP/​ContentSecurityPolicyTest.php Adds normalization regression tests.
system/​HTTP/​ContentSecurityPolicy.php Changes option normalization and header generation.

Review effort: Balanced

Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
@gr8man
gr8man force-pushed the fix/csp-add-option-string-conversion branch from 74c4afa to 6e01df9 Compare October 18, 2026 21:54
@carson-codeigniter4 carson-codeigniter4 Bot added the stale Pull requests with conflicts label Oct 18, 2026
@carson-codeigniter4

This comment has been minimized.

@gr8man
gr8man force-pushed the fix/csp-add-option-string-conversion branch from 6e01df9 to 607c93d Compare October 18, 2026 21:57
@carson-codeigniter4 carson-codeigniter4 Bot removed the stale Pull requests with conflicts label Oct 18, 2026
@paulbalandan paulbalandan changed the title fix: normalize string values and keys in ContentSecurityPolicy::addOption() fix: normalize string values and keys in ContentSecurityPolicy::addOption() Oct 19, 2026
@michalsn
michalsn requested a balanced review from Copilot October 19, 2026 17:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
Comment on lines +908 to +917
if (in_array($target, ['sandbox', 'frameAncestors'], true) && isset($this->{$target}[''])) {
$emptyReportOnly = $this->{$target}[''];

foreach ($this->{$target} as $val => $reportOnly) {
if ($val !== '' && $reportOnly === $emptyReportOnly) {
unset($this->{$target}['']);
break;
}
}
}
Comment thread system/HTTP/ContentSecurityPolicy.php Outdated
Comment on lines +1048 to +1050
if (! in_array($name, ['sandbox', 'frame-ancestors'], true)) {
return;
}
@carson-codeigniter4 carson-codeigniter4 Bot added the github_actions Pull requests that update Github_actions code label Oct 19, 2026
…tion()

- Normalize space-separated and empty strings into array options via stringToOptions()
- Cast numeric string array keys back to string to prevent TypeError under strict types
- Preserve empty source lists as deny-all directives across all directives
- Retain empty markers when other options move between reporting modes
- Add comprehensive regression tests for config and runtime directives
@gr8man
gr8man force-pushed the fix/csp-add-option-string-conversion branch from a37fe7c to 3fbbfb9 Compare October 19, 2026 19:51
@carson-codeigniter4 carson-codeigniter4 Bot removed the github_actions Pull requests that update Github_actions code label Oct 19, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants