Skip to content

fix(config): handle null from yaml_parse_file in Yaml adapter #16776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 23, 2025

Conversation

ucando
Copy link

@ucando ucando commented Jun 22, 2025

Description

This PR fixes a potential bug in Phalcon\Config\Adapter\Yaml where yaml_parse_file() may return null when parsing an empty YAML file. Without proper handling, this could result in a null value being passed to the parent constructor, leading to unexpected errors.

Changes

  • ✅ Added a null check in the constructor of Yaml adapter and fallback to an empty array.
  • ✅ Added a unit test (configAdapterYamlConstructHandlesEmptyYaml) to ensure that empty YAML files return an empty configuration array without throwing an exception.
  • ✅ Added tests/_data/assets/config/empty.yml as a test fixture file.

ucando added 3 commits June 22, 2025 23:17
### Fixed

- Fixed `Phalcon\Config\Adapter\Yaml` constructor to handle `null` return values from `yaml_parse_file()`, ensuring empty configuration files are treated as empty arrays instead of throwing errors.
### Added

- Added a unit test to verify that empty YAML files are handled correctly and return an empty config array instead of causing errors.
empty config file
@niden
Copy link
Member

niden commented Jun 23, 2025

Also an entry in the CHANGELOG for this change please.

@niden
Copy link
Member

niden commented Jun 23, 2025

And it seems you need to run composer cs-fix to fix PHPCS issues.

Check the Github Actions for where the issues are

Thanks

@niden
Copy link
Member

niden commented Jun 23, 2025

Thank you @ucando

@niden niden merged commit 1f8c232 into phalcon:5.0.x Jun 23, 2025
41 of 42 checks passed
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