-http mode broke when the user omitted the mandatory final `/` in the Jenkins URL.

cli/src/main/java/hudson/cli/CLI.java

@@ -146,19 +146,16 @@ public class CLI implements AutoCloseable {
         this.authorization = factory.authorization;
         ExecutorService exec = factory.exec;
         
-        String url = jenkins.toExternalForm();
-        if(!url.endsWith("/"))  url+='/';
-
         ownsPool = exec==null;
         pool = exec!=null ? exec : Executors.newCachedThreadPool(new NamingThreadFactory(Executors.defaultThreadFactory(), "CLI.pool"));
 
         Channel _channel;
         try {
-            _channel = connectViaCliPort(jenkins, getCliTcpPort(url));
+            _channel = connectViaCliPort(jenkins, getCliTcpPort(jenkins));
         } catch (IOException e) {
             LOGGER.log(Level.FINE, "Failed to connect via CLI port. Falling back to HTTP", e);
             try {
-                _channel = connectViaHttp(url);
+                _channel = connectViaHttp(jenkins);
             } catch (IOException e2) {
                 e.addSuppressed(e2);
                 throw e;
@@ -177,12 +174,11 @@ public class CLI implements AutoCloseable {
      * @deprecated Specific to {@link Mode#REMOTING}.
      */
     @Deprecated
-    private Channel connectViaHttp(String url) throws IOException {
+    private Channel connectViaHttp(URL url) throws IOException {
         LOGGER.log(FINE, "Trying to connect to {0} via Remoting over HTTP", url);
-        URL jenkins = new URL(url + "cli?remoting=true");
 
-        FullDuplexHttpStream con = new FullDuplexHttpStream(jenkins,authorization);
-        Channel ch = new Channel("Chunked connection to "+jenkins,
+        FullDuplexHttpStream con = new FullDuplexHttpStream(url, "cli?remoting=true", authorization);
+        Channel ch = new Channel("Chunked connection to " + url,
                 pool,con.getInputStream(),con.getOutputStream());
         final long interval = 15*1000;
         final long timeout = (interval * 3) / 4;
@@ -303,13 +299,14 @@ public class CLI implements AutoCloseable {
 
     /**
      * If the server advertises CLI endpoint, returns its location.
+     * @deprecated Specific to {@link Mode#REMOTING}.
      */
-    protected CliPort getCliTcpPort(String url) throws IOException {
-        URL _url = new URL(url);
-        if (_url.getHost()==null || _url.getHost().length()==0) {
+    @Deprecated
+    protected CliPort getCliTcpPort(URL url) throws IOException {
+        if (url.getHost()==null || url.getHost().length()==0) {
             throw new IOException("Invalid URL: "+url);
         }
-        URLConnection head = _url.openConnection();
+        URLConnection head = url.openConnection();
         try {
             head.connect();
         } catch (IOException e) {
@@ -561,6 +558,10 @@ public class CLI implements AutoCloseable {
             return -1;
         }
 
+        if (!url.endsWith("/")) {
+            url += '/';
+        }
+
         if(args.isEmpty())
             args = Arrays.asList("help"); // default to help
 
@@ -643,7 +644,7 @@ public class CLI implements AutoCloseable {
 
     private static int sshConnection(String jenkinsUrl, String user, List<String> args, PrivateKeyProvider provider, final boolean strictHostKey) throws IOException {
         Logger.getLogger(SecurityUtils.class.getName()).setLevel(Level.WARNING); // suppress: BouncyCastle not registered, using the default JCE provider
-        URL url = new URL(jenkinsUrl + "/login");
+        URL url = new URL(jenkinsUrl + "login");
         URLConnection conn = url.openConnection();
         String endpointDescription = conn.getHeaderField("X-SSH-Endpoint");
 
@@ -711,8 +712,7 @@ public class CLI implements AutoCloseable {
 
     private static int plainHttpConnection(String url, List<String> args, CLIConnectionFactory factory) throws IOException, InterruptedException {
         LOGGER.log(FINE, "Trying to connect to {0} via plain protocol over HTTP", url);
-        URL jenkins = new URL(url + "cli?remoting=false");
-        FullDuplexHttpStream streams = new FullDuplexHttpStream(jenkins, factory.authorization);
+        FullDuplexHttpStream streams = new FullDuplexHttpStream(new URL(url), "cli?remoting=false", factory.authorization);
         class ClientSideImpl extends PlainCLIProtocol.ClientSide {
             boolean complete;
             int exit = -1;

cli/src/main/java/hudson/cli/CliPort.java

@@ -8,9 +8,9 @@ import java.security.KeyFactory;
 import java.security.PublicKey;
 import java.security.spec.X509EncodedKeySpec;
 
-/**
- * @author Kohsuke Kawaguchi
- */
+    /**
+     * @deprecated Specific to Remoting mode.
+     */
 public final class CliPort {
     /**
      * The TCP endpoint to talk to.

cli/src/main/java/hudson/cli/FullDuplexHttpStream.java

@@ -20,7 +20,7 @@ import org.apache.commons.codec.binary.Base64;
  * @author Kohsuke Kawaguchi
  */
 public class FullDuplexHttpStream {
-    private final URL target;
+    private final URL base;
     /**
      * Authorization header value needed to get through the HTTP layer.
      */
@@ -48,16 +48,31 @@ public class FullDuplexHttpStream {
         return null;
     }
 
+    @Deprecated
+    public FullDuplexHttpStream(URL target, String authorization) throws IOException {
+        this(new URL(target.toString().replaceFirst("/cli.*$", "")), target.toString().replaceFirst("(.+)/cli.*$", "$1"), authorization);
+    }
+
     /**
+     * @param base the base URL of Jenkins
      * @param target
      *      The endpoint that we are making requests to.
      * @param authorization
      *      The value of the authorization header, if non-null.
      */
-    public FullDuplexHttpStream(URL target, String authorization) throws IOException {
-        this.target = target;
+    public FullDuplexHttpStream(URL base, String relativeTarget, String authorization) throws IOException {
+        if (!base.toString().endsWith("/")) {
+            throw new IllegalArgumentException(base.toString());
+        }
+        if (relativeTarget.startsWith("/")) {
+            throw new IllegalArgumentException(relativeTarget);
+        }
+
+        this.base = base;
         this.authorization = authorization;
 
+        URL target = new URL(base, relativeTarget);
+
         CrumbData crumbData = new CrumbData();
 
         UUID uuid = UUID.randomUUID(); // so that the server can correlate those two connections
@@ -128,8 +143,7 @@ public class FullDuplexHttpStream {
     	}
 
     	private String createCrumbUrlBase() {
-            String url = target.toExternalForm();    		
-            return new StringBuilder(url.substring(0, url.lastIndexOf("/cli"))).append("/crumbIssuer/api/xml/").toString();
+            return base + "crumbIssuer/api/xml/";
     	}
 
     	private String readData(String dest) throws IOException {

test/src/test/java/hudson/cli/CLIActionTest.java

@@ -68,7 +68,7 @@ public class CLIActionTest {
     public void testDuplexHttp() throws Exception {
         pool = Executors.newCachedThreadPool();
         try {
-            FullDuplexHttpStream con = new FullDuplexHttpStream(new URL(j.getURL(), "cli"), null);
+            FullDuplexHttpStream con = new FullDuplexHttpStream(j.getURL(), "cli", null);
             Channel ch = new ChannelBuilder("test connection", pool).build(con.getInputStream(), con.getOutputStream());
             ch.close();
         } finally {
@@ -80,7 +80,7 @@ public class CLIActionTest {
     public void security218() throws Exception {
         pool = Executors.newCachedThreadPool();
         try {
-            FullDuplexHttpStream con = new FullDuplexHttpStream(new URL(j.getURL(), "cli"), null);
+            FullDuplexHttpStream con = new FullDuplexHttpStream(j.getURL(), "cli", null);
             Channel ch = new ChannelBuilder("test connection", pool).build(con.getInputStream(), con.getOutputStream());
             ch.call(new Security218());
             fail("Expected the call to be rejected");
@@ -241,7 +241,8 @@ public class CLIActionTest {
         FileUtils.copyURLToFile(j.jenkins.getJnlpJars("jenkins-cli.jar").getURL(), jar);
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         assertEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
-            "java", "-Dfile.encoding=ISO-8859-2", "-Duser.language=cs", "-Duser.country=CZ", "-jar", jar.getAbsolutePath(), "-s", j.getURL().toString(),"-noKeyAuth", "test-diagnostic").
+            "java", "-Dfile.encoding=ISO-8859-2", "-Duser.language=cs", "-Duser.country=CZ", "-jar", jar.getAbsolutePath(),
+                "-s", j.getURL().toString()./* just checking */replaceFirst("/$", ""), "-noKeyAuth", "test-diagnostic").
             stdout(baos).stderr(System.err).join());
         assertEquals("encoding=ISO-8859-2 locale=cs_CZ", baos.toString().trim());
         // TODO test that stdout/stderr are in expected encoding (not true of -remoting mode!)

test/src/test/java/hudson/cli/CLITest.java

@@ -109,7 +109,7 @@ public class CLITest {
         assertThat(baos.toString(), containsString("Authenticated as: admin"));
         baos = new ByteArrayOutputStream();
         assertEquals(0, new Launcher.LocalLauncher(StreamTaskListener.fromStderr()).launch().cmds(
-            "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString(), "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath(), "-strictHostKey", "who-am-i"
+            "java", "-Duser.home=" + home, "-jar", jar.getAbsolutePath(), "-s", r.getURL().toString()./* just checking */replaceFirst("/$", ""), "-ssh", "-user", "admin", "-i", privkey.getAbsolutePath(), "-strictHostKey", "who-am-i"
         ).stdout(baos).stderr(System.err).join());
         assertThat(baos.toString(), containsString("Authenticated as: admin"));
     }

test/src/test/java/jenkins/security/Security218BlackBoxTest.java

@@ -277,7 +277,7 @@ public class Security218BlackBoxTest {
                         try {
                             CLI cli = new CLI(r.getURL()) {
                                 @Override
-                                protected CliPort getCliTcpPort(String url) throws IOException {
+                                protected CliPort getCliTcpPort(URL url) throws IOException {
                                     return new CliPort(new InetSocketAddress(proxySocket.getInetAddress(), proxySocket.getLocalPort()), /* ignore identity */ null, 1);
                                 }
                             };