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

[JENKINS-66347] Keystore not working #208

Merged
merged 1 commit into from Aug 11, 2021
Merged

Conversation

basil
Copy link
Member

@basil basil commented Aug 11, 2021

See JENKINS-66347. My testing for #207 didn't include paths with spaces, so I introduced a regression when the keystore path contains spaces. Here I'm adding quoting for anything that could contain spaces:

  • --httpsKeyStore
  • --extraLibFolder
  • JENKINS_JAVA_CMD
  • JENKINS_HOME
  • JENKINS_WAR

To plumb through the result to daemonize(1) I need to use eval in POSIX shell. Cleaner alternatives are possible in bash(1), but this is a plain sh(1) script.

To make the code easier to read I inlined JAVA_CMD. It was only used once, so the extra level of indirection just made the code harder to read.

Tested by putting a space in JENKINS_HOME and --extraLibFolder and verifying that the arguments were correctly parsed:

[basil@fedora ~]$ sudo cat /proc/2690/cmdline | tr '\0' '\n'
/etc/alternatives/java
-Djava.awt.headless=true
-DJENKINS_HOME=/var/lib/jenkinstest 2
-jar
/usr/lib/jenkinstest/jenkinstest.war
--logfile=/var/log/jenkinstest/jenkinstest.log
--webroot=/var/cache/jenkinstest/war
--httpPort=7777
--debug=5
--handlerCountMax=100
--handlerCountMaxIdle=20
--extraLibFolder=/home/basil/folder 1

@basil
Copy link
Member Author

basil commented Aug 11, 2021

With a little more effort I tested this change with a --httpsKeyStore path that contained a space and a --httpsKeyStorePassword that contained both a space and a dollar sign, so I'm pretty confident this will fix the user-reported issue.

@timja timja merged commit 57d0345 into jenkinsci:master Aug 11, 2021
@basil basil deleted the JENKINS-66347 branch August 11, 2021 13:30
MarkEWaite added a commit to MarkEWaite/packaging that referenced this pull request Aug 11, 2021
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.

None yet

2 participants