From 41d09e0fa0b61e61bc3e2393b1fc2f1d85893ae2 Mon Sep 17 00:00:00 2001
From: Basil Crow <me@basilcrow.com>
Date: Thu, 9 Nov 2023 07:09:00 -0800
Subject: [PATCH] Stop recommending `-jnlpUrl` (#8639)

---
 .../main/java/hudson/slaves/JNLPLauncher.java | 56 +++++++++++++++++++
 .../hudson/slaves/JNLPLauncher/main.jelly     | 33 ++++++-----
 src/checkstyle/checkstyle-configuration.xml   |  2 +-
 3 files changed, 75 insertions(+), 16 deletions(-)

diff --git a/core/src/main/java/hudson/slaves/JNLPLauncher.java b/core/src/main/java/hudson/slaves/JNLPLauncher.java
index e8bf009eb9b3..917097d8267b 100644
--- a/core/src/main/java/hudson/slaves/JNLPLauncher.java
+++ b/core/src/main/java/hudson/slaves/JNLPLauncher.java
@@ -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;
@@ -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;
@@ -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()) {
+            sb.append("-webSocket ");
+        }
+        if (tunnel != null) {
+            sb.append(" -tunnel ");
+            sb.append(tunnel);
+            sb.append(' ');
+        }
+        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)) {
+            return input;
+        }
+        Escaper escaper =
+                Escapers.builder().addEscape('"', "\\\"").addEscape('`', "\\`").build();
+        return "\"" + escaper.escape(input) + "\"";
+    }
+
+    /**
+     * {@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)) {
+            return input;
+        }
+        Escaper escaper = Escapers.builder().addEscape('"', "\\\"").build();
+        return "\"" + escaper.escape(input) + "\"";
+    }
+
     /**
      * Gets work directory options as a String.
      *
diff --git a/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly b/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly
index 96066cc8e216..621d9ffdf068 100644
--- a/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly
+++ b/core/src/main/resources/hudson/slaves/JNLPLauncher/main.jelly
@@ -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>
diff --git a/src/checkstyle/checkstyle-configuration.xml b/src/checkstyle/checkstyle-configuration.xml
index d2454e254844..92261fb123bf 100644
--- a/src/checkstyle/checkstyle-configuration.xml
+++ b/src/checkstyle/checkstyle-configuration.xml
@@ -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"/>