Skip to content

refactor(HTTP): optimize file path parsing in download() method - #10330

Merged
paulbalandan merged 6 commits into
codeigniter4:developfrom
gr8man:optimize-download-response
Jun 29, 2026
Merged

refactor(HTTP): optimize file path parsing in download() method#10330
paulbalandan merged 6 commits into
codeigniter4:developfrom
gr8man:optimize-download-response

Conversation

@gr8man

Copy link
Copy Markdown
Contributor

Description
This PR optimizes the file path parsing logic in the ResponseTrait::download() method.

What was changed and why:

  1. Replaced the explode('/', str_replace(DIRECTORY_SEPARATOR, '/', $filename)) array-based approach with PHP's native basename($filename) function. This reduces memory allocation (no array garbage collection) and is significantly faster (~4.5x faster in benchmarks).
  2. Removed the redundant local $filepath variable and double conditionals by assigning directly to the constructor within the physical file path block. This reduces cognitive load and improves readability.
  3. The method signature remains completely unchanged to ensure strict Backward Compatibility (BC).
  4. Added testGetDownloadResponseByExtremeFilePath to unit tests to ensure that basename() securely and properly handles file names containing tricky symbols (e.g., !@#$%) across different operating systems.

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

@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 add a changelog entry.

Comment threadtests/system/HTTP/ResponseTest.php Outdated
Comment threadtests/system/HTTP/ResponseTest.php Outdated
@michalsnmichalsn added the refactor Pull requests that refactor code label Jun 23, 2026
@gr8man
gr8manforce-pushed the optimize-download-response branch from 1dc27d8 to d78460eCompareJune 23, 2026 21:38
Comment threadtests/system/HTTP/ResponseTest.php Outdated
@gr8man
gr8manforce-pushed the optimize-download-response branch from 9569f36 to 10d2a54CompareJune 24, 2026 18:22
@michalsn

Copy link
Copy Markdown
Member

Crap... sorry - this is totally my fault.

Please remove the changelog entry. We don't add entries for refactoring, as there are no real changes for end users.

Comment threadtests/system/HTTP/ResponseTest.php
@gr8man
gr8manforce-pushed the optimize-download-response branch from c365e13 to efdc674CompareJune 25, 2026 20:23
@gr8man
gr8manforce-pushed the optimize-download-response branch from efdc674 to c661ee3CompareJune 25, 2026 20:24

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

Thank you!

@paulbalandanpaulbalandan changed the title refactor(HTTP): optimize file path parsing in download() methodrefactor(HTTP): optimize file path parsing in download() methodJun 29, 2026
@paulbalandan
paulbalandan merged commit fa890c2 into codeigniter4:developJun 29, 2026
56 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactorPull requests that refactor code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@gr8man@michalsn@paulbalandan