-
Notifications
You must be signed in to change notification settings - Fork 270
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
Fix shell_escape of unless command #550
Conversation
A bug was introduced in 74ea1de . It resulted in an error: ``` Error: /Stage[main]/Java::Config/Exec[create-java-alternatives]: Could not evaluate: Could not find command 'alternatives\' ``` The effective exec was: ```puppet exec{'create-java-alternatives': path => '/usr/bin:/usr/sbin:/bin:/sbi command => ['alternatives', '--install', '/usr/bin/java', 'java', $java::use_java_alternative_path, '20000'], unless => shell_escape("alternatives --display java | grep -q $java::use_java_alternative_path}"), } ``` This errors since the spaces inside the `shell_escape` are also escaped to `\ `. This patch only shell_escapes the external supplied string `java::java_alternative_path` which was I expect the object the exercise. Configuration to trigger the bug, but it should not be hard. ``` class{'java': distribution => 'jdk', java_alternative => '/usr/lib/jvm/java-11-openjdk/bin/java', java_alternative_path => '/usr/lib/jvm/java-11-openjdk/bin/java', java_home => '/usr/lib/jvm/java-11-openjdk/', package => 'java-11-openjdk-devel', version => 'present', } ```
|
|
java::config is a classthat may have no external impact to Forge modules. This module is declared in 79 of 580 indexed public
|
|
@treydock for info 9.0.1 is broken your keycloak module which you don't support yet :-) |
|
Avoiding this completely using https://forge.puppet.com/modules/puppet/alternatives would be a good idea. |
CLA has be signed for me on behalf of CERN IT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for catching this
A bug was introduced in 74ea1de .
It resulted in an error:
The effective exec was:
This errors since the spaces inside the
shell_escapeare also escaped to\.This patch only shell_escapes the external supplied string
java::java_alternative_pathwhich was I expect the object the exercise.Configuration to trigger the bug, but it should not be hard.