Skip to content
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

(FACT-1317) Fix string values that look like booleans #1269

Merged
merged 2 commits into from
Feb 10, 2016

Conversation

MikaelSmith
Copy link

YAML will read boolean strings as boolean types. When the schema states
the Fact type is a string, we should ensure it's read as a string by
YAML parsers. Quote any boolean literal representation in a
string_value.

Also fix a prior mistake in the verify_facts acceptance test, which no
longer needs to check for boolean classes for a string.

@joshcooper
Copy link
Contributor

Looks like travis is complaining about trailing white space:

/home/travis/build/puppetlabs/facter/lib/src/util/string.cc:124:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

@joshcooper
Copy link
Contributor

For ruby 2.1.5p273, YAML.load doesn't parse single character y|n as boolean. I don't know if that we care or if it depends on a particular yaml version.

irb(main):012:0> inputs = "y|Y|yes|Yes|YES|n|N|no|No|NO|true|True|TRUE|false|False|FALSE|on|On|ON|off|Off|OFF".split('|')
=> ["y", "Y", "yes", "Yes", "YES", "n", "N", "no", "No", "NO", "true", "True", "TRUE", "false", "False", "FALSE", "on", "On", "ON", "off", "Off", "OFF"]
irb(main):013:0> outputs = inputs.select {|value| YAML.load(value).is_a?(TrueClass) }
=> ["yes", "Yes", "YES", "true", "True", "TRUE", "on", "On", "ON"]
irb(main):014:0> inputs - outputs
=> ["y", "Y", "n", "N", "no", "No", "NO", "false", "False", "FALSE", "off", "Off", "OFF"]

YAML will read boolean strings as boolean types. When the schema states
the Fact type is a string, we should ensure it's read as a string by
YAML parsers. Quote any boolean literal representation in a
string_value.

Also fix a prior mistake in the verify_facts acceptance test, which no
longer needs to check for boolean classes for a string.
@MikaelSmith
Copy link
Author

Overquoting shouldn't be an issue, I just tried to avoid doing it in some legacy cases (like ipv4 addresses). Pushed a fix for the whitespace.

joshcooper added a commit that referenced this pull request Feb 10, 2016
(FACT-1317) Fix string values that look like booleans
@joshcooper joshcooper merged commit 39ad907 into puppetlabs:master Feb 10, 2016
@johnduarte
Copy link

@MikaelSmith does this need to get merged into stable as well?

@MikaelSmith
Copy link
Author

oh oops, yes. I'll cherry-pick up.

@MikaelSmith MikaelSmith deleted the FACT-1317 branch February 10, 2016 22:48
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.

3 participants