From 0d5f35f19c4b60b67af3f2acc977fca129236b36 Mon Sep 17 00:00:00 2001
From: Filippo Gangi Dino <filippo.gangidino@welaika.com>
Date: Tue, 17 Apr 2018 10:19:46 +0200
Subject: [PATCH] Passing ftp connection params in extended format

---
 lib/photocopier/ftp.rb | 18 +++++++++++------
 spec/ftp_spec.rb       | 44 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/lib/photocopier/ftp.rb b/lib/photocopier/ftp.rb
index ac33d1f..0ac1561 100644
--- a/lib/photocopier/ftp.rb
+++ b/lib/photocopier/ftp.rb
@@ -45,7 +45,7 @@ def lftp(local, remote, reverse, exclude)
       command = [
           "set ftp:list-options -a",
           "set cmd:fail-exit true",
-          "open #{remote_ftp_url}",
+          "open #{remote_ftp_params}",
           "find -d 1 #{remote} || mkdir -p #{remote}",
           "lcd #{local}",
           "cd #{remote}",
@@ -55,14 +55,20 @@ def lftp(local, remote, reverse, exclude)
       run "lftp -c '#{command}'"
     end
 
+    def remote_ftp_params
+      params = []
+      if options[:user].present?
+        params << '--user'
+        params << CGI.escape(options[:user])
+        params << "--password #{CGI.escape(options[:password])}" if options[:password].present?
+      end
+      params << remote_ftp_url
+      params.join(' ')
+    end
+
     def remote_ftp_url
       url = options[:scheme].dup.presence || "ftp"
       url << "://"
-      if options[:user].present?
-        url << CGI.escape(options[:user])
-        url << ":#{CGI.escape(options[:password])}" if options[:password].present?
-        url << "@"
-      end
       url << options[:host]
       url
     end
diff --git a/spec/ftp_spec.rb b/spec/ftp_spec.rb
index 9e4c330..f24d5e9 100644
--- a/spec/ftp_spec.rb
+++ b/spec/ftp_spec.rb
@@ -21,29 +21,62 @@
     end
   end
 
-  context "#remote_ftp_url" do
+  context "#remote_ftp_params" do
     let(:options) do { host: "host" } end
 
     it "should build an ftp url" do
-      expect(ftp.send(:remote_ftp_url)).to eq("ftp://host")
+      expect(ftp.send(:remote_ftp_params)).to eq("ftp://host")
     end
 
     context "given an username" do
       let(:options) do { host: "host", user: "user" } end
 
       it "should add it to the url" do
-        expect(ftp.send(:remote_ftp_url)).to eq("ftp://user@host")
+        expect(ftp.send(:remote_ftp_params)).to eq("--user user ftp://host")
       end
 
       context "given a password" do
         let(:options) do { host: "host", user: "user", password: "password" } end
 
         it "should add it to the url" do
-          expect(ftp.send(:remote_ftp_url)).to eq("ftp://user:password@host")
+          expect(ftp.send(:remote_ftp_params)).to eq("--user user --password password ftp://host")
+        end
+      end
+
+      context "given a password with special chars" do
+        let(:options) do { host: "host", user: "user", password: "&Bk_qR]fJnW!" } end
+
+        it "should add it to the url" do
+          expect(ftp.send(:remote_ftp_params)).to eq("--user user --password %26Bk_qR%5DfJnW%21 ftp://host")
         end
       end
     end
 
+    context "given a scheme (protocol)" do
+      let(:options) do { scheme: "ftps", host: "host" } end
+
+      it "should add it to the url" do
+        expect(ftp.send(:remote_ftp_params)).to eq("ftps://host")
+      end
+
+      context "when called repeatedly" do
+        it "should add it to the url" do
+          ftp.send(:remote_ftp_params)
+          ftp.send(:remote_ftp_params)
+          expect(ftp.send(:remote_ftp_params)).to eq("ftps://host")
+        end
+      end
+
+    end
+  end
+
+  context "#remote_ftp_url" do
+    let(:options) do { host: "host" } end
+
+    it "should build an ftp url" do
+      expect(ftp.send(:remote_ftp_url)).to eq("ftp://host")
+    end
+
     context "given a scheme (protocol)" do
       let(:options) do { scheme: "ftps", host: "host" } end
 
@@ -58,7 +91,6 @@
           expect(ftp.send(:remote_ftp_url)).to eq("ftps://host")
         end
       end
-
     end
   end
 
@@ -104,7 +136,7 @@
       lftp_commands = [
         'set ftp:list-options -a',
         'set cmd:fail-exit true',
-        'open ftp://user:pass%21%22%27%2C%3B%24u%26V%5Es@example.com',
+        'open --user user --password pass%21%22%27%2C%3B%24u%26V%5Es ftp://example.com',
         'find -d 1 remote\\ dir || mkdir -p remote\\ dir',
         'lcd local\\ dir',
         'cd remote\\ dir',