From fcb8550cb1145753dd1e8666c7cbd7aa9f7ef3f8 Mon Sep 17 00:00:00 2001
From: Mohamed ABDELLANI <abdellani@gmail.com>
Date: Tue, 16 May 2023 10:57:46 +0100
Subject: [PATCH] extract file path sanitizer to an independent class

---
 .../admin/product_import_controller.rb        |  4 +--
 app/services/file_path_sanitizer.rb           | 11 ++++++
 .../admin/product_import_controller_spec.rb   | 34 -------------------
 spec/services/file_path_sanitizer_spec.rb     | 33 ++++++++++++++++++
 4 files changed, 45 insertions(+), 37 deletions(-)
 create mode 100644 app/services/file_path_sanitizer.rb
 create mode 100644 spec/services/file_path_sanitizer_spec.rb

diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb
index 3a288eb538..3149735a5c 100644
--- a/app/controllers/admin/product_import_controller.rb
+++ b/app/controllers/admin/product_import_controller.rb
@@ -118,9 +118,7 @@ module Admin
     end
 
     def sanitize_file_path(file_path)
-      pathname = Pathname.new(file_path)
-      raise_invalid_file_path unless pathname.file?
-      pathname.realpath
+      FilePathSanitizer.new.sanitize(file_path, on_error: method(:raise_invalid_file_path))
     end
 
     def validate_file_path(file_path)
diff --git a/app/services/file_path_sanitizer.rb b/app/services/file_path_sanitizer.rb
new file mode 100644
index 0000000000..e853017edc
--- /dev/null
+++ b/app/services/file_path_sanitizer.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class FilePathSanitizer
+  def sanitize(file_path, on_error: nil)
+    pathname = Pathname.new(file_path)
+    return pathname.realpath if  pathname.file?
+
+    on_error&.call
+    false
+  end
+end
diff --git a/spec/controllers/admin/product_import_controller_spec.rb b/spec/controllers/admin/product_import_controller_spec.rb
index 431d8a2ba5..88f6793ad5 100644
--- a/spec/controllers/admin/product_import_controller_spec.rb
+++ b/spec/controllers/admin/product_import_controller_spec.rb
@@ -45,38 +45,4 @@ describe Admin::ProductImportController, type: :controller do
       end
     end
   end
-
-  describe 'sanitize_file_path' do
-    let(:folder_path){ '/tmp/product_import123' }
-    let(:file_path) { "#{folder_path}/import.csv" }
-
-    before do
-      FileUtils.mkdir_p(folder_path)
-      File.new(file_path, 'w') unless File.exist?(file_path)
-    end
-
-    def subject(path)
-      # expose the private method sanitize_file_path
-      described_class.class_eval { public :sanitize_file_path }
-      described_class.new.__send__(:sanitize_file_path, path)
-    end
-
-    it 'should covert relative path to absolute' do
-      path = "/tmp/product_import123/import.csv"
-      expect(subject(path).to_s).to eq file_path
-
-      path1 = "/../../tmp/product_import123/import.csv"
-      expect(subject(path1).to_s).to eq file_path
-
-      path2 = "/etc/../../../tmp/product_import123/import.csv"
-      expect(subject(path2).to_s).to eq file_path
-    end
-
-    it "raise an exception if the file doesn't exist" do
-      path = '/tmp/product_import123/import1.csv'
-      allow_any_instance_of(described_class).to receive(:raise_invalid_file_path)
-        .and_raise('Invalid File Path')
-      expect{ subject(path) }.to raise_error('Invalid File Path')
-    end
-  end
 end
diff --git a/spec/services/file_path_sanitizer_spec.rb b/spec/services/file_path_sanitizer_spec.rb
new file mode 100644
index 0000000000..4a47c2c1b2
--- /dev/null
+++ b/spec/services/file_path_sanitizer_spec.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe FilePathSanitizer do
+  let(:folder_path){ '/tmp/product_import123' }
+  let(:file_path) { "#{folder_path}/import.csv" }
+
+  before do
+    FileUtils.mkdir_p(folder_path)
+    File.new(file_path, 'w') unless File.exist?(file_path)
+  end
+  let(:object) { described_class.new }
+
+  it 'should covert relative path to absolute' do
+    path = "/tmp/product_import123/import.csv"
+    expect(object.sanitize(path).to_s).to eq file_path
+
+    path1 = "/../../tmp/product_import123/import.csv"
+    expect(object.sanitize(path1).to_s).to eq file_path
+
+    path2 = "/etc/../../../tmp/product_import123/import.csv"
+    expect(object.sanitize(path2).to_s).to eq file_path
+  end
+
+  it "call errors callback if the file doesn't exist" do
+    path = '/tmp/product_import123/import1.csv'
+    error_callback = double('error_callback')
+    expect(error_callback).to receive(:call)
+
+    expect( object.sanitize(path, on_error: error_callback) ).to eq(false)
+  end
+end