Skip to content

fix: env() TypeError for non-string $_SERVER values + esc() fixes - #10305

Merged
paulbalandan merged 11 commits into
codeigniter4:developfrom
gr8man:fix/common-env-typeerror-esc-leak
Jun 15, 2026
Merged

fix: env() TypeError for non-string $_SERVER values + esc() fixes#10305
paulbalandan merged 11 commits into
codeigniter4:developfrom
gr8man:fix/common-env-typeerror-esc-leak

Conversation

@gr8man

Copy link
Copy Markdown
Contributor

Fix: env() TypeError in CLI + esc() improvements

What

Fixes two bugs in system/Common.php.

env() — In CLI, $_SERVER contains non-string values (argc is int, argv is array). Passing them to strtolower() threw a TypeError under declare(strict_types=1). Added an is_string() guard before the match expression; non-string values are returned as-is.

esc() — Three fixes applied together:

  • foreach ($data as &$value) left a dangling reference after the loop; added unset($value) to prevent silent mutation of the last array element in the calling scope
  • $encoding was not propagated in recursive array calls
  • Replaced single static $escaper with static $escapers[] keyed by encoding for correct per-encoding caching

Tests

Added a data-provider test for env() covering non-string $_SERVER/$_ENV values (int, array, float), and three tests verifying the esc() reference leak is gone.

Files changed

  • system/Common.php
  • tests/system/CommonFunctionsTest.php

- env(): guard non-string values (int argc, array argv in CLI) before
strtolower() to prevent TypeError under declare(strict_types=1)
- esc(): propagate $encoding in recursive array calls (was ignored before),
add early return after array processing, replace single static $escaper
with static $escapers[] cache keyed by encoding
- tests: data-provider test for env() non-string types,
three tests for esc() foreach reference leak
@gr8man
gr8manforce-pushed the fix/common-env-typeerror-esc-leak branch from 53a365d to 449dbaeCompareJune 11, 2026 21:43

@michalsnmichalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please send one change per PR.

This needs a changelog entry.

Comment threadsystem/Common.php Outdated
Comment threadsystem/Common.php Outdated
@michalsnmichalsn added the bug Verified issues on the current code behavior or pull requests that will fix them label Jun 12, 2026
Comment threadtests/system/CommonFunctionsTest.php Outdated
gr8manand others added 4 commits June 12, 2026 18:28
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
@michalsn

Copy link
Copy Markdown
Member

Apart from the changelog entry, we also need some tests to prove the fixed esc() behavior (with changing $encoding) is actually working.

@gr8man
gr8man requested a review from michalsnJune 12, 2026 17:43

@michalsnmichalsn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We still need two things - see my previous comment.

@gr8man
gr8man requested a review from michalsnJune 12, 2026 20:01
Comment threadtests/system/CommonFunctionsTest.php Outdated
Comment threadCHANGELOG.md Outdated
Comment threaduser_guide_src/source/changelogs/v4.7.4.rst Outdated
@github-actionsgithub-actionsBot added the stale Pull requests with conflicts label Jun 14, 2026
@github-actions

This comment was marked as outdated.

@gr8man
gr8man requested a review from michalsnJune 14, 2026 20:34
@michalsnmichalsn removed the stale Pull requests with conflicts label Jun 14, 2026
@paulbalandanpaulbalandan changed the title fix: env() TypeError for non-string $_SERVER values + esc() fixesfix: env() TypeError for non-string $_SERVER values + esc() fixesJun 15, 2026
@paulbalandan
paulbalandan merged commit e793e58 into codeigniter4:developJun 15, 2026
57 checks passed
@paulbalandan

Copy link
Copy Markdown
Member

Thank you, @gr8man

maniaba pushed a commit to maniaba/CodeIgniter4 that referenced this pull request Jun 17, 2026
…es (codeigniter4#10305)
* fix: env() TypeError for non-string $_SERVER values + esc() fixes
- env(): guard non-string values (int argc, array argv in CLI) before
strtolower() to prevent TypeError under declare(strict_types=1)
- esc(): propagate $encoding in recursive array calls (was ignored before),
add early return after array processing, replace single static $escaper
with static $escapers[] cache keyed by encoding
- tests: data-provider test for env() non-string types,
three tests for esc() foreach reference leak
* Apply suggestion from @michalsn
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
* Apply suggestion from @michalsn
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
* Update system/Common.php
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
* style: cs-fix
* test: add tests for esc() encoding changes and update changelog
* Update tests/system/CommonFunctionsTest.php
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
* docs: move env/esc fix changelog entry to v4.7.4.rst
* docs: split Common changelog entry
---------
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugVerified 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.

3 participants

@gr8man@michalsn@paulbalandan