Skip to content

Handle empty and non-mapping YAML and JSON config files - #142

Open
tas50 wants to merge 1 commit into
chef:mainfrom
tas50:fix-empty-and-non-hash-config-files
Open

tas50 wants to merge 1 commit into
chef:mainfrom
tas50:fix-empty-and-non-hash-config-files

Conversation

@tas50

@tas50 tas50 commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Empty YAML files crashed. An empty or comment-only YAML file parses to nil, so from_file("config.yml") raised NoMethodError: undefined method 'each' for nil. It is now treated as an empty config. A newly created config file with nothing set yet is common.

Files whose top level wasn't a mapping failed badly:

  • JSON [1, 2] raised NoMethodError: undefined method 'to_sym' for an instance of Integer.
  • YAML - one / - two raised nothing. It silently set options named one and two to nil.

Both now raise ArgumentError: config.yml must contain a mapping of config options at the top level, not Array.

The check lives in a new private from_parsed_file helper used by from_yaml and from_json. TOML is unaffected, since a TOML document is always a table. An empty JSON file already fails clearly with JSON::ParserError, so it is left alone.

Tests

Added specs for an empty YAML file, a comment-only YAML file, a top-level YAML list, and a top-level JSON array. All four failed before the change. The full suite passes and cookstyle is clean.

An empty or comment-only YAML file parses to nil, which crashed
from_hash with NoMethodError. Treat it as an empty config.

A file whose top level was a list either crashed (JSON) or silently set
each element as an option with a nil value (YAML). Raise an
ArgumentError that names the file instead.

Signed-off-by: Tim Smith <tsmith84@proton.me>
@tas50
tas50 force-pushed the fix-empty-and-non-hash-config-files branch from 627428a to 2d4ba3e Compare September 28, 2026 02:03
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.

1 participant