Skip to content

(MODULES-905) Narrow the confinement in bool2str #258

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 2 commits into from
May 16, 2014
Merged

(MODULES-905) Narrow the confinement in bool2str #258

merged 2 commits into from
May 16, 2014

Conversation

mckern
Copy link
Contributor

@mckern mckern commented May 15, 2014

Previously, bool2str() accepted a broad array of boolean values and
bare strings, without any attempt to validate that the strings in any
way resembled "true" or "false" (or any of the other values bool2num()
accepts). This commit narrows the input confinement to TrueClass and
FalseClass, which means that bool2str() will only interpolate
strict boolean values now.

Previously, bool2str() accepted a broad array of boolean values and
bare strings, without any attempt to validate that the strings in any
way resembled "true" or "false" (or any of the other values bool2num()
accepts). This commit narrows the input confinement to TrueClass and
FalseClass, which means that bool2str() will only interpolate
strict boolean values now.
@apenney
Copy link

apenney commented May 15, 2014

I hate to be a massive pain but can we add a test that proves it fails? :)

@mckern
Copy link
Contributor Author

mckern commented May 15, 2014

Sure. It'll take me a little bit to circle back around to them but I'll throw it over the wall today.

The extended spec tests validate that the common types of values
that could be passed to bool2str() are rejected.
apenney pushed a commit that referenced this pull request May 16, 2014
(MODULES-905) Narrow the confinement in bool2str
@apenney apenney merged commit 0cda858 into puppetlabs:master May 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants