Add new CodeOcean::File#read method

* With a new method, we can simplify our code to handle differences between file.content and file.native_file.read

app/controllers/submissions_controller.rb

@@ -28,7 +28,7 @@ class SubmissionsController < ApplicationController
     stringio = Zip::OutputStream.write_buffer do |zio|
       @files.each do |file|
         zio.put_next_entry(file.filepath.delete_prefix('/'))
-        zio.write(file.content.presence || file.native_file.read)
+        zio.write(file.read)
       end
 
       # zip exercise description
@@ -56,11 +56,7 @@ class SubmissionsController < ApplicationController
   def download_file
     raise Pundit::NotAuthorizedError if @embed_options[:disable_download]
 
-    if @file.native_file?
-      send_file(@file.native_file.path)
-    else
-      send_data(@file.content, filename: @file.name_with_extension)
-    end
+    send_data(@file.read, filename: @file.name_with_extension)
   end
 
   def index
@@ -71,7 +67,7 @@ class SubmissionsController < ApplicationController
 
   def render_file
     if @file.native_file?
-      send_file(@file.native_file.path, disposition: 'inline')
+      send_data(@file.read, filename: @file.name_with_extension, disposition: 'inline')
     else
       render(plain: @file.content)
     end

app/models/code_ocean/file.rb

@@ -56,6 +56,17 @@ module CodeOcean
       define_method("#{role}?") { self.role == role }
     end
 
+    def read
+      if native_file?
+        valid = Pathname(native_file.current_path).fnmatch? ::File.join(native_file.root, '**')
+        return nil unless valid
+
+        native_file.read
+      else
+        content
+      end
+    end
+
     def ancestor_id
       file_id || id
     end
@@ -83,12 +94,7 @@ module CodeOcean
     end
 
     def hash_content
-      self.hashed_content = Digest::MD5.new.hexdigest(if file_type.try(:binary?)
-                                                        ::File.new(native_file.file.path,
-                                                          'r').read
-                                                      else
-                                                        content
-                                                      end)
+      self.hashed_content = Digest::MD5.new.hexdigest(read || '')
     end
     private :hash_content
 

app/services/proforma_service/convert_exercise_to_task.rb

@@ -112,11 +112,11 @@ module ProformaService
 
     def add_content_to_task_file(file, task_file)
       if file.native_file.present?
-        file = ::File.new(file.native_file.file.path, 'r')
-        task_file.content = file.read
+        file_content = file.read
+        task_file.content = file_content
         task_file.used_by_grader = false
         task_file.binary = true
-        task_file.mimetype = MimeMagic.by_magic(file).type
+        task_file.mimetype = MimeMagic.by_magic(file_content).type
       else
         task_file.content = file.content
         task_file.used_by_grader = true

lib/runner/strategy/docker_container_pool.rb

@@ -93,16 +93,12 @@ class Runner::Strategy::DockerContainerPool < Runner::Strategy
       end
 
       local_file_path = local_path(file.filepath)
-      if file.file_type.binary?
-        FileUtils.cp(file.native_file.path, local_file_path)
-      else
       begin
-          File.write(local_file_path, file.content)
+        File.write(local_file_path, file.read)
       rescue IOError => e
         raise Runner::Error::WorkspaceError.new("Could not create file #{file.filepath}: #{e.inspect}")
       end
     end
-    end
     FileUtils.chmod_R('+rwtX', local_workspace_path)
   ensure
     Rails.logger.debug { "#{Time.zone.now.getutc.inspect}: Finished copying files" }

lib/runner/strategy/poseidon.rb

@@ -116,7 +116,7 @@ class Runner::Strategy::Poseidon < Runner::Strategy
     copy = files.map do |file|
       {
         path: file.filepath,
-        content: Base64.strict_encode64(file.content.presence || file.native_file.read || ''),
+        content: Base64.strict_encode64(file.read || ''),
       }
     end
 

spec/controllers/submissions_controller_spec.rb

@@ -131,7 +131,7 @@ describe SubmissionsController do
         expect_http_status(:ok)
 
         it 'renders the file content' do
-          expect(response.body).to eq(file.native_file.read)
+          expect(response.body).to eq(file.read)
         end
       end
 

spec/lib/runner/strategy/docker_container_pool_spec.rb

@@ -160,7 +160,7 @@ describe Runner::Strategy::DockerContainerPool do
       let(:files) { [build(:file, :image)] }
 
       it 'copies the file inside the workspace' do
-        expect(FileUtils).to receive(:cp).with(files.first.native_file.path, local_path.join(files.first.filepath))
+        expect(File).to receive(:write).with(local_path.join(files.first.filepath), files.first.read)
         container_pool.copy_files(files)
       end
     end

spec/models/code_ocean/file_spec.rb

@@ -56,4 +56,24 @@ describe CodeOcean::File do
       expect(file.errors[:weight]).to be_present
     end
   end
+
+  context 'with a native file' do
+    let(:file) { create(:file, :image) }
+
+    after { file.native_file.remove! }
+
+    context 'when the path has not been modified' do
+      it 'reads the native file' do
+        expect(file.read).to be_present
+      end
+    end
+
+    context 'when the path has been modified' do
+      before { file.update(native_file: '../../../../secrets.yml') }
+
+      it 'does not read the native file' do
+        expect(file.read).not_to be_present
+      end
+    end
+  end
 end