Skip to content
New issue

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

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

Already on GitHub? # to your account

add single quotes for credentials #374

Closed
wants to merge 1 commit into from

Conversation

iakovgan
Copy link
Contributor

also strip inital credentials for single quotes, for backward compatibility

also strip inital credentials for single quotes, for backward compatibility
@jenkinsadmin
Copy link

Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests

JENKINS_USERNAME="<%= @ui_user -%>"
JENKINS_PASSWORD="<%= @ui_pass -%>"
# credentials should be single quoted
JENKINS_USERNAME="'<%= @ui_user.gsub!(/^\'|\'?$/, '') -%>'"
Copy link
Member

Choose a reason for hiding this comment

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

This will raise an exception when @ui_user is nil (undef).

Why isn't JENKINS_USERNAME='<%= @ui_user -%>' sufficient to stop the shell from interpolating the string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are right - exception on undef is really bad.

This regexp supposed to remove quotes if user input is already using quoted value as a workaround. So we could prevent double quoting.

@jhoblitt
Copy link
Member

jhoblitt commented Oct 3, 2015

A possible alternative solution would be to use shellquote().

@jhoblitt jhoblitt added bug Something isn't working needs-tests labels Oct 11, 2015
@rtyler rtyler added this to the 1.6.0 - Kato milestone Oct 11, 2015
rtyler pushed a commit to rtyler/puppet-jenkins that referenced this pull request Oct 11, 2015
@jhoblitt
Copy link
Member

@iakovgan This PR was fixed up and merged as #395. Thank you for contributing!

@jhoblitt jhoblitt closed this Oct 11, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working needs-tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants