Modernize library for 4.x: security fixes, strict types, tooling updates#25
Merged
Conversation
Bug and security fixes (each with a regression test):
- Cap placeholder expansion at 25 passes / 1 MiB to prevent memory
exhaustion from circular references with surrounding text
- Ignore HTTP_* keys in $_SERVER for ${env.*} expansion, since those
originate from client-supplied request headers in a web context
- Expand falsy environment variables (e.g. VAR=0) correctly by checking
getenv() against false instead of truthiness
- Preserve non-string types (bool, int, float) when expanding via
reference data; expandPropertyWithReferenceData() now returns mixed
- Expand single-placeholder strings once instead of twice, eliminating
duplicate logger/stringifier side effects
- Restore the original value if preg_replace_callback() fails
Modernization (BC breaks documented in RELEASE.md):
- Add declare(strict_types=1) and full type declarations everywhere
- Make StringifierInterface::stringifyArray() an instance method
- Require array type for expandArrayProperties() $reference_array
Tooling and CI:
- Update PHPUnit ^9 to ^10.5 || ^11 || ^12 || ^13; migrate config and
convert tests to attributes with static data providers
- Add phpstan (level 5) to require-dev and the composer test script
- Drop abandoned greg-1-anderson/composer-test-scenarios
- Run CI on main and *.x branches; add PHP 8.5 to the matrix, a
composer audit step, checkout@v6, and Dependabot config
- Reach 100% line/method/class coverage; clean up env-var fixtures
between tests
- Fix README example syntax error, .gitignore malformed line, stale
RELEASE.md script references; add .editorconfig and composer metadata
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Multi-dimension review and modernization of the library for the 4.x major version. 55 candidate findings were adversarially verified; the 35 confirmed ones are fixed here, each behavior change developed test-first.
Bug and security fixes (each with a regression test)
['a' => 'x${b}', 'b' => 'y${a}']) doubled the value length every pass until the process crashed. Expansion is now capped at 25 passes / 1 MiB.${env.*}no longer readsHTTP_*keys from$_SERVER— in a web context those come from client-supplied request headers (Symfony precedent).VAR=0previously failed a truthiness check ongetenv().expandPropertyWithReferenceData()declared?string, silently coercing booleans to"1"and numerics to strings. Now returnsmixed.preg_replace_callback()failure now restores the original value instead of nulling it.Modernization (BC breaks documented in RELEASE.md)
declare(strict_types=1)and full parameter/return types everywhere.StringifierInterface::stringifyArray()is now an instance method.expandArrayProperties()requiresarray $reference_array.Tooling and CI
^9→^10.5 || ^11 || ^12 || ^13; config migrated (the old config silently ran zero tests under PHPUnit 13), tests converted to attributes with static providers, env-var fixtures cleaned up intearDown().composer test.greg-1-anderson/composer-test-scenarios.mainand*.x(pushes to4.xpreviously never ran CI), adds PHP 8.5 to the matrix, acomposer auditstep,checkout@v6, and Dependabot for composer + GitHub Actions..gitignorelast line, staleRELEASE.mdscript references; added.editorconfigand composer metadata.Test plan
composer test(lint + PHPUnit + phpcs + phpstan) passes locally on PHP 8.4🤖 Generated with Claude Code