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.

Sign up for GitHub

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

Jump to bottom

(maint) - Cause dirname to fail when passed undef #913

Closed
wants to merge 1 commit into from
Closed

(maint) - Cause dirname to fail when passed undef #913

wants to merge 1 commit into from

Conversation

b4ldr
Copy link
Collaborator

@b4ldr b4ldr commented May 30, 2018

currently the dirname function returns '.' if passed undef. this PR causes dirname to fail in this instance, it will also cause dirname to fail if an empty string is passed.

test before patch

# puppet apply -e 'notify{dirname(undef): }'                                          
Notice: Compiled catalog for jobo-5685.local in environment production in 0.04 seconds
Notice: .
Notice: /Stage[main]/Main/Notify[.]/message: defined 'message' as '.'
Notice: Applied catalog in 0.02 seconds

test after patch

# puppet apply -e 'notify{dirname(undef): }' 
Error: Evaluation Error: Error while evaluating a Function Call, dirname(): Requires a none empty string (line: 1, column: 8) on node jobo-5685.local

@b4ldr b4ldr mentioned this pull request May 30, 2018
Closed
pmcmaw
pmcmaw reviewed Jun 15, 2018
View reviewed changes
lib/puppet/parser/functions/dirname.rb Outdated
@@ -16,6 +16,10 @@ module Puppet::Parser::Functions
unless arguments[0].is_a?(String)
raise(Puppet::ParseError, 'dirname(): Requires string as argument')
end
# undef is converted to an empty string ''
if arguments[0].empty?
raise(Puppet::ParseError, 'dirname(): Requires a none empty string as argument')
Copy link
Contributor

@pmcmaw pmcmaw Jun 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @b4ldr,
Can you update:
none empty to non-empty and I will get your change merged.

@pmcmaw pmcmaw changed the title cause dirname to fail when passed undef (maint) - Cause dirname to fail when passed undef Jun 15, 2018
@b4ldr b4ldr mentioned this pull request Jun 15, 2018
Merged
@b4ldr
Copy link
Collaborator Author

b4ldr commented Jun 15, 2018

@pmcmaw i some how messed up the rebase so i have created a new clean PR here #921

@b4ldr b4ldr closed this Jun 15, 2018
@pmcmaw
Copy link
Contributor

pmcmaw commented Jun 15, 2018

Thats great! Ill have a look at it, when it passes all tests I will work on getting it merged. Thank you for the quick response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmcmaw pmcmaw pmcmaw left review comments

Assignees
No one assigned
Labels
maintenance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@b4ldr @pmcmaw