Skip to content

Commit

Permalink
Stop recommending -jnlpUrl (#8639)
Browse files Browse the repository at this point in the history
  • Loading branch information
@basil
basil committed Nov 9, 2023
1 parent e4fe504 commit 41d09e0
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 16 deletions.
56 changes: 56 additions & 0 deletions core/src/main/java/hudson/slaves/JNLPLauncher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

package hudson.slaves;

import com.google.common.escape.Escaper;
import com.google.common.escape.Escapers;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
Expand All @@ -38,6 +40,7 @@
import jenkins.slaves.RemotingWorkDirSettings;
import jenkins.util.SystemProperties;
import jenkins.websocket.WebSockets;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -195,6 +198,59 @@ public void launch(SlaveComputer computer, TaskListener listener) {
@Restricted(NoExternalUse.class)
public static /*almost final*/ Descriptor<ComputerLauncher> DESCRIPTOR;

@NonNull
@Restricted(NoExternalUse.class)
public String getRemotingOptionsUnix(@NonNull Computer computer) {
return getRemotingOptions(escapeUnix(computer.getName()));
}

@NonNull
@Restricted(NoExternalUse.class)
public String getRemotingOptionsWindows(@NonNull Computer computer) {
return getRemotingOptions(escapeWindows(computer.getName()));
}

private String getRemotingOptions(String computerName) {
StringBuilder sb = new StringBuilder();
sb.append("-name ");
sb.append(computerName);
sb.append(' ');
if (isWebSocket()) {

Check warning on line 218 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 218 is only partially covered, one branch is missing
sb.append("-webSocket ");

Check warning on line 219 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line 

Line 219 is not covered by tests
}
if (tunnel != null) {

Check warning on line 221 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 221 is only partially covered, one branch is missing
sb.append(" -tunnel ");
sb.append(tunnel);
sb.append(' ');

Check warning on line 224 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 222-224 are not covered by tests
}
return sb.toString();
}

/**
* {@link Jenkins#checkGoodName(String)} saves us from most troublesome characters, but we still have to deal with
* spaces and therefore with double quotes and backticks.
*/
private static String escapeUnix(String input) {
if (StringUtils.isAlphanumeric(input)) {

Check warning on line 234 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 234 is only partially covered, one branch is missing
return input;
}
Escaper escaper =
Escapers.builder().addEscape('"', "\\\"").addEscape('`', "\\`").build();
return "\"" + escaper.escape(input) + "\"";

Check warning on line 239 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 238-239 are not covered by tests
}

/**
* {@link Jenkins#checkGoodName(String)} saves us from most troublesome characters, but we still have to deal with
* spaces and therefore with double quotes.
*/
private static String escapeWindows(String input) {
if (StringUtils.isAlphanumeric(input)) {

Check warning on line 247 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 247 is only partially covered, one branch is missing
return input;
}
Escaper escaper = Escapers.builder().addEscape('"', "\\\"").build();
return "\"" + escaper.escape(input) + "\"";

Check warning on line 251 in core/src/main/java/hudson/slaves/JNLPLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines 

Lines 250-251 are not covered by tests
}

/**
* Gets work directory options as a String.
*
Expand Down
33 changes: 18 additions & 15 deletions core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,71 +35,74 @@ THE SOFTWARE.
<j:set var="jenkinsURL" value="${h.inferHudsonURL(request)}"/>
<j:set var="copy_agent_jar_unix" value="curl -sO ${jenkinsURL}jnlpJars/agent.jar" />
<j:set var="copy_agent_jar_windows" value="curl.exe -sO ${jenkinsURL}jnlpJars/agent.jar" />
<j:set var="copy_java_cmd" value="java -jar agent.jar -jnlpUrl ${jenkinsURL}${it.url}jenkins-agent.jnlp ${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_unix" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_windows" value="java -jar agent.jar -url ${jenkinsURL} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" />
<j:if test="${h.hasPermission(it, it.CONNECT)}">
<j:choose>
<j:when test="${it.ACL.hasPermission(app.ANONYMOUS, it.CONNECT)}">
<h3>
${%slaveAgent.cli.run} (Unix)
<l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd}"/>
<l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd_unix}"/>
</h3>
<pre>
${copy_agent_jar_unix}
${copy_java_cmd}
${copy_java_cmd_unix}
</pre>

<h3>
${%slaveAgent.cli.run} (Windows)
<l:copyButton text="${copy_agent_jar_windows} &amp; ${copy_java_cmd}"/>
<l:copyButton text="${copy_agent_jar_windows} &amp; ${copy_java_cmd_windows}"/>
</h3>
<pre>
${copy_agent_jar_windows}
${copy_java_cmd}
${copy_java_cmd_windows}
</pre>

</j:when>
<j:otherwise>
<j:set var="copy_java_cmd_secret" value="java -jar agent.jar -jnlpUrl ${jenkinsURL}${it.url}jenkins-agent.jnlp -secret ${it.jnlpMac} ${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_secret_unix" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_secret_windows" value="java -jar agent.jar -url ${jenkinsURL} -secret ${it.jnlpMac} ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" />
<h3>
${%slaveAgent.cli.run} (Unix)
<l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd_secret}"/>
<l:copyButton text="${copy_agent_jar_unix};${copy_java_cmd_secret_unix}"/>
</h3>
<!-- TODO conceal secret w/ JS if possible -->
<pre>
${copy_agent_jar_unix}
${copy_java_cmd_secret}
${copy_java_cmd_secret_unix}
</pre>

<h3>
${%slaveAgent.cli.run} (Windows)
<l:copyButton text="${copy_agent_jar_windows} &amp; ${copy_java_cmd_secret}"/>
<l:copyButton text="${copy_agent_jar_windows} &amp; ${copy_java_cmd_secret_windows}"/>
</h3>
<!-- TODO conceal secret w/ JS if possible -->
<pre>
${copy_agent_jar_windows}
${copy_java_cmd_secret}
${copy_java_cmd_secret_windows}
</pre>

<j:set var="copy_secret_to_file" value="echo ${it.jnlpMac} &gt; secret-file" />
<j:set var="copy_java_cmd_secret2" value="java -jar agent.jar -jnlpUrl ${jenkinsURL}${it.url}jenkins-agent.jnlp -secret @secret-file ${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_secret2_unix" value="java -jar agent.jar -url ${jenkinsURL} -secret @secret-file ${it.launcher.getRemotingOptionsUnix(it)}${it.launcher.getWorkDirOptions(it)}" />
<j:set var="copy_java_cmd_secret2_windows" value="java -jar agent.jar -url ${jenkinsURL} -secret @secret-file ${it.launcher.getRemotingOptionsWindows(it)}${it.launcher.getWorkDirOptions(it)}" />
<h3>
${%slaveAgent.cli.run.secret} (Unix)
<l:copyButton text="${copy_secret_to_file};${copy_agent_jar_unix};${copy_java_cmd_secret2}"/>
<l:copyButton text="${copy_secret_to_file};${copy_agent_jar_unix};${copy_java_cmd_secret2_unix}"/>
</h3>
<pre>
${copy_secret_to_file}
${copy_agent_jar_unix}
${copy_java_cmd_secret2}
${copy_java_cmd_secret2_unix}
</pre>

<h3>
${%slaveAgent.cli.run.secret} (Windows)
<l:copyButton text="${copy_secret_to_file} &amp; ${copy_agent_jar_windows} &amp; ${copy_java_cmd_secret2}"/>
<l:copyButton text="${copy_secret_to_file} &amp; ${copy_agent_jar_windows} &amp; ${copy_java_cmd_secret2_windows}"/>
</h3>
<pre>
${copy_secret_to_file}
${copy_agent_jar_windows}
${copy_java_cmd_secret2}
${copy_java_cmd_secret2_windows}
</pre>
</j:otherwise>
</j:choose>
Expand Down
2 changes: 1 addition & 1 deletion src/checkstyle/checkstyle-configuration.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
<!-- prevent the use of jsr-305 annotations and Spring utilities -->
<property name="illegalClasses" value="javax.annotation.MatchesPattern.Checker, javax.annotation.Nonnegative.Checker, javax.annotation.Nonnull.Checker, javax.annotation.RegEx.Checker, javax.annotation.CheckForNull, javax.annotation.CheckForSigned, javax.annotation.CheckReturnValue, javax.annotation.Detainted, javax.annotation.MatchesPattern, javax.annotation.Nonnegative, javax.annotation.Nonnull, javax.annotation.Nullable, javax.annotation.OverridingMethodsMustInvokeSuper, javax.annotation.ParametersAreNonnullByDefault, javax.annotation.ParametersAreNullableByDefault, javax.annotation.PropertyKey, javax.annotation.RegEx, javax.annotation.Signed, javax.annotation.Syntax, javax.annotation.Tainted, javax.annotation.Untainted, javax.annotation.WillClose, javax.annotation.WillCloseWhenClosed, javax.annotation.WillNotClose, javax.annotation.concurrent.GuardedBy, javax.annotation.concurrent.Immutable, javax.annotation.concurrent.NotThreadSafe, javax.annotation.concurrent.ThreadSafe, javax.annotation.meta.TypeQualifierValidator, javax.annotation.meta.When, javax.annotation.meta.Exclusive, javax.annotation.meta.Exhaustive, javax.annotation.meta.TypeQualifier, javax.annotation.meta.TypeQualifierDefault, javax.annotation.meta.TypeQualifierNickname, org.apache.log4j.Logger, org.springframework.util.Assert, org.springframework.util.StringUtils"/>
<!-- Prevent the expansion of Guava usages and ban internal library packages -->
<property name="illegalPkgs" value="com.google.common.base, com.google.common.escape, com.google.common.eventbus, com.google.common.graph, com.google.common.hash, com.google.common.html, com.google.common.io, com.google.common.math, com.google.common.net, com.google.common.reflect, com.google.common.xml, com.google.thirdparty, jline.internal"/>
<property name="illegalPkgs" value="com.google.common.base, com.google.common.eventbus, com.google.common.graph, com.google.common.hash, com.google.common.html, com.google.common.io, com.google.common.math, com.google.common.net, com.google.common.reflect, com.google.common.xml, com.google.thirdparty, jline.internal"/>
</module>
<module name="RedundantImport"/>
<module name="UnusedImports"/>
Expand Down

0 comments on commit 41d09e0

Please sign in to comment.