Merge pull request #3373 from betagouv/reinforce-clamav-service-response

If ClamavService fails then raise an error

app/services/clamav_service.rb

@@ -1,15 +1,21 @@
 class ClamavService
   def self.safe_file?(file_path)
     if Rails.env.development?
-      Rails.logger.info("Rails.env = development => fake scan") # FIXME : remove me
       return true
     end
 
     FileUtils.chmod(0666, file_path)
 
     client = ClamAV::Client.new
-    response = client.execute(ClamAV::Commands::ScanCommand.new(file_path))
+    response = client.execute(ClamAV::Commands::ScanCommand.new(file_path)).first
-    Rails.logger.info("ClamAV response for #{file_path} : #{response.first.class.name}") # FIXME : remove me
+    if response.class == ClamAV::SuccessResponse
-    response.first.class != ClamAV::VirusResponse
+      true
+    elsif response.class == ClamAV::VirusResponse
+      false
+    elsif response.class == ClamAV::ErrorResponse
+      raise "ClamAV ErrorResponse : #{response.error_str}"
+    else
+      raise "ClamAV unkown response #{response.class.name}"
+    end
   end
 end

spec/services/clamav_service_spec.rb

@@ -4,17 +4,27 @@ describe ClamavService do
   describe '.safe_file?' do
     let(:path_file) { '/tmp/plop.txt' }
 
-    subject { ClamavService.safe_file? path_file }
+    subject { ClamavService.safe_file?(path_file) }
 
     before do
-      client = instance_double("ClamAV::Client", :execute => [ClamAV::SuccessResponse])
+      client = double("ClamAV::Client", execute: [response])
       allow(ClamAV::Client).to receive(:new).and_return(client)
+      allow(FileUtils).to receive(:chmod).with(0666, path_file).and_return(true)
     end
 
-    it 'change permission of file path' do
+    context 'When response type is ClamAV::SuccessResponse' do
-      allow(FileUtils).to receive(:chmod).with(0666, path_file).and_return(true)
+      let(:response) { ClamAV::SuccessResponse.new("OK") }
+      it { expect(subject).to eq(true) }
+    end
 
-      subject
+    context 'When response type is ClamAV::VirusResponse' do
+      let(:response) { ClamAV::VirusResponse.new("KO", "VirusN4ame") }
+      it { expect(subject).to eq(false) }
+    end
+
+    context 'When response type is ClamAV::ErrorResponse' do
+      let(:response) { ClamAV::ErrorResponse.new("File not found") }
+      it { expect { subject }.to raise_error("ClamAV ErrorResponse : File not found") }
     end
   end
 end