From 09c0d9b658374203f5b6ce75926241bc19511c92 Mon Sep 17 00:00:00 2001
From: Oleg Nenashev <o.v.nenashev@gmail.com>
Date: Sun, 5 Feb 2017 16:34:07 +0100
Subject: [PATCH 1/4] [JENKINS-41730] - Add option to ignore the
 X-Jenkins-Agent-Protocols header + improve diagnostics

- [x] Add the `org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader` property, which disables the cache
- [x] Print warning in the case if the list of supported protocols is empty
- [x] Add documentation
---
 docs/configuration.md                         | 12 +++++++--
 .../engine/JnlpAgentEndpointResolver.java     | 27 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/docs/configuration.md b/docs/configuration.md
index 55b7a2bf2..d7c5a50dd 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -99,10 +99,18 @@ These properties require independent configuration on both sides of the channel.
       <td>org.jenkinsci.remoting.nio.NioChannelHub.disabled</td>
       <td>false</td>
       <td>2.62.3</td>
-      <td>?</td>
+      <td>TODO</td>
       <td><a href="https://issues.jenkins-ci.org/browse/JENKINS-39290">JENKINS-39290</a></td>
       <td>Boolean flag to disable NIO-based socket connection handling, and switch back to classic IO. Used to isolate the problem.</td>
-    </tr>-
+    </tr>
+    <tr>
+      <td>org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader</td>
+      <td>false</td>
+      <td>TODO</td>    
+      <td>TODO</td>
+      <td><a href="https://issues.jenkins-ci.org/browse/JENKINS-41730">JENKINS-41730</a></td>
+      <td>If enabled, the remoting client will ignore the list of supported protocols returned by the server. Then it will try to connect by iterating through available protocols till it finds the server accepts connection.</td>
+    </tr>
     <!--Template
     <tr>
       <td></td>
diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
index 562454669..7b42a82f1 100644
--- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
+++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -76,6 +76,17 @@ public class JnlpAgentEndpointResolver {
 
     private String tunnel;
 
+    /**
+     * If enabled, remoting will ignore the list of supported protocols returned by the server.
+     * This option can be used in order to workaround issues when the headers cannot be delivered
+     * from the server due to whatever reason (e.g. JENKINS-41730).
+     * Ignoring of the supported protocol headers leads to higher connection times because remoting agent 
+     * has to try connecting protocols till it finds the supported one.
+     * @since TODO
+     */
+    private static final boolean IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER =
+            Boolean.getBoolean(JnlpAgentEndpointResolver.class.getName() + ".ignoreJenkinsAgentProtocolsHeader");
+    
     public JnlpAgentEndpointResolver(String... jenkinsUrls) {
         this.jenkinsUrls = new ArrayList<String>(Arrays.asList(jenkinsUrls));
     }
@@ -165,6 +176,15 @@ public JnlpAgentEndpoint resolve() throws IOException {
                 List<String> protocols = header(con, "X-Jenkins-Agent-Protocols");
                 if (protocols != null) {
                     agentProtocolNames = new HashSet<String>();
+                    
+                    if (protocols.isEmpty() && !IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER) {
+                        LOGGER.log(Level.WARNING, "Received the empty list of supported protocols from the server. " +
+                                "All protocols are disabled on the master side OR the 'X-Jenkins-Agent-Protocols' header is corrupted (JENKINS-41730). " +
+                                "The agent won't be able to connect, in the case of the header corruption consider using " +
+                                "'-Dorg.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader=true' " +
+                                "to disable caching of the supported protocols.");
+                    }
+                    
                     for (String names : protocols) {
                         for (String name : names.split(",")) {
                             name = name.trim();
@@ -233,7 +253,12 @@ public int compare(String o1, String o2) {
                     if (tokens[1].length() > 0) port = Integer.parseInt(tokens[1]);
                 }
                 
-                return new JnlpAgentEndpoint(host, port, identity, agentProtocolNames, selectedJenkinsURL);
+                if (IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER) {
+                    LOGGER.log(Level.INFO, "Ignoring the cached list of remoting protocols provided by the server. " +
+                            "'-Dorg.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader' flag is set");
+                }
+                
+                return new JnlpAgentEndpoint(host, port, identity, IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER ? null : agentProtocolNames, selectedJenkinsURL);
             } finally {
                 con.disconnect();
             }

From 7d7237fa81318f859f91b2e9cc6e2815b9a29e33 Mon Sep 17 00:00:00 2001
From: Oleg Nenashev <o.v.nenashev@gmail.com>
Date: Mon, 6 Feb 2017 14:34:07 +0100
Subject: [PATCH 2/4] [JENKINS-41730] - Switch to
 org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry
 according to the feedback from @stephenc

---
 docs/configuration.md                         |  4 +-
 .../engine/JnlpAgentEndpointResolver.java     | 52 ++++++++++++-------
 2 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/docs/configuration.md b/docs/configuration.md
index d7c5a50dd..c6affbb4d 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -104,12 +104,12 @@ These properties require independent configuration on both sides of the channel.
       <td>Boolean flag to disable NIO-based socket connection handling, and switch back to classic IO. Used to isolate the problem.</td>
     </tr>
     <tr>
-      <td>org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader</td>
+      <td>org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry</td>
       <td>false</td>
       <td>TODO</td>    
       <td>TODO</td>
       <td><a href="https://issues.jenkins-ci.org/browse/JENKINS-41730">JENKINS-41730</a></td>
-      <td>If enabled, the remoting client will ignore the list of supported protocols returned by the server. Then it will try to connect by iterating through available protocols till it finds the server accepts connection.</td>
+      <td>If specified, only the protocols from the list will be tried during the connection. The option provides protocol names, but the order of the check is defined internally and cannot be changed.</td>
     </tr>
     <!--Template
     <tr>
diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
index 7b42a82f1..16a0966e2 100644
--- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
+++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -77,15 +77,14 @@ public class JnlpAgentEndpointResolver {
     private String tunnel;
 
     /**
-     * If enabled, remoting will ignore the list of supported protocols returned by the server.
-     * This option can be used in order to workaround issues when the headers cannot be delivered
+     * If specified, only the protocols from the list will be tried during the connection.
+     * The option provides protocol names, but the order of the check is defined internally and cannot be changed.
+     * This option can be also used in order to workaround issues when the headers cannot be delivered
      * from the server due to whatever reason (e.g. JENKINS-41730).
-     * Ignoring of the supported protocol headers leads to higher connection times because remoting agent 
-     * has to try connecting protocols till it finds the supported one.
      * @since TODO
      */
-    private static final boolean IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER =
-            Boolean.getBoolean(JnlpAgentEndpointResolver.class.getName() + ".ignoreJenkinsAgentProtocolsHeader");
+    private static String PROTOCOL_NAMES_TO_TRY =
+            System.getProperty(JnlpAgentEndpointResolver.class.getName() + ".protocolNamesToTry");
     
     public JnlpAgentEndpointResolver(String... jenkinsUrls) {
         this.jenkinsUrls = new ArrayList<String>(Arrays.asList(jenkinsUrls));
@@ -175,16 +174,8 @@ public JnlpAgentEndpoint resolve() throws IOException {
                 host = defaultString(first(header(con, "X-Jenkins-JNLP-Host")), salURL.getHost());
                 List<String> protocols = header(con, "X-Jenkins-Agent-Protocols");
                 if (protocols != null) {
+                    // Take the list of protocols to try from the headers
                     agentProtocolNames = new HashSet<String>();
-                    
-                    if (protocols.isEmpty() && !IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER) {
-                        LOGGER.log(Level.WARNING, "Received the empty list of supported protocols from the server. " +
-                                "All protocols are disabled on the master side OR the 'X-Jenkins-Agent-Protocols' header is corrupted (JENKINS-41730). " +
-                                "The agent won't be able to connect, in the case of the header corruption consider using " +
-                                "'-Dorg.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader=true' " +
-                                "to disable caching of the supported protocols.");
-                    }
-                    
                     for (String names : protocols) {
                         for (String name : names.split(",")) {
                             name = name.trim();
@@ -193,7 +184,31 @@ public JnlpAgentEndpoint resolve() throws IOException {
                             }
                         }
                     }
+                    
+                    if (agentProtocolNames.isEmpty()) {
+                        LOGGER.log(Level.WARNING, "Received the empty list of supported protocols from the server. " +
+                                "All protocols are disabled on the master side OR the 'X-Jenkins-Agent-Protocols' header is corrupted (JENKINS-41730). " +
+                                "In the case of the header corruption as a workaround you can use the " +
+                                "'org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry' system property " +
+                                "to define the supported protocols.");
+                    }
+                    
+                    LOGGER.log(Level.INFO, "Remoting server accepts the following protocols: {0}", agentProtocolNames);
+                }
+                
+                if (PROTOCOL_NAMES_TO_TRY != null) {
+                    // Take a list of protocols to try from the system property
+                    agentProtocolNames = new HashSet<String>();
+                    LOGGER.log(Level.INFO, "Ignoring the list of supported remoting protocols provided by the server, because the " +
+                        "'org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry' property is defined. Will try {0}", PROTOCOL_NAMES_TO_TRY);
+                    for (String name : PROTOCOL_NAMES_TO_TRY.split(",")) {
+                        name = name.trim();
+                        if (!name.isEmpty()) {
+                            agentProtocolNames.add(name);
+                        } 
+                    }
                 }
+                
                 String idHeader = first(header(con, "X-Instance-Identity"));
                 RSAPublicKey identity = null;
                 if (idHeader != null) {
@@ -253,12 +268,9 @@ public int compare(String o1, String o2) {
                     if (tokens[1].length() > 0) port = Integer.parseInt(tokens[1]);
                 }
                 
-                if (IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER) {
-                    LOGGER.log(Level.INFO, "Ignoring the cached list of remoting protocols provided by the server. " +
-                            "'-Dorg.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.ignoreJenkinsAgentProtocolsHeader' flag is set");
-                }
                 
-                return new JnlpAgentEndpoint(host, port, identity, IGNORE_SUPPORTED_PROTOCOL_NAMES_HEADER ? null : agentProtocolNames, selectedJenkinsURL);
+                
+                return new JnlpAgentEndpoint(host, port, identity, agentProtocolNames, selectedJenkinsURL);
             } finally {
                 con.disconnect();
             }

From bc7a1a123d3f8b015abd3100031834f3f776bbb2 Mon Sep 17 00:00:00 2001
From: Oleg Nenashev <o.v.nenashev@gmail.com>
Date: Mon, 13 Feb 2017 13:38:51 +0300
Subject: [PATCH 3/4] [JENKINS-41730] - Pront the list of supported protocols
 iff it is not empty

---
 .../jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
index 16a0966e2..6681d3f21 100644
--- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
+++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -191,9 +191,9 @@ public JnlpAgentEndpoint resolve() throws IOException {
                                 "In the case of the header corruption as a workaround you can use the " +
                                 "'org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver.protocolNamesToTry' system property " +
                                 "to define the supported protocols.");
+                    } else {
+                        LOGGER.log(Level.INFO, "Remoting server accepts the following protocols: {0}", agentProtocolNames);
                     }
-                    
-                    LOGGER.log(Level.INFO, "Remoting server accepts the following protocols: {0}", agentProtocolNames);
                 }
                 
                 if (PROTOCOL_NAMES_TO_TRY != null) {

From 5177e40483138e3d18919c7f23697f0abcbc4d6b Mon Sep 17 00:00:00 2001
From: Oleg Nenashev <o.v.nenashev@gmail.com>
Date: Mon, 13 Feb 2017 13:40:52 +0300
Subject: [PATCH 4/4] [JENKINS-41730] - Get rid of unnecessary line breaks

---
 .../jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java    | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
index 6681d3f21..3aed699c1 100644
--- a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
+++ b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -268,8 +268,6 @@ public int compare(String o1, String o2) {
                     if (tokens[1].length() > 0) port = Integer.parseInt(tokens[1]);
                 }
                 
-                
-                
                 return new JnlpAgentEndpoint(host, port, identity, agentProtocolNames, selectedJenkinsURL);
             } finally {
                 con.disconnect();