Skip to content

fix(Router): remove esc(strip_tags()) from RouteCollection default setters - #10517

Open
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/route-collection-esc-identifiers
Open

fix(Router): remove esc(strip_tags()) from RouteCollection default setters#10517
gr8man wants to merge 1 commit into
codeigniter4:developfrom
gr8man:fix/route-collection-esc-identifiers

Conversation

@gr8man

@gr8man gr8man commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description
RouteCollection::setDefaultNamespace(), setDefaultController(), and setDefaultMethod() previously applied esc(strip_tags()) to their input arguments.

esc() is designed for HTML escaping in view contexts and encodes HTML entities (such as & into &), which is inappropriate for internal PHP identifiers (namespaces, controller class names, method names). This PR removes esc(strip_tags()) from these setters so that identifiers are preserved as intended without HTML entity conversion.

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

@michalsn michalsn 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.

I agree with the direction: HTML escaping does not belong on routing identifiers, especially since the Config\Routing constructor path already stores these values unescaped.

However, the PR currently demonstrates a cleanup rather than a patch-level bug. For valid UTF-8 PHP identifiers, the old code leaves permitted characters unchanged. Values such as Foo&Bar are not valid declared identifiers, and the tests only verify storage rather than a failed dispatch. Removing strip_tags() also makes the change broader than preventing entity encoding.

Please:

  • describe this as removing inappropriate input transformation and move it from Bugs Fixed to Changes;
  • update resource() and presenter() similarly, or explain why they are excluded. The $from handling in create() is a separate concern and should remain outside this PR.

Without a reproducible dispatch failure, I would target the 4.8 branch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants